Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S933203AbcKPS6m (ORCPT ); Wed, 16 Nov 2016 13:58:42 -0500 Received: from mail-wm0-f48.google.com ([74.125.82.48]:38379 "EHLO mail-wm0-f48.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932676AbcKPS6k (ORCPT ); Wed, 16 Nov 2016 13:58:40 -0500 MIME-Version: 1.0 In-Reply-To: <20161116100925.GM3142@twins.programming.kicks-ass.net> References: <20161114173946.501528675@infradead.org> <20161114174446.486581399@infradead.org> <20161115073322.GC28248@kroah.com> <20161115080314.GD3142@twins.programming.kicks-ass.net> <20161116100925.GM3142@twins.programming.kicks-ass.net> From: Kees Cook Date: Wed, 16 Nov 2016 10:58:38 -0800 X-Google-Sender-Auth: vDA2dhAYnbljulpHZ3XN_-RisYk Message-ID: Subject: Re: [RFC][PATCH 2/7] kref: Add kref_read() To: Peter Zijlstra Cc: Greg KH , Will Deacon , "Reshetova, Elena" , Arnd Bergmann , Thomas Gleixner , Ingo Molnar , "H. Peter Anvin" , David Windsor , LKML , Alexei Starovoitov , Daniel Borkmann Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1487 Lines: 48 On Wed, Nov 16, 2016 at 2:09 AM, Peter Zijlstra wrote: > On Tue, Nov 15, 2016 at 12:53:35PM -0800, Kees Cook wrote: >> >> What should we do about things like this (bpf_prog_put() and callbacks >> from kernel/bpf/syscall.c): >> >> >> static void bpf_prog_uncharge_memlock(struct bpf_prog *prog) >> { >> struct user_struct *user = prog->aux->user; >> >> atomic_long_sub(prog->pages, &user->locked_vm); >> free_uid(user); >> } >> >> static void __bpf_prog_put_rcu(struct rcu_head *rcu) >> { >> struct bpf_prog_aux *aux = container_of(rcu, struct bpf_prog_aux, rcu); >> >> free_used_maps(aux); >> bpf_prog_uncharge_memlock(aux->prog); >> bpf_prog_free(aux->prog); >> } >> >> void bpf_prog_put(struct bpf_prog *prog) >> { >> if (atomic_dec_and_test(&prog->aux->refcnt)) >> call_rcu(&prog->aux->rcu, __bpf_prog_put_rcu); >> } >> >> >> Not only do we want to protect prog->aux->refcnt, but I think we want >> to protect user->locked_vm too ... I don't think it's sane for >> user->locked_vm to be a stats_t ? > > Why would you want to mess with locked_vm? You seem of the opinion that > everything atomic_t is broken, this isn't the case. What I mean to say is that while the refcnt here should clearly be converted to kref or refcount_t, it looks like locked_vm should become a new stats_t. However, it seems weird for locked_vm to ever wrap either... -Kees -- Kees Cook Nexus Security