|
Message-ID: <CAKv+Gu_ss7a4putFyStN6RaK_OM3fFezB3MhF1O1+9=0RaupZA@mail.gmail.com> 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.