Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Thu, 3 Aug 2017 23:42:49 +0100
From: Ard Biesheuvel <ard.biesheuvel@...aro.org>
To: Kees Cook <keescook@...omium.org>
Cc: Arnd Bergmann <arnd@...db.de>, 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 3 August 2017 at 23:14, Kees Cook <keescook@...omium.org> wrote:
> On Thu, Aug 3, 2017 at 11:27 AM, Ard Biesheuvel
> <ard.biesheuvel@...aro.org> wrote:
>> On 3 August 2017 at 05:35, Kees Cook <keescook@...omium.org> wrote:
>>> On Thu, Jul 6, 2017 at 4:16 PM, Kees Cook <keescook@...omium.org> wrote:
>>>> On Thu, Jul 6, 2017 at 2:44 PM, Kees Cook <keescook@...omium.org> wrote:
>>>>> 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>
>>>>>
>>>>> Ard, I'd be curious what you see for "size" difference between builds
>>>>> and if it's visible with hackbench or other things?
>>>>
>>>> Hm, not all that bad on the size front:
>>>>
>>>>    text                 data            bss                 dec
>>>>             hex filename
>>>> 10950705        5592525 13955072        30498302        1d15dfe vmlinux
>>>> 11048035        5592365 13955072        30595472        1d2d990
>>>> vmlinux.initautobyref
>>>>
>>>> And yes, as expected, wow there are a lot of notices in verbose mode. ;)
>>>>
>>>> My pet favorite, from the NAKed patch I sent forever ago, is covered
>>>> (as expected):
>>>>
>>>> net/socket.c: In function ‘SYSC_getsockname’:
>>>> net/socket.c:1605:26: note: auto variable will be forcibly initialized
>>>>   struct sockaddr_storage address;
>>>>                           ^~~~~~~
>>>
>>> While this was an RFC, it seems to work well and, as Daniel mentioned,
>>> provides another benchmark for future optimizations of this kind of
>>> protection. Besides the COMPILE_TEST change already discussed, any
>>> other changes or objections before I carry this in -next?
>>>
>>
>> Sounds reasonable to me.
>
> Actually, I just looked at the diff between structleak and
> initautobyref, and it's essentially 1 test (and the removal of all the
> __user-detection code):
>
> -               /* if the type is of interest, examine the variable */
> -               if (TYPE_USERSPACE(type))
> +               /* initialize the variable if its address is taken */
> +               if (TREE_ADDRESSABLE (var))
>
> Perhaps instead of a whole new plugin, could we just add the
> functionality to the existing structleak plugin as a Kconfig option?
> Like maybe CONFIG_GCC_PLUGIN_STRUCTLEAK_TAKEN?
>

Yeah, it is mostly the same code. As you know, it was mainly intended
as a PoC but given the interest to merge this functionality for real,
I will do another pass and try to incorporate it more cleanly.

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.