Received: by 2002:a25:6193:0:0:0:0:0 with SMTP id v141csp1136019ybb; Wed, 1 Apr 2020 16:39:18 -0700 (PDT) X-Google-Smtp-Source: APiQypLKfW45YLeLc65SQ6L9V3uZDj6V3fNh4I2g2m1/iGcy6a10pSdCvB7TTS5XWmjvn6BKe8JP X-Received: by 2002:aca:cdcd:: with SMTP id d196mr360902oig.16.1585784357978; Wed, 01 Apr 2020 16:39:17 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1585784357; cv=none; d=google.com; s=arc-20160816; b=chaTxQn/zOe7Y1ty2ab6rG6L++KUXw23GsKz7eI/jgLA9ie63XEY/tnur6f8qxcv09 OHf7B+2t675F2LpxKI/9LsSLENKVe6pW2+fHGI4V2j7ggHCR/nRClNBdBMqu8SmSvl+O lm9W0LgclMrGiu6qCoyW305WuTZDC1m3byYY6YbDtxfLpEA5Ytt33/UWKQki63g452Yn wG9edSx+F932+CoQdTgKZN26m/jbCaUOg5v80oJsr9aT2HolaE3EtcgHlgc22e23J6sd KDDz/oR6echx5OBxjH5MvTQfbE7o0BM6nw9zx+FBy5tpnuMWcLC76wQSB8bOE6xnT+7v j9zA== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:sender:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:dkim-signature; bh=vaEwhKzYh+esdrS+zsnAFLt/hs58JrgfVkhu1l8Vw2I=; b=FvQASNn4X79YAKBHu4zh6QmpOI/XqXgBxvj2wkeOakCxmpJp4yLRyHKp6mENOx9taj pa7XBc0h0Wdj/Ep33wiVaHc1gda869iIv17YbTrf5rQ1J8HrcWOXUt7giB4WuRC4Iqbc RdvpwvGDsl1sGf2LWB/rHXpAZwVzgaOJc69x7T3knw+GsZcvxUZA3KMz4k/Oct4fQC/O XUyBWQK9Tywj/DekYsm3GiCr/z/H4zaxYPEOEwnN/dG2V8UyNrn+Y444uijCRa0IEaUD YybJkVpFlnNV9Hys59PWZct/qLOP9Gmm8jRG5ucWk60smd5WzocBOoumCRsyKLKKHkhZ mNlQ== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@google.com header.s=20161025 header.b=j5KKPNVG; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=REJECT sp=REJECT dis=NONE) header.from=google.com Return-Path: Received: from vger.kernel.org (vger.kernel.org. [209.132.180.67]) by mx.google.com with ESMTP id c20si1471710otf.37.2020.04.01.16.39.05; Wed, 01 Apr 2020 16:39:17 -0700 (PDT) Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67; Authentication-Results: mx.google.com; dkim=pass header.i=@google.com header.s=20161025 header.b=j5KKPNVG; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=REJECT sp=REJECT dis=NONE) header.from=google.com Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1733272AbgDAXhn (ORCPT + 99 others); Wed, 1 Apr 2020 19:37:43 -0400 Received: from mail-lj1-f195.google.com ([209.85.208.195]:45043 "EHLO mail-lj1-f195.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1732503AbgDAXhn (ORCPT ); Wed, 1 Apr 2020 19:37:43 -0400 Received: by mail-lj1-f195.google.com with SMTP id p14so1272208lji.11 for ; Wed, 01 Apr 2020 16:37:40 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=vaEwhKzYh+esdrS+zsnAFLt/hs58JrgfVkhu1l8Vw2I=; b=j5KKPNVGYu2o585DNi9+bxWpfCgtnw9/ky+To5RepGt/igy9pFKVIFK7gwid+7S/4Z XNWvqNuC+XDfoVVC5kx18GZ/N4ge7YG3Criti6WRl/S2yfVjCIAWxiK8K9NzAE+T5QfY esfbUOrI7sqPMSDl1sZkOaeN1eE3GHNT8RbSqXUUDA9qgNV3nxfJg3BIF8le84hKV7+1 nsr6rfFXN8QbwHRZyl66rSV9YafdY5w8ZaaqinYWnLmxZMv4Upb6knX0ua3uBj/Ns41s jHme40iA8VPDDl7u2FrE3JPBkYOvRML2CCu8dUKCcqUDcWmIuulDGzgqCbruxRtW0XsM 26wA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=vaEwhKzYh+esdrS+zsnAFLt/hs58JrgfVkhu1l8Vw2I=; b=T0FrwZVzWfIKbCeMBM7vzE/i6hqEkPVg+NpzvN4N8kl6+d++mOA1ymOsSfFn3HUft7 /p5phxSiK3yGxnOC7a9otAXYXSVnRYi019SSqGMG7j9xpLtC94LmSE0gcJ+yBGcJZBbL JszvodVy2h4uyrS4fHO52KMctIs+sZz8BYoc8LY91GMU0+EqGY8hoYmjidC8/TQ2Fdg/ 63vsTl5S2WOYwonqaEalvtrXq0+3Ef2eG1qlDYNQhcA9zfG4vTgtWwCauNfTv0ILfGNg oCM6x/boUUA5AAxXo+sMbUjXY98OclrCSPgx3pdGaCSLJn4RQRUcmWSQWgZdTS6YnJPX gwtg== X-Gm-Message-State: AGi0PuazHIM7JsAh+13bnLvAowwKBHz8UqHu5VIF5mX3NFo0/ZAarxei 1nhrkOxyWNXIBr8ji9ivD0FsmC79jk3AdMvCI70B6A== X-Received: by 2002:a2e:9247:: with SMTP id v7mr313238ljg.215.1585784259500; Wed, 01 Apr 2020 16:37:39 -0700 (PDT) MIME-Version: 1.0 References: <20200324215049.GA3710@pi3.com.pl> <202003291528.730A329@keescook> <87zhbvlyq7.fsf_-_@x220.int.ebiederm.org> In-Reply-To: <87zhbvlyq7.fsf_-_@x220.int.ebiederm.org> From: Jann Horn Date: Thu, 2 Apr 2020 01:37:13 +0200 Message-ID: Subject: Re: [PATCH] signal: Extend exec_id to 64bits To: "Eric W. Biederman" , Alan Stern , Andrea Parri , Will Deacon , Peter Zijlstra , Boqun Feng , Nicholas Piggin , David Howells , Jade Alglave , Luc Maranget , "Paul E. McKenney" , Akira Yokosawa , Daniel Lustig Cc: Linus Torvalds , Adam Zabrocki , kernel list , Kernel Hardening , Oleg Nesterov , Andy Lutomirski , Bernd Edlinger , Kees Cook , Andrew Morton , stable Content-Type: text/plain; charset="UTF-8" Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org +memory model folks because this seems like a pretty sharp corner On Wed, Apr 1, 2020 at 10:50 PM Eric W. Biederman wrote: > Replace the 32bit exec_id with a 64bit exec_id to make it impossible > to wrap the exec_id counter. With care an attacker can cause exec_id > wrap and send arbitrary signals to a newly exec'd parent. This > bypasses the signal sending checks if the parent changes their > credentials during exec. > > The severity of this problem can been seen that in my limited testing > of a 32bit exec_id it can take as little as 19s to exec 65536 times. > Which means that it can take as little as 14 days to wrap a 32bit > exec_id. Adam Zabrocki has succeeded wrapping the self_exe_id in 7 > days. Even my slower timing is in the uptime of a typical server. > Which means self_exec_id is simply a speed bump today, and if exec > gets noticably faster self_exec_id won't even be a speed bump. > > Extending self_exec_id to 64bits introduces a problem on 32bit > architectures where reading self_exec_id is no longer atomic and can > take two read instructions. Which means that is is possible to hit > a window where the read value of exec_id does not match the written > value. So with very lucky timing after this change this still > remains expoiltable. > > I have updated the update of exec_id on exec to use WRITE_ONCE > and the read of exec_id in do_notify_parent to use READ_ONCE > to make it clear that there is no locking between these two > locations. > > Link: https://lore.kernel.org/kernel-hardening/20200324215049.GA3710@pi3.com.pl > Fixes: 2.3.23pre2 > Cc: stable@vger.kernel.org > Signed-off-by: "Eric W. Biederman" > --- > > Linus would you prefer to take this patch directly or I could put it in > a brach and send you a pull request. > > fs/exec.c | 2 +- > include/linux/sched.h | 4 ++-- > kernel/signal.c | 2 +- > 3 files changed, 4 insertions(+), 4 deletions(-) > > diff --git a/fs/exec.c b/fs/exec.c > index 0e46ec57fe0a..d55710a36056 100644 > --- a/fs/exec.c > +++ b/fs/exec.c > @@ -1413,7 +1413,7 @@ void setup_new_exec(struct linux_binprm * bprm) > > /* An exec changes our domain. We are no longer part of the thread > group */ > - current->self_exec_id++; > + WRITE_ONCE(current->self_exec_id, current->self_exec_id + 1); GCC will generate code for this without complaining, but I think it'll probably generate a tearing store on 32-bit platforms: $ cat volatile-8.c typedef unsigned long long u64; static volatile u64 n; void blah(void) { n = n + 1; } $ gcc -O2 -m32 -c -o volatile-8.o volatile-8.c -Wall $ objdump --disassemble=blah volatile-8.o [...] b: 8b 81 00 00 00 00 mov 0x0(%ecx),%eax 11: 8b 91 04 00 00 00 mov 0x4(%ecx),%edx 17: 83 c0 01 add $0x1,%eax 1a: 83 d2 00 adc $0x0,%edx 1d: 89 81 00 00 00 00 mov %eax,0x0(%ecx) 23: 89 91 04 00 00 00 mov %edx,0x4(%ecx) [...] $ You could maybe use atomic64_t to work around that? atomic64_read() and atomic64_set() should be straightforward READ_ONCE() / WRITE_ONCE() on 64-bit systems while compiling into something more complicated on 32-bit. > flush_signal_handlers(current, 0); > } > EXPORT_SYMBOL(setup_new_exec); > diff --git a/include/linux/sched.h b/include/linux/sched.h > index 04278493bf15..0323e4f0982a 100644 > --- a/include/linux/sched.h > +++ b/include/linux/sched.h > @@ -939,8 +939,8 @@ struct task_struct { > struct seccomp seccomp; > > /* Thread group tracking: */ > - u32 parent_exec_id; > - u32 self_exec_id; > + u64 parent_exec_id; > + u64 self_exec_id; > > /* Protection against (de-)allocation: mm, files, fs, tty, keyrings, mems_allowed, mempolicy: */ > spinlock_t alloc_lock; > diff --git a/kernel/signal.c b/kernel/signal.c > index 9ad8dea93dbb..5383b562df85 100644 > --- a/kernel/signal.c > +++ b/kernel/signal.c > @@ -1926,7 +1926,7 @@ bool do_notify_parent(struct task_struct *tsk, int sig) > * This is only possible if parent == real_parent. > * Check if it has changed security domain. > */ > - if (tsk->parent_exec_id != tsk->parent->self_exec_id) > + if (tsk->parent_exec_id != READ_ONCE(tsk->parent->self_exec_id)) > sig = SIGCHLD; > } > > -- > 2.20.1 >