Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAGXu5j+-AjMKxOYDOpbmJ4X357JRGWViAOmqyqd6bu_w47mffw@mail.gmail.com>
Date: Mon, 5 Mar 2018 13:02:58 -0800
From: Kees Cook <keescook@...omium.org>
To: Linus Torvalds <torvalds@...ux-foundation.org>
Cc: Dave Hansen <dave.hansen@...ux.intel.com>, Alexander Popov <alex.popov@...ux.com>, 
	Kernel Hardening <kernel-hardening@...ts.openwall.com>, PaX Team <pageexec@...email.hu>, 
	Brad Spengler <spender@...ecurity.net>, Ingo Molnar <mingo@...nel.org>, 
	Andy Lutomirski <luto@...nel.org>, Tycho Andersen <tycho@...ho.ws>, Laura Abbott <labbott@...hat.com>, 
	Mark Rutland <mark.rutland@....com>, Ard Biesheuvel <ard.biesheuvel@...aro.org>, 
	Borislav Petkov <bp@...en8.de>, Richard Sandiford <richard.sandiford@....com>, 
	Thomas Gleixner <tglx@...utronix.de>, "H . Peter Anvin" <hpa@...or.com>, 
	Peter Zijlstra <a.p.zijlstra@...llo.nl>, "Dmitry V . Levin" <ldv@...linux.org>, 
	Emese Revfy <re.emese@...il.com>, Jonathan Corbet <corbet@....net>, 
	Andrey Ryabinin <aryabinin@...tuozzo.com>, 
	"Kirill A . Shutemov" <kirill.shutemov@...ux.intel.com>, Thomas Garnier <thgarnie@...gle.com>, 
	Andrew Morton <akpm@...ux-foundation.org>, Alexei Starovoitov <ast@...nel.org>, Josef Bacik <jbacik@...com>, 
	Masami Hiramatsu <mhiramat@...nel.org>, Nicholas Piggin <npiggin@...il.com>, 
	Al Viro <viro@...iv.linux.org.uk>, "David S . Miller" <davem@...emloft.net>, 
	Ding Tianhong <dingtianhong@...wei.com>, David Woodhouse <dwmw@...zon.co.uk>, 
	Josh Poimboeuf <jpoimboe@...hat.com>, Steven Rostedt <rostedt@...dmis.org>, 
	Dominik Brodowski <linux@...inikbrodowski.net>, Juergen Gross <jgross@...e.com>, 
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>, Dan Williams <dan.j.williams@...el.com>, 
	Mathias Krause <minipli@...glemail.com>, Vikas Shivappa <vikas.shivappa@...ux.intel.com>, 
	Kyle Huey <me@...ehuey.com>, Dmitry Safonov <dsafonov@...tuozzo.com>, 
	Will Deacon <will.deacon@....com>, Arnd Bergmann <arnd@...db.de>, X86 ML <x86@...nel.org>, 
	LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH RFC v9 4/7] x86/entry: Erase kernel stack in syscall_trace_enter()

On Mon, Mar 5, 2018 at 12:15 PM, Linus Torvalds
<torvalds@...ux-foundation.org> wrote:
> This is the first I see of any of this, it was apparently not actually
> posted to lkml or anything like that.

This series was still RFC and narrowly focused (e.g. we recently had
compiler folks reviewing it). I'd say we're now to the point where
it's getting mature enough for larger review (I commented on the 0/n
patch to this end earlier today).

> It doesn't actually seem to help *find* bugs at all. As such, it's
> another "paper over and forget" thing that just adds fairly high
> overhead when it's enabled.

This specific class of flaw ("uninitialized" stack contents being used
or leaked) is already being looked for by tons of tools like KASan.
There are teams of people working on it, and they still haven't found
all the cases where these flaws appear.

Luckily we don't have to pick one or the other: we can continue to
look for bugs while defending against them ever happening in the first
place. We've already seen multiple cases of this just with the by-ref
initialization plugin, where a stack content leak goes away because we
asked the compiler to please initialize the memory for us when we
forgot to do it ourselves. Getting the compiler to help us seems like
the obviously correct thing to do, since we're using such a
memory-safety-unfriendly language. :)

> I'm NAK'ing it sight-unseen (see above) just because I'm tired of
> these kinds of pointless things that don't actually strive to improve
> on the kernel, just add more and more overhead for nebulous "things
> may happen", and that just make the code uglier.

I hope you'll reconsider. In this case, I think it does improve the
kernel, especially if we can gain more complete coverage through
native compiler options (instead of just a plugin). Right now, for
example, the kernel is littered with memset()s because the compiler
can't be trusted to correctly zero-init padding, etc. This is an
endless source of bugs, and this patch series provides a comprehensive
and fast way to keep the stack cleared. (See the benchmark I asked for
that compared this 1% "clear only what was used" against the 30% to
wipe the entire stack every time. The latter is obviously insane, and
the former is quite clever and kills an entire bug class.)

-Kees

-- 
Kees Cook
Pixel Security

Powered by blists - more mailing lists

Confused about mailing lists and their use? Read about mailing lists on Wikipedia and check out these guidelines on proper formatting of your messages.