From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-vk0-f46.google.com ([209.85.213.46]:35526 "EHLO mail-vk0-f46.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753235AbcIWVEw (ORCPT ); Fri, 23 Sep 2016 17:04:52 -0400 Received: by mail-vk0-f46.google.com with SMTP id 192so2023650vkl.2 for ; Fri, 23 Sep 2016 14:04:51 -0700 (PDT) MIME-Version: 1.0 In-Reply-To: <1474663238-22134-3-git-send-email-jann@thejh.net> References: <1474663238-22134-1-git-send-email-jann@thejh.net> <1474663238-22134-3-git-send-email-jann@thejh.net> From: Andy Lutomirski Date: Fri, 23 Sep 2016 14:04:30 -0700 Message-ID: Subject: Re: [PATCH v2 2/8] exec: turn self_exec_id into self_privunit To: Jann Horn Cc: Alexander Viro , Roland McGrath , Oleg Nesterov , John Johansen , James Morris , "Serge E. Hallyn" , Paul Moore , Stephen Smalley , Eric Paris , Casey Schaufler , Kees Cook , Andrew Morton , Janis Danisevskis , Seth Forshee , "Eric . Biederman" , Thomas Gleixner , Benjamin LaHaise , Ben Hutchings , Linus Torvalds , Linux FS Devel , LSM List , "security@kernel.org" Content-Type: text/plain; charset=UTF-8 Sender: linux-fsdevel-owner@vger.kernel.org List-ID: On Fri, Sep 23, 2016 at 1:40 PM, Jann Horn wrote: > This ensures that self_privunit ("privilege unit locally unique ID") > is only shared by processes that share the mm_struct and the signal_struct; > not just spatially, but also temporally. In other words, if you do execve() > or clone() without CLONE_THREAD, you get a new privunit that has never > been used before. > > One reason for doing this is that it prevents an attacker from sending an > arbitrary signal to a parent process after performing 2^32-1 execve() > calls. > > The second reason for this is that it permits using the self_exec_luid in > a later patch to check during a ptrace access whether subject and object > are temporally and spatially equal for privilege checking purposes. > > The implementation of locally unique IDs is in sched.h and exec.c for now > because those are the only users so far - if anything else wants to use > them in the future, they can be moved elsewhere. > > changed in v2: > - have 2^64 IDs per CPU instead of 2^64 shared ones (luid scheme, > suggested by Andy Lutomirski) > - take task_lock for reading in setup_new_exec() while bumping the LUID > > Signed-off-by: Jann Horn > --- > fs/exec.c | 41 +++++++++++++++++++++++++++++++++++++++-- > include/linux/sched.h | 17 +++++++++++++++-- > kernel/fork.c | 5 +++-- > kernel/signal.c | 5 ++++- > 4 files changed, 61 insertions(+), 7 deletions(-) > > diff --git a/fs/exec.c b/fs/exec.c > index 84430ee..fcc11f0 100644 > --- a/fs/exec.c > +++ b/fs/exec.c > @@ -1281,6 +1281,34 @@ void would_dump(struct linux_binprm *bprm, struct file *file) > } > EXPORT_SYMBOL(would_dump); > > +static DEFINE_PER_CPU(u64, luid_counters); > + > +static int __init init_luid_counters(void) > +{ > + unsigned int cpu; > + > + for_each_possible_cpu(cpu) { > + /* value 0 is reserved for init */ > + per_cpu(luid_counters, cpu) = 1; > + } > + > + return 0; > +} > +early_initcall(init_luid_counters); How about static DEFINE_PER_CPU(u64, luid_counters) = 1? You could optionally use local64_t instead, which would let you avoid needing to think about preemption. > + > +/* > + * Allocates a new LUID and writes the allocated LUID to @out. > + * This function must not be called from IRQ context. > + */ > +void fill_luid(struct luid *out) > +{ > + preempt_disable(); > + out->count = raw_cpu_read(luid_counters); > + raw_cpu_add(luid_counters, 1); > + out->cpu = smp_processor_id(); > + preempt_enable(); > +} > + I would call this alloc_luid().