On Tue, Apr 9, 2019 at 11:02 AM Alexander Potapenko <glider@xxxxxxxxxx> wrote: > > On Tue, Apr 9, 2019 at 10:38 AM Masahiro Yamada > <yamada.masahiro@xxxxxxxxxxxxx> wrote: > > > > On Tue, Apr 9, 2019 at 2:04 AM Alexander Potapenko <glider@xxxxxxxxxx> wrote: > > > > > > CONFIG_INIT_ALL_MEMORY is going to be an umbrella config for options > > > that force heap and stack initialization. > > > The rationale behind doing so is to reduce the severity of bugs caused > > > by using uninitialized memory. > > > > > > CONFIG_INIT_ALL_STACK turns on stack initialization based on > > > -ftrivial-auto-var-init in Clang builds and on > > > -fplugin-arg-structleak_plugin-byref-all in GCC builds. > > > > > > -ftrivial-auto-var-init is a Clang flag that provides trivial > > > initializers for uninitialized local variables, variable fields and > > > padding. > > > > > > It has three possible values: > > > pattern - uninitialized locals are filled with a fixed pattern > > > (mostly 0xAA on 64-bit platforms, see https://reviews.llvm.org/D54604 > > > for more details) likely to cause crashes when uninitialized value is > > > used; > > > zero (it's still debated whether this flag makes it to the official > > > Clang release) - uninitialized locals are filled with zeroes; > > > uninitialized (default) - uninitialized locals are left intact. > > > > > > The proposed config builds the kernel with > > > -ftrivial-auto-var-init=pattern. > > > > > > Developers have the possibility to opt-out of this feature on a > > > per-variable basis by using __attribute__((uninitialized)). > > > > > > For GCC builds, CONFIG_INIT_ALL_STACK is simply wired up to > > > CONFIG_GCC_PLUGIN_STRUCTLEAK_BYREF_ALL. No opt-out is possible at the > > > moment. > > > > > > Signed-off-by: Alexander Potapenko <glider@xxxxxxxxxx> > > > Cc: Masahiro Yamada <yamada.masahiro@xxxxxxxxxxxxx> > > > Cc: James Morris <jmorris@xxxxxxxxx> > > > Cc: "Serge E. Hallyn" <serge@xxxxxxxxxx> > > > Cc: Nick Desaulniers <ndesaulniers@xxxxxxxxxx> > > > Cc: Kostya Serebryany <kcc@xxxxxxxxxx> > > > Cc: Dmitry Vyukov <dvyukov@xxxxxxxxxx> > > > Cc: Kees Cook <keescook@xxxxxxxxxxxx> > > > Cc: Sandeep Patil <sspatil@xxxxxxxxxxx> > > > Cc: linux-security-module@xxxxxxxxxxxxxxx > > > Cc: linux-kbuild@xxxxxxxxxxxxxxx > > > Cc: kernel-hardening@xxxxxxxxxxxxxxxxxx > > > --- > > > v2: > > > - addressed Kees Cook's comments: added GCC support > > > v3: addressed Masahiro Yamada's comments: > > > - dropped per-file opt-out mechanism > > > - fixed GCC_PLUGINS dependencies > > > --- > > > Makefile | 3 ++- > > > scripts/Makefile.initmem | 10 ++++++++++ > > > security/Kconfig | 1 + > > > security/Kconfig.initmem | 29 +++++++++++++++++++++++++++++ > > > 4 files changed, 42 insertions(+), 1 deletion(-) > > > create mode 100644 scripts/Makefile.initmem > > > create mode 100644 security/Kconfig.initmem > > > > > > diff --git a/Makefile b/Makefile > > > index f070e0d65186..028ca37878fd 100644 > > > --- a/Makefile > > > +++ b/Makefile > > > @@ -448,7 +448,7 @@ export HOSTCXX KBUILD_HOSTCXXFLAGS LDFLAGS_MODULE CHECK CHECKFLAGS > > > > > > export KBUILD_CPPFLAGS NOSTDINC_FLAGS LINUXINCLUDE OBJCOPYFLAGS KBUILD_LDFLAGS > > > export KBUILD_CFLAGS CFLAGS_KERNEL CFLAGS_MODULE > > > -export CFLAGS_KASAN CFLAGS_KASAN_NOSANITIZE CFLAGS_UBSAN > > > +export CFLAGS_KASAN CFLAGS_KASAN_NOSANITIZE CFLAGS_UBSAN CFLAGS_INITMEM > > > export KBUILD_AFLAGS AFLAGS_KERNEL AFLAGS_MODULE > > > export KBUILD_AFLAGS_MODULE KBUILD_CFLAGS_MODULE KBUILD_LDFLAGS_MODULE > > > export KBUILD_AFLAGS_KERNEL KBUILD_CFLAGS_KERNEL > > > @@ -840,6 +840,7 @@ KBUILD_ARFLAGS := $(call ar-option,D) > > > include scripts/Makefile.kasan > > > include scripts/Makefile.extrawarn > > > include scripts/Makefile.ubsan > > > +include scripts/Makefile.initmem > > > > > > # Add any arch overrides and user supplied CPPFLAGS, AFLAGS and CFLAGS as the > > > # last assignments > > > diff --git a/scripts/Makefile.initmem b/scripts/Makefile.initmem > > > new file mode 100644 > > > index 000000000000..a6253d78fe35 > > > --- /dev/null > > > +++ b/scripts/Makefile.initmem > > > @@ -0,0 +1,10 @@ > > > +ifdef CONFIG_INIT_ALL_STACK > > > + > > > +# Clang's -ftrivial-auto-var-init=pattern flag initializes the > > > +# uninitialized parts of local variables (including fields and padding) > > > +# with a fixed pattern (0xAA in most cases). > > > +ifdef CONFIG_CC_HAS_AUTO_VAR_INIT > > > + CFLAGS_INITMEM := -ftrivial-auto-var-init=pattern > > > +endif > > > + > > > +endif > > > > Now that you dropped per-file control, > > why is this necessary? > > Also, CFLAGS_INITMEM is not wired up to anything, > > so this patch has no effect. > There's CFLAGS_INITMEM in the top Makefile, so I suppose it should be working. Ah, now I see. Yes, I was too hasty with this patchset. > But I agree there's no point in adding extra files for a single build flag. > > > > I think you can add the following to top Makefile, and that's it. > > > > ifdef CONFIG_AUTO_VAR_INIT > > KBUILD_CFLAGS += -ftrivial-auto-var-init=pattern > > endif > > > > > > > > > > > diff --git a/security/Kconfig b/security/Kconfig > > > index e4fe2f3c2c65..cc12a39424dd 100644 > > > --- a/security/Kconfig > > > +++ b/security/Kconfig > > > @@ -230,6 +230,7 @@ config STATIC_USERMODEHELPER_PATH > > > If you wish for all usermode helper programs to be disabled, > > > specify an empty string here (i.e. ""). > > > > > > +source "security/Kconfig.initmem" > > > source "security/selinux/Kconfig" > > > source "security/smack/Kconfig" > > > source "security/tomoyo/Kconfig" > > > diff --git a/security/Kconfig.initmem b/security/Kconfig.initmem > > > new file mode 100644 > > > index 000000000000..5e49a55382ad > > > --- /dev/null > > > +++ b/security/Kconfig.initmem > > > @@ -0,0 +1,29 @@ > > > +menu "Initialize all memory" > > > + > > > +config CC_HAS_AUTO_VAR_INIT > > > + def_bool $(cc-option,-ftrivial-auto-var-init=pattern) > > > + > > > +config INIT_ALL_MEMORY > > > + bool "Initialize all memory" > > > + default n > > > + help > > > + Enforce memory initialization to mitigate infoleaks and make > > > + the control-flow bugs depending on uninitialized values more > > > + deterministic. > > > + > > > +if INIT_ALL_MEMORY > > > + > > > +config INIT_ALL_STACK > > > + bool "Initialize all stack" > > > + depends on INIT_ALL_MEMORY > > > + depends on CC_HAS_AUTO_VAR_INIT || (HAVE_GCC_PLUGINS && PLUGIN_HOSTCC != "") > > > + select GCC_PLUGINS if !CC_HAS_AUTO_VAR_INIT > > > + select GCC_PLUGIN_STRUCTLEAK if !CC_HAS_AUTO_VAR_INIT > > > + select GCC_PLUGIN_STRUCTLEAK_BYREF_ALL if !CC_HAS_AUTO_VAR_INIT > > > + default y > > > + help > > > + Initialize uninitialized stack data with a fixed pattern > > > + (0x00 in GCC, 0xAA in Clang). > > > + > > > > > > I am not sure about the benefit of these umbrella CONFIGs > > since these make 'depends on' and 'select' complicated. > > I will leave it to Kees, though. > > > > > > > > > +endif # INIT_ALL_MEMORY > > > +endmenu > > > -- > > > 2.21.0.392.gf8f6787159e-goog > > > > > > > > > -- > > Best Regards > > Masahiro Yamada > > > > -- > Alexander Potapenko > Software Engineer > > Google Germany GmbH > Erika-Mann-Straße, 33 > 80636 München > > Geschäftsführer: Paul Manicle, Halimah DeLaine Prado > Registergericht und -nummer: Hamburg, HRB 86891 > Sitz der Gesellschaft: Hamburg -- Alexander Potapenko Software Engineer Google Germany GmbH Erika-Mann-Straße, 33 80636 München Geschäftsführer: Paul Manicle, Halimah DeLaine Prado Registergericht und -nummer: Hamburg, HRB 86891 Sitz der Gesellschaft: Hamburg
