[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAGXu5j+FDprZBxBKPoBV0mw_oAEG+6Rh-8QNBcniVPxVTQ5gaQ@mail.gmail.com>
Date: Thu, 6 Jul 2017 14:44:14 -0700
From: Kees Cook <keescook@...omium.org>
To: Arnd Bergmann <arnd@...db.de>
Cc: Ard Biesheuvel <ard.biesheuvel@...aro.org>,
Kernel Hardening <kernel-hardening@...ts.openwall.com>,
Linus Torvalds <torvalds@...ux-foundation.org>
Subject: Re: [RFC/RFT PATCH] gcc-plugins: force initialize auto variables
whose addresses are taken
On Thu, Jul 6, 2017 at 4:09 AM, Arnd Bergmann <arnd@...db.de> wrote:
> On Thu, Jul 6, 2017 at 12:13 PM, Ard Biesheuvel
> <ard.biesheuvel@...aro.org> wrote:
>> To prevent leaking stack contents in cases where it is not possible
>> for the compiler to figure out whether an automatic variable has been
>> initialized or not, add a plugin that forcibly initializes all automatic
>> variables of struct/union types if their address is taken at any point.
>>
>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@...aro.org>
>
> If we only do this for variables that have their address taken, we miss
> a lot of cases that the compiler (both clang and gcc) decide not to warn
> about but that can still cause undefined behavior, e.g.:
>
> extern int g(void);
> int f(void)
> {
> int i;
>
> switch (g()) {
> case 1:
> i = 0;
> }
>
> return i;
> }
>
> which gets compiled without warning under the assumption that
> g() always returns '1':
>
> 0000000000000000 <f>:
> 0: 48 83 ec 08 sub $0x8,%rsp
> 4: e8 00 00 00 00 callq 9 <f+0x9>
> 5: R_X86_64_PC32 g-0x4
> 9: b8 00 00 00 00 mov $0x0,%eax
> e: 48 83 c4 08 add $0x8,%rsp
> 12: c3 retq
>
> Detecting those cases from the plugin may be a lot harder.
>
>> --- a/arch/Kconfig
>> +++ b/arch/Kconfig
>> @@ -443,6 +443,15 @@ config GCC_PLUGIN_STRUCTLEAK_VERBOSE
>> initialized. Since not all existing initializers are detected
>> by the plugin, this can produce false positive warnings.
>>
>> +config GCC_PLUGIN_INITAUTOBYREF
>> + bool "Force initialization of auto variables that have their address taken"
>> + depends on GCC_PLUGINS
>> + help
>> +
>> +config GCC_PLUGIN_INITAUTOBYREF_VERBOSE
>> + bool "Report uninitialized auto variables that have their address taken"
>> + depends on GCC_PLUGIN_INITAUTOBYREF
>
> I think this should be
>
> depends on GCC_PLUGIN_INITAUTOBYREF || COMPILE_TEST
>
> to avoid producing output in an allmodconfig build.
Err, surely you mean:
depends on GCC_PLUGIN_INITAUTOBYREF && !COMPILE_TEST
?
I've done this with the other _VERBOSE settings, and yeah, this one
should do it too.
Ard, I'd be curious what you see for "size" difference between builds
and if it's visible with hackbench or other things?
-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.
