Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Thu, 6 Jul 2017 13:09:02 +0200
From: Arnd Bergmann <arnd@...db.de>
To: Ard Biesheuvel <ard.biesheuvel@...aro.org>
Cc: Kernel Hardening <kernel-hardening@...ts.openwall.com>, Kees Cook <keescook@...omium.org>, 
	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 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.

      Arnd

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.