[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <CAGXu5jJ=_zS3U_NgpC8xpQokXXch6Z-rz0LVLWS2z6XS0POKuw@mail.gmail.com>
Date: Thu, 3 Aug 2017 15:43:40 -0700
From: Kees Cook <keescook@...omium.org>
To: Ard Biesheuvel <ard.biesheuvel@...aro.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 Thu, Aug 3, 2017 at 3:42 PM, Ard Biesheuvel
<ard.biesheuvel@...aro.org> wrote:
> 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.
Yup, I just hadn't actually looked to see how much was reused. :) Thanks!
-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.
