|
Message-ID: <CA+55aFx45SM4VxD-V4qLo8=hxqYtGH+qBeB74KekwCkJK7D=xA@mail.gmail.com> Date: Wed, 14 Mar 2018 09:29:07 -0700 From: Linus Torvalds <torvalds@...ux-foundation.org> To: Florian Weimer <fweimer@...hat.com> Cc: Kees Cook <keescook@...omium.org>, Ingo Molnar <mingo@...nel.org>, P J P <pjp@...oraproject.org>, Ard Biesheuvel <ard.biesheuvel@...aro.org>, Steven Rostedt <rostedt@...dmis.org>, Arnd Bergmann <arnd@...db.de>, Daniel Micay <danielmicay@...il.com>, 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>, 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: Fully initialized stack usage (was Re: [PATCH RFC v9 4/7] x86/entry: Erase kernel stack in syscall_trace_enter()) On Wed, Mar 14, 2018 at 7:21 AM, Florian Weimer <fweimer@...hat.com> wrote: > > What would be the model for the kernel? Write zero to the padding > initially, and on copying structs, make sure that you either copy the > padding from the source, or clear the target? > > Clearing the padding while copying might be somewhat expensive. Oh, no, I wouldn't expect that at all. A structure copy assignment should act as a memcpy. I think the semantics should be that any padding is basically "unnamed content". So copying copies the whole struct, padding and all. Just as an example: struct a { char a; int b; }; int fn(struct a *); int test(struct a *dummy) { struct a a1 = { 1, 2 }; struct a a2 = { }; struct a a3 = *dummy; a2.b = 123; return fn(&a1) + fn(&a2) + fn(&a3); } and you can see examples of the bug, but also examples of right behavior. My current gcc (7.3.1-2 from F27) does movb $1, 8(%rsp) movl $2, 12(%rsp) for that first one - which is problematic because it will pass uninitialized state to 'fn()'. The second one results in good code: movl $0, 16(%rsp) movl $123, 20(%rsp) which is good and doesn't leave the padding uninitialized, and the third one results in movq (%rdi), %rax movq %rax, 24(%rsp) which is again fine. So it mostly works as expected, but things that initialize the structure one member at a time end up entirely skipping the padding, resulting in uninitialized parts on the stack being passed around. That's why I say that I think the semantics we would want is that padding acts as unnamed entries, ie it should _act_ as if the structure declaration was struct a { char a; char __padding[3]; int b; }; and that initialization of 'a1' then only iterated over the actual named entries, so that a1 = { 1, 2 } initializer would act like: struct a a1 = { .a = 1, .b = 2 }; which currently makes gcc generate movl $0, 8(%rsp) movl $2, 12(%rsp) movb $1, 8(%rsp) and initializes things properly. Ok, so gcc is admittedly oddly _stupid_ about it (a little bit clever, a little bit stupid), but that would still be quite acceptablke for the kernel. Note that the "unnamed padding" fields would automatically do the right thing for structure copies and everything else. It would literally only affect the case of assigning fields one by one, and force the padding to be zeroed too. So as far as implementation would go, I would suggest that one possible implementation is literally to create those dummy padding entries in the struct definition, and add a flag for "real" entries, and make unnamed initializers skip padding ones. But I haven't looked at how the initializer generation works in gcc, there may be some clever place that already knows about "oh, we skipped over padding, so let's just initialize it". 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.