Re: [PATCH RFC v9 4/7] x86/entry: Erase kernel stack in syscall_trace_enter()

[Linus Torvalds <torvalds@linux-foundation.org>]

On Mon, Mar 5, 2018 at 1:02 PM, Kees Cook <keescook@chromium.org> wrote:
> 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.

Right.

So let's do the *smart* thing, not the *stupid* thing.

This "mindlessly clear stack after use" is stupid.

There are smart things we can do, and it's not just about "find the
problems" like KASAN, but also "avoid undefined behavior".

I absolutely detest undefined compiler behavior. We should fix it. One
of the biggest mistakes C ever did was to have "undefined" in the
standard, and we already obviously limit that kind of broken behavior
with -fwrapv and -fno-strict-alias.

Both of those disable what I consider to be just bugs in the C
standard that should not be allowed for system software - it's a class
of "stupid compiler tricks to generate bad code", which by definition
a compiler shouldn't do. A compiler should generate *good* code, not
random bad code.

And yes:

> 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. :)

This is more of the *smart* kind of behavior - I'm also perfectly
willing to say that automatic variables should just always initialize
to zero, exactly the same way static variables do.

And it doesn't necessarily generate any worse code.

Why? Because it's the _intelligent_ thing to do - unlike some
completely mindless idiotic "clear the stack" patch.

Now, I haven't actually seen the patches, I've only seen signs of
them, but the signs I have seen very much seem to say "this is the
mindless and *stupid* kind of crap that we should not do".

Things like adding hundreds of lines of new asm code, just because.

So by all means, push the zero initializations of automatic variables.
That's just good sense.

But don't push this crap.

              Linus