[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Tue, 6 Mar 2018 13:01:20 -0800
From: Linus Torvalds <torvalds@...ux-foundation.org>
To: Arnd Bergmann <arnd@...db.de>
Cc: Ard Biesheuvel <ard.biesheuvel@...aro.org>, Daniel Micay <danielmicay@...il.com>,
Ingo Molnar <mingo@...nel.org>, Kees Cook <keescook@...omium.org>,
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>, Andy Lutomirski <luto@...nel.org>,
Tycho Andersen <tycho@...ho.ws>, Laura Abbott <labbott@...hat.com>, Mark Rutland <mark.rutland@....com>,
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>, 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 Tue, Mar 6, 2018 at 12:42 PM, Arnd Bergmann <arnd@...db.de> wrote:
>
> Forcing gcc to allocate a stack slot and zero-initialize it should
> find many bugs by adding valid warnings, but also add lots of
> false positives as well as prevent important optimizations in other
> places that are actually well-defined.
Oh, no, the "force gcc to allocate a stack slot" would be absolutely insane.
You should never do that. Anybody who does that should be shot.
But you don't have to force any stack allocation. You should just
initialize it to zero (*without* the stack allocation).
Then the optimization passes will just remove the initialization in
99.9% of all cases. Only very occasionally - when gcc cannot see it
being overwritten - would it remain. And those are exactly the cases
where you *want* it to remain.
Of course, it is possible that you can't just do that from a plugin.
But I can almost guarantee that it would be trivial to do as a gcc
switch if any gcc people wanted to do it. Just look at each scalar
auto variable, and if it doesn't have an initializer, just add one to
zero completely mindlessly.
As the crusaders said: "Kill them all and let the optimizer sort them out".
(Note: the "trivial to do" is about scalar values only. Non-scalar
values have more complexities, like struct padding issues etc).
>> If somebody has big arrays on the stack, that's often a problem
>> anyway. It may be common in non-kernel code, but kernel code is very
>> special.
>
> I can think of a few cases that are important, this one is well-known:
>
> int core_sys_select(int n, fd_set __user *inp, fd_set __user *outp,
> fd_set __user *exp, struct timespec64 *end_time)
> {
> ....
> /* Allocate small arguments on the stack to save memory and be faster */
> long stack_fds[SELECT_STACK_ALLOC/sizeof(long)];
Oh, I can well imagine things like this.
But they will be a handful, they will be seen in profiles, and they'd
be easy to mark. And then you can validate the ones you mark, instead
of worrying about all the ones you don't even know about.
> There is also the really scary code like:
>
> #define SKCIPHER_REQUEST_ON_STACK(name, tfm) \
> char __##name##_desc[sizeof(struct skcipher_request) + \
> crypto_skcipher_reqsize(tfm)] CRYPTO_MINALIGN_ATTR; \
> struct skcipher_request *name = (void *)__##name##_desc
The crypto stuff does disgusting things. It used to do VLA's in
structures too, that has morphed into using these crazy XYZ_ON_STACK()
defines.
They eventually need to fix their own crap at some point. It shouldn't
be something we care about from a design standpoint.
Linus
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.
