Kernel-hardening archive on lore.kernel.org
 help / color / Atom feed
From: Jann Horn <jannh@google.com>
To: Linus Torvalds <torvalds@linux-foundation.org>
Cc: "Eric W. Biederman" <ebiederm@xmission.com>,
	Alan Stern <stern@rowland.harvard.edu>,
	 Andrea Parri <parri.andrea@gmail.com>,
	Will Deacon <will@kernel.org>,
	 Peter Zijlstra <peterz@infradead.org>,
	Boqun Feng <boqun.feng@gmail.com>,
	 Nicholas Piggin <npiggin@gmail.com>,
	David Howells <dhowells@redhat.com>,
	 Jade Alglave <j.alglave@ucl.ac.uk>,
	Luc Maranget <luc.maranget@inria.fr>,
	 "Paul E. McKenney" <paulmck@kernel.org>,
	Akira Yokosawa <akiyks@gmail.com>,
	 Daniel Lustig <dlustig@nvidia.com>,
	Adam Zabrocki <pi3@pi3.com.pl>,
	 kernel list <linux-kernel@vger.kernel.org>,
	 Kernel Hardening <kernel-hardening@lists.openwall.com>,
	Oleg Nesterov <oleg@redhat.com>,
	 Andy Lutomirski <luto@amacapital.net>,
	Bernd Edlinger <bernd.edlinger@hotmail.de>,
	 Kees Cook <keescook@chromium.org>,
	Andrew Morton <akpm@linux-foundation.org>,
	 stable <stable@vger.kernel.org>, Marco Elver <elver@google.com>,
	 Dmitry Vyukov <dvyukov@google.com>,
	kasan-dev <kasan-dev@googlegroups.com>
Subject: Re: [PATCH] signal: Extend exec_id to 64bits
Date: Thu, 2 Apr 2020 03:35:44 +0200
Message-ID: <CAG48ez1f82re_V=DzQuRHpy7wOWs1iixrah4GYYxngF1v-moZw@mail.gmail.com> (raw)
In-Reply-To: <CAHk-=wgM3qZeChs_1yFt8p8ye1pOaM_cX57BZ_0+qdEPcAiaCQ@mail.gmail.com>

On Thu, Apr 2, 2020 at 1:55 AM Linus Torvalds
<torvalds@linux-foundation.org> wrote:
> On Wed, Apr 1, 2020 at 4:51 PM Linus Torvalds
> <torvalds@linux-foundation.org> wrote:
> >
> > It's literally testing a sequence counter for equality. If you get
> > tearing in the high bits on the write (or the read), you'd still need
> > to have the low bits turn around 4G times to get a matching value.
>
> Put another way: first you'd have to work however many weeks to do 4
> billion execve() calls, and then you need to hit basically a
> single-instruction race to take advantage of it.
>
> Good luck with that. If you have that kind of God-like capability,
> whoever you're attacking stands no chance in the first place.

I'm not really worried about someone being able to hit this in
practice, especially given that the only thing it lets you do is send
signals; I just think that the code is wrong in principle, and that we
should avoid having "technically wrong, but works in practice" code in
the kernel.

This kind of intentional race might also trip up testing tools like
the upcoming KCSAN instrumentation, unless it is specifically
annotated as an intentionally racing instruction. (For now, KCSAN is
64-bit only, so it won't actually be able to detect this though; and
the current KCSAN development branch actually incorrectly considers
WRITE_ONCE() to always be atomic; but still, in principle it's the
kind of issue KCSAN is supposed to detect, I think.)

But even without KCSAN, if we have tearing stores racing with loads, I
think that we ought to *at least* have a comment noting that we're
intentionally doing that so that people don't copy this kind of code
to a place where the high bits change more frequently and correctness
matters more.

Since the read is already protected by the tasklist_lock, an
alternative might be to let the execve path also take that lock to
protect the sequence number update, given that execve is not a
particularly hot path. Or we could hand-code the equality check and
increment operations to be properly race-free.

  reply index

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-03-24 21:50 Curiosity around 'exec_id' and some problems associated with it Adam Zabrocki
2020-03-29 22:43 ` Kees Cook
2020-03-30  8:34   ` Oleg Nesterov
2020-03-31  4:29   ` Adam Zabrocki
2020-04-01 20:47   ` [PATCH] signal: Extend exec_id to 64bits Eric W. Biederman
2020-04-01 20:55     ` Linus Torvalds
2020-04-01 21:03       ` Eric W. Biederman
2020-04-01 23:37     ` Jann Horn
2020-04-01 23:51       ` Linus Torvalds
2020-04-01 23:55         ` Linus Torvalds
2020-04-02  1:35           ` Jann Horn [this message]
2020-04-02  2:05             ` Linus Torvalds
2020-04-02 13:11               ` Eric W. Biederman
2020-04-02 18:06                 ` Linus Torvalds
2020-04-02  4:46     ` Jann Horn
2020-04-02 14:14       ` Eric W. Biederman
2020-04-03  2:11       ` Adam Zabrocki
2020-04-02  7:19     ` Kees Cook
2020-04-02  7:22     ` Bernd Edlinger

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CAG48ez1f82re_V=DzQuRHpy7wOWs1iixrah4GYYxngF1v-moZw@mail.gmail.com' \
    --to=jannh@google.com \
    --cc=akiyks@gmail.com \
    --cc=akpm@linux-foundation.org \
    --cc=bernd.edlinger@hotmail.de \
    --cc=boqun.feng@gmail.com \
    --cc=dhowells@redhat.com \
    --cc=dlustig@nvidia.com \
    --cc=dvyukov@google.com \
    --cc=ebiederm@xmission.com \
    --cc=elver@google.com \
    --cc=j.alglave@ucl.ac.uk \
    --cc=kasan-dev@googlegroups.com \
    --cc=keescook@chromium.org \
    --cc=kernel-hardening@lists.openwall.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=luc.maranget@inria.fr \
    --cc=luto@amacapital.net \
    --cc=npiggin@gmail.com \
    --cc=oleg@redhat.com \
    --cc=parri.andrea@gmail.com \
    --cc=paulmck@kernel.org \
    --cc=peterz@infradead.org \
    --cc=pi3@pi3.com.pl \
    --cc=stable@vger.kernel.org \
    --cc=stern@rowland.harvard.edu \
    --cc=torvalds@linux-foundation.org \
    --cc=will@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Kernel-hardening archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/kernel-hardening/0 kernel-hardening/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 kernel-hardening kernel-hardening/ https://lore.kernel.org/kernel-hardening \
		kernel-hardening@lists.openwall.com
	public-inbox-index kernel-hardening

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/com.openwall.lists.kernel-hardening


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git