* [PATCH v3 0/2] RFC: introduce CONFIG_INIT_ALL_MEMORY @ 2019-04-08 17:04 Alexander Potapenko 2019-04-08 17:04 ` [PATCH v3 1/2] initmem: introduce CONFIG_INIT_ALL_MEMORY and CONFIG_INIT_ALL_STACK Alexander Potapenko 2019-04-08 17:04 ` [PATCH v3 2/2] initmem: introduce CONFIG_INIT_ALL_HEAP Alexander Potapenko 0 siblings, 2 replies; 12+ messages in thread From: Alexander Potapenko @ 2019-04-08 17:04 UTC (permalink / raw) To: yamada.masahiro, jmorris, serge Cc: linux-security-module, linux-kbuild, ndesaulniers, kcc, dvyukov, keescook, sspatil, kernel-hardening This patch is a part of a bigger initiative to allow initializing heap/stack memory in the Linux kernels by default. The rationale behind doing so is to reduce the severity of bugs caused by using uninitialized memory. Over the last two years KMSAN (https://github.com/google/kmsan/) has found more than a hundred bugs running in a really moderate setup (orders of magnitude less CPU/months than KASAN). Some of those bugs led to information leaks if uninitialized memory was copied to the userspace, other could cause DoS because of subverted control flow. A lot more bugs remain uncovered, so we want to provide the distros and OS vendors with a last resort measure to mitigate such bugs. Our plan is to introduce configuration flags to force initialization of stack and heap variables with a fixed pattern. This is going to render information leaks inefficient (as we'll only leak pattern data) and make uses of uninitialized values in conditions more deterministic and discoverable. The stack instrumentation part is based on Clang's -ftrivial-auto-var-init (see https://reviews.llvm.org/D54604 ; there's also a GCC feature request for a similar flag: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87210) or GCC's -fplugin-arg-structleak_plugin-byref-all The heap initialization part is compiler-agnostic and is based on the existing CONFIG_SLUB_DEBUG and CONFIG_PAGE_POISONING. Alexander Potapenko (2): initmem: introduce CONFIG_INIT_ALL_MEMORY and CONFIG_INIT_ALL_STACK initmem: introduce CONFIG_INIT_ALL_HEAP Makefile | 3 ++- mm/page_poison.c | 5 +++++ mm/slub.c | 2 ++ scripts/Makefile.initmem | 10 ++++++++++ security/Kconfig | 1 + security/Kconfig.initmem | 40 ++++++++++++++++++++++++++++++++++++++++ 6 files changed, 60 insertions(+), 1 deletion(-) create mode 100644 scripts/Makefile.initmem create mode 100644 security/Kconfig.initmem -- 2.21.0.392.gf8f6787159e-goog ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v3 1/2] initmem: introduce CONFIG_INIT_ALL_MEMORY and CONFIG_INIT_ALL_STACK 2019-04-08 17:04 [PATCH v3 0/2] RFC: introduce CONFIG_INIT_ALL_MEMORY Alexander Potapenko @ 2019-04-08 17:04 ` Alexander Potapenko 2019-04-08 22:15 ` Randy Dunlap 2019-04-09 8:37 ` Masahiro Yamada 2019-04-08 17:04 ` [PATCH v3 2/2] initmem: introduce CONFIG_INIT_ALL_HEAP Alexander Potapenko 1 sibling, 2 replies; 12+ messages in thread From: Alexander Potapenko @ 2019-04-08 17:04 UTC (permalink / raw) To: yamada.masahiro, jmorris, serge Cc: linux-security-module, linux-kbuild, ndesaulniers, kcc, dvyukov, keescook, sspatil, kernel-hardening 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@google.com> Cc: Masahiro Yamada <yamada.masahiro@socionext.com> Cc: James Morris <jmorris@namei.org> Cc: "Serge E. Hallyn" <serge@hallyn.com> Cc: Nick Desaulniers <ndesaulniers@google.com> Cc: Kostya Serebryany <kcc@google.com> Cc: Dmitry Vyukov <dvyukov@google.com> Cc: Kees Cook <keescook@chromium.org> Cc: Sandeep Patil <sspatil@android.com> Cc: linux-security-module@vger.kernel.org Cc: linux-kbuild@vger.kernel.org Cc: kernel-hardening@lists.openwall.com --- 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 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). + +endif # INIT_ALL_MEMORY +endmenu -- 2.21.0.392.gf8f6787159e-goog ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH v3 1/2] initmem: introduce CONFIG_INIT_ALL_MEMORY and CONFIG_INIT_ALL_STACK 2019-04-08 17:04 ` [PATCH v3 1/2] initmem: introduce CONFIG_INIT_ALL_MEMORY and CONFIG_INIT_ALL_STACK Alexander Potapenko @ 2019-04-08 22:15 ` Randy Dunlap 2019-04-09 8:29 ` Alexander Potapenko 2019-04-09 8:37 ` Masahiro Yamada 1 sibling, 1 reply; 12+ messages in thread From: Randy Dunlap @ 2019-04-08 22:15 UTC (permalink / raw) To: Alexander Potapenko, yamada.masahiro, jmorris, serge Cc: linux-security-module, linux-kbuild, ndesaulniers, kcc, dvyukov, keescook, sspatil, kernel-hardening On 4/8/19 10:04 AM, Alexander Potapenko wrote: > 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 This "depends on" is redundant with the "if INIT_ALL_MEMORY" above it. Similar comment for patch 2/2 and INIT_ALL_HEAP. > + 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). > + > +endif # INIT_ALL_MEMORY > +endmenu -- ~Randy ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v3 1/2] initmem: introduce CONFIG_INIT_ALL_MEMORY and CONFIG_INIT_ALL_STACK 2019-04-08 22:15 ` Randy Dunlap @ 2019-04-09 8:29 ` Alexander Potapenko 0 siblings, 0 replies; 12+ messages in thread From: Alexander Potapenko @ 2019-04-09 8:29 UTC (permalink / raw) To: Randy Dunlap Cc: Masahiro Yamada, James Morris, Serge E. Hallyn, linux-security-module, Linux Kbuild mailing list, Nick Desaulniers, Kostya Serebryany, Dmitriy Vyukov, Kees Cook, Sandeep Patil, Kernel Hardening On Tue, Apr 9, 2019 at 12:15 AM Randy Dunlap <rdunlap@infradead.org> wrote: > > On 4/8/19 10:04 AM, Alexander Potapenko wrote: > > 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 > > This "depends on" is redundant with the "if INIT_ALL_MEMORY" above it. Ack, thank you! > Similar comment for patch 2/2 and INIT_ALL_HEAP. > > > + 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). > > + > > +endif # INIT_ALL_MEMORY > > +endmenu > > > -- > ~Randy -- 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 ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v3 1/2] initmem: introduce CONFIG_INIT_ALL_MEMORY and CONFIG_INIT_ALL_STACK 2019-04-08 17:04 ` [PATCH v3 1/2] initmem: introduce CONFIG_INIT_ALL_MEMORY and CONFIG_INIT_ALL_STACK Alexander Potapenko 2019-04-08 22:15 ` Randy Dunlap @ 2019-04-09 8:37 ` Masahiro Yamada 2019-04-09 9:02 ` Alexander Potapenko 2019-04-09 17:06 ` Kees Cook 1 sibling, 2 replies; 12+ messages in thread From: Masahiro Yamada @ 2019-04-09 8:37 UTC (permalink / raw) To: Alexander Potapenko Cc: James Morris, Serge E. Hallyn, linux-security-module, Linux Kbuild mailing list, Nick Desaulniers, Kostya Serebryany, Dmitry Vyukov, Kees Cook, sspatil, Kernel Hardening On Tue, Apr 9, 2019 at 2:04 AM Alexander Potapenko <glider@google.com> 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@google.com> > Cc: Masahiro Yamada <yamada.masahiro@socionext.com> > Cc: James Morris <jmorris@namei.org> > Cc: "Serge E. Hallyn" <serge@hallyn.com> > Cc: Nick Desaulniers <ndesaulniers@google.com> > Cc: Kostya Serebryany <kcc@google.com> > Cc: Dmitry Vyukov <dvyukov@google.com> > Cc: Kees Cook <keescook@chromium.org> > Cc: Sandeep Patil <sspatil@android.com> > Cc: linux-security-module@vger.kernel.org > Cc: linux-kbuild@vger.kernel.org > Cc: kernel-hardening@lists.openwall.com > --- > 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. 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 ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v3 1/2] initmem: introduce CONFIG_INIT_ALL_MEMORY and CONFIG_INIT_ALL_STACK 2019-04-09 8:37 ` Masahiro Yamada @ 2019-04-09 9:02 ` Alexander Potapenko 2019-04-09 9:03 ` Alexander Potapenko 2019-04-09 17:06 ` Kees Cook 1 sibling, 1 reply; 12+ messages in thread From: Alexander Potapenko @ 2019-04-09 9:02 UTC (permalink / raw) To: Masahiro Yamada Cc: James Morris, Serge E. Hallyn, linux-security-module, Linux Kbuild mailing list, Nick Desaulniers, Kostya Serebryany, Dmitry Vyukov, Kees Cook, Sandeep Patil, Kernel Hardening On Tue, Apr 9, 2019 at 10:38 AM Masahiro Yamada <yamada.masahiro@socionext.com> wrote: > > On Tue, Apr 9, 2019 at 2:04 AM Alexander Potapenko <glider@google.com> 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@google.com> > > Cc: Masahiro Yamada <yamada.masahiro@socionext.com> > > Cc: James Morris <jmorris@namei.org> > > Cc: "Serge E. Hallyn" <serge@hallyn.com> > > Cc: Nick Desaulniers <ndesaulniers@google.com> > > Cc: Kostya Serebryany <kcc@google.com> > > Cc: Dmitry Vyukov <dvyukov@google.com> > > Cc: Kees Cook <keescook@chromium.org> > > Cc: Sandeep Patil <sspatil@android.com> > > Cc: linux-security-module@vger.kernel.org > > Cc: linux-kbuild@vger.kernel.org > > Cc: kernel-hardening@lists.openwall.com > > --- > > 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. 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 ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v3 1/2] initmem: introduce CONFIG_INIT_ALL_MEMORY and CONFIG_INIT_ALL_STACK 2019-04-09 9:02 ` Alexander Potapenko @ 2019-04-09 9:03 ` Alexander Potapenko 0 siblings, 0 replies; 12+ messages in thread From: Alexander Potapenko @ 2019-04-09 9:03 UTC (permalink / raw) To: Masahiro Yamada Cc: James Morris, Serge E. Hallyn, linux-security-module, Linux Kbuild mailing list, Nick Desaulniers, Kostya Serebryany, Dmitry Vyukov, Kees Cook, Sandeep Patil, Kernel Hardening On Tue, Apr 9, 2019 at 11:02 AM Alexander Potapenko <glider@google.com> wrote: > > On Tue, Apr 9, 2019 at 10:38 AM Masahiro Yamada > <yamada.masahiro@socionext.com> wrote: > > > > On Tue, Apr 9, 2019 at 2:04 AM Alexander Potapenko <glider@google.com> 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@google.com> > > > Cc: Masahiro Yamada <yamada.masahiro@socionext.com> > > > Cc: James Morris <jmorris@namei.org> > > > Cc: "Serge E. Hallyn" <serge@hallyn.com> > > > Cc: Nick Desaulniers <ndesaulniers@google.com> > > > Cc: Kostya Serebryany <kcc@google.com> > > > Cc: Dmitry Vyukov <dvyukov@google.com> > > > Cc: Kees Cook <keescook@chromium.org> > > > Cc: Sandeep Patil <sspatil@android.com> > > > Cc: linux-security-module@vger.kernel.org > > > Cc: linux-kbuild@vger.kernel.org > > > Cc: kernel-hardening@lists.openwall.com > > > --- > > > 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 ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v3 1/2] initmem: introduce CONFIG_INIT_ALL_MEMORY and CONFIG_INIT_ALL_STACK 2019-04-09 8:37 ` Masahiro Yamada 2019-04-09 9:02 ` Alexander Potapenko @ 2019-04-09 17:06 ` Kees Cook 1 sibling, 0 replies; 12+ messages in thread From: Kees Cook @ 2019-04-09 17:06 UTC (permalink / raw) To: Masahiro Yamada Cc: Alexander Potapenko, James Morris, Serge E. Hallyn, linux-security-module, Linux Kbuild mailing list, Nick Desaulniers, Kostya Serebryany, Dmitry Vyukov, Sandeep Patil, Kernel Hardening On Tue, Apr 9, 2019 at 1:38 AM Masahiro Yamada <yamada.masahiro@socionext.com> wrote: > > +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. Yeah, I think this could use some more cleanup to get a single config that represents that if gcc plugins are available (instead of having it split across two configs now). Additionally I think this needs refactoring against the current menu choices for GCC_PLUGIN_STRUCTLEAK. Let me send a proposed patch in a few hours... -- Kees Cook ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v3 2/2] initmem: introduce CONFIG_INIT_ALL_HEAP 2019-04-08 17:04 [PATCH v3 0/2] RFC: introduce CONFIG_INIT_ALL_MEMORY Alexander Potapenko 2019-04-08 17:04 ` [PATCH v3 1/2] initmem: introduce CONFIG_INIT_ALL_MEMORY and CONFIG_INIT_ALL_STACK Alexander Potapenko @ 2019-04-08 17:04 ` Alexander Potapenko 2019-04-08 17:39 ` Jann Horn 2019-04-09 9:32 ` Mark Rutland 1 sibling, 2 replies; 12+ messages in thread From: Alexander Potapenko @ 2019-04-08 17:04 UTC (permalink / raw) To: yamada.masahiro, jmorris, serge Cc: linux-security-module, linux-kbuild, ndesaulniers, kcc, dvyukov, keescook, sspatil, kernel-hardening This config option enables CONFIG_SLUB_DEBUG and CONFIG_PAGE_POISONING without the need to pass any boot parameters. No performance optimizations are done at the moment to reduce double initialization of memory regions. Signed-off-by: Alexander Potapenko <glider@google.com> Cc: Masahiro Yamada <yamada.masahiro@socionext.com> Cc: James Morris <jmorris@namei.org> Cc: "Serge E. Hallyn" <serge@hallyn.com> Cc: Nick Desaulniers <ndesaulniers@google.com> Cc: Kostya Serebryany <kcc@google.com> Cc: Dmitry Vyukov <dvyukov@google.com> Cc: Kees Cook <keescook@chromium.org> Cc: Sandeep Patil <sspatil@android.com> Cc: linux-security-module@vger.kernel.org Cc: linux-kbuild@vger.kernel.org Cc: kernel-hardening@lists.openwall.com --- v3: - addressed comments by Masahiro Yamada (Kconfig fixes) --- mm/page_poison.c | 5 +++++ mm/slub.c | 2 ++ security/Kconfig.initmem | 11 +++++++++++ 3 files changed, 18 insertions(+) diff --git a/mm/page_poison.c b/mm/page_poison.c index 21d4f97cb49b..a1985f33f635 100644 --- a/mm/page_poison.c +++ b/mm/page_poison.c @@ -12,9 +12,14 @@ static bool want_page_poisoning __read_mostly; static int __init early_page_poison_param(char *buf) { +#ifdef CONFIG_INIT_ALL_HEAP + want_page_poisoning = true; + return 0; +#else if (!buf) return -EINVAL; return strtobool(buf, &want_page_poisoning); +#endif } early_param("page_poison", early_page_poison_param); diff --git a/mm/slub.c b/mm/slub.c index 1b08fbcb7e61..00e0197d3f35 100644 --- a/mm/slub.c +++ b/mm/slub.c @@ -1287,6 +1287,8 @@ static int __init setup_slub_debug(char *str) if (*str == ',') slub_debug_slabs = str + 1; out: + if (IS_ENABLED(CONFIG_INIT_ALL_HEAP)) + slub_debug |= SLAB_POISON; return 1; } diff --git a/security/Kconfig.initmem b/security/Kconfig.initmem index 5e49a55382ad..37cc10a2eeb5 100644 --- a/security/Kconfig.initmem +++ b/security/Kconfig.initmem @@ -13,6 +13,17 @@ config INIT_ALL_MEMORY if INIT_ALL_MEMORY +config INIT_ALL_HEAP + bool "Initialize all heap" + depends on INIT_ALL_MEMORY + select PAGE_POISONING + select PAGE_POISONING_NO_SANITY + select PAGE_POISONING_ZERO + select SLUB_DEBUG if SLUB + default y + help + Enable page poisoning and SLUB poisoning by default. + config INIT_ALL_STACK bool "Initialize all stack" depends on INIT_ALL_MEMORY -- 2.21.0.392.gf8f6787159e-goog ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH v3 2/2] initmem: introduce CONFIG_INIT_ALL_HEAP 2019-04-08 17:04 ` [PATCH v3 2/2] initmem: introduce CONFIG_INIT_ALL_HEAP Alexander Potapenko @ 2019-04-08 17:39 ` Jann Horn 2019-04-09 9:32 ` Mark Rutland 1 sibling, 0 replies; 12+ messages in thread From: Jann Horn @ 2019-04-08 17:39 UTC (permalink / raw) To: Alexander Potapenko Cc: Masahiro Yamada, James Morris, Serge E. Hallyn, linux-security-module, linux-kbuild, Nick Desaulniers, Kostya Serebryany, Dmitry Vyukov, Kees Cook, sspatil, Kernel Hardening, Laura Abbott On Mon, Apr 8, 2019 at 7:20 PM Alexander Potapenko <glider@google.com> wrote: > This config option enables CONFIG_SLUB_DEBUG and CONFIG_PAGE_POISONING > without the need to pass any boot parameters. > > No performance optimizations are done at the moment to reduce double > initialization of memory regions. [...] > diff --git a/mm/page_poison.c b/mm/page_poison.c > index 21d4f97cb49b..a1985f33f635 100644 > --- a/mm/page_poison.c > +++ b/mm/page_poison.c > @@ -12,9 +12,14 @@ static bool want_page_poisoning __read_mostly; > > static int __init early_page_poison_param(char *buf) > { > +#ifdef CONFIG_INIT_ALL_HEAP > + want_page_poisoning = true; > + return 0; > +#else > if (!buf) > return -EINVAL; > return strtobool(buf, &want_page_poisoning); > +#endif > } > early_param("page_poison", early_page_poison_param); > > diff --git a/mm/slub.c b/mm/slub.c > index 1b08fbcb7e61..00e0197d3f35 100644 > --- a/mm/slub.c > +++ b/mm/slub.c > @@ -1287,6 +1287,8 @@ static int __init setup_slub_debug(char *str) > if (*str == ',') > slub_debug_slabs = str + 1; > out: > + if (IS_ENABLED(CONFIG_INIT_ALL_HEAP)) > + slub_debug |= SLAB_POISON; > return 1; > } I don't understand how this is supposed to work. As far as I can tell, the "slub_debug |= SLAB_POISON;" only happens if you actually pass in a "slub_debug" boot parameter? Same thing for "want_page_poisoning = true;". Also, didn't Laura suggest in https://www.openwall.com/lists/kernel-hardening/2019/04/08/4 that a different approach might be more sensible to reduce the performance hit? ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v3 2/2] initmem: introduce CONFIG_INIT_ALL_HEAP 2019-04-08 17:04 ` [PATCH v3 2/2] initmem: introduce CONFIG_INIT_ALL_HEAP Alexander Potapenko 2019-04-08 17:39 ` Jann Horn @ 2019-04-09 9:32 ` Mark Rutland 2019-04-09 9:53 ` Alexander Potapenko 1 sibling, 1 reply; 12+ messages in thread From: Mark Rutland @ 2019-04-09 9:32 UTC (permalink / raw) To: Alexander Potapenko Cc: yamada.masahiro, jmorris, serge, linux-security-module, linux-kbuild, ndesaulniers, kcc, dvyukov, keescook, sspatil, kernel-hardening On Mon, Apr 08, 2019 at 07:04:18PM +0200, Alexander Potapenko wrote: > This config option enables CONFIG_SLUB_DEBUG and CONFIG_PAGE_POISONING > without the need to pass any boot parameters. > > No performance optimizations are done at the moment to reduce double > initialization of memory regions. > > Signed-off-by: Alexander Potapenko <glider@google.com> > Cc: Masahiro Yamada <yamada.masahiro@socionext.com> > Cc: James Morris <jmorris@namei.org> > Cc: "Serge E. Hallyn" <serge@hallyn.com> > Cc: Nick Desaulniers <ndesaulniers@google.com> > Cc: Kostya Serebryany <kcc@google.com> > Cc: Dmitry Vyukov <dvyukov@google.com> > Cc: Kees Cook <keescook@chromium.org> > Cc: Sandeep Patil <sspatil@android.com> > Cc: linux-security-module@vger.kernel.org > Cc: linux-kbuild@vger.kernel.org > Cc: kernel-hardening@lists.openwall.com > --- > v3: > - addressed comments by Masahiro Yamada (Kconfig fixes) > --- > mm/page_poison.c | 5 +++++ > mm/slub.c | 2 ++ > security/Kconfig.initmem | 11 +++++++++++ > 3 files changed, 18 insertions(+) > > diff --git a/mm/page_poison.c b/mm/page_poison.c > index 21d4f97cb49b..a1985f33f635 100644 > --- a/mm/page_poison.c > +++ b/mm/page_poison.c > @@ -12,9 +12,14 @@ static bool want_page_poisoning __read_mostly; > > static int __init early_page_poison_param(char *buf) > { > +#ifdef CONFIG_INIT_ALL_HEAP > + want_page_poisoning = true; > + return 0; > +#else > if (!buf) > return -EINVAL; > return strtobool(buf, &want_page_poisoning); > +#endif > } > early_param("page_poison", early_page_poison_param); IIUC this is only called if page_poison is passed on the command line, so want_page_poisoning won't be set automatically unless that's passed. Presumably you want to initialize it at definition with: static bool want_page_poisoning __read_mostly = IS_ENABLED(CONFIG_INIT_ALL_HEAP); with: #ifndef CONFIG_INIT_ALL_HEAP static int __init early_page_poison_param(char *buf) { ... } early_param("page_poison", early_page_poison_param); #endif ... so that it can't be disabled? Thanks, Mark. ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v3 2/2] initmem: introduce CONFIG_INIT_ALL_HEAP 2019-04-09 9:32 ` Mark Rutland @ 2019-04-09 9:53 ` Alexander Potapenko 0 siblings, 0 replies; 12+ messages in thread From: Alexander Potapenko @ 2019-04-09 9:53 UTC (permalink / raw) To: Mark Rutland Cc: Masahiro Yamada, James Morris, Serge E. Hallyn, linux-security-module, Linux Kbuild mailing list, Nick Desaulniers, Kostya Serebryany, Dmitriy Vyukov, Kees Cook, Sandeep Patil, Kernel Hardening On Tue, Apr 9, 2019 at 11:32 AM Mark Rutland <mark.rutland@arm.com> wrote: > > On Mon, Apr 08, 2019 at 07:04:18PM +0200, Alexander Potapenko wrote: > > This config option enables CONFIG_SLUB_DEBUG and CONFIG_PAGE_POISONING > > without the need to pass any boot parameters. > > > > No performance optimizations are done at the moment to reduce double > > initialization of memory regions. > > > > Signed-off-by: Alexander Potapenko <glider@google.com> > > Cc: Masahiro Yamada <yamada.masahiro@socionext.com> > > Cc: James Morris <jmorris@namei.org> > > Cc: "Serge E. Hallyn" <serge@hallyn.com> > > Cc: Nick Desaulniers <ndesaulniers@google.com> > > Cc: Kostya Serebryany <kcc@google.com> > > Cc: Dmitry Vyukov <dvyukov@google.com> > > Cc: Kees Cook <keescook@chromium.org> > > Cc: Sandeep Patil <sspatil@android.com> > > Cc: linux-security-module@vger.kernel.org > > Cc: linux-kbuild@vger.kernel.org > > Cc: kernel-hardening@lists.openwall.com > > --- > > v3: > > - addressed comments by Masahiro Yamada (Kconfig fixes) > > --- > > mm/page_poison.c | 5 +++++ > > mm/slub.c | 2 ++ > > security/Kconfig.initmem | 11 +++++++++++ > > 3 files changed, 18 insertions(+) > > > > diff --git a/mm/page_poison.c b/mm/page_poison.c > > index 21d4f97cb49b..a1985f33f635 100644 > > --- a/mm/page_poison.c > > +++ b/mm/page_poison.c > > @@ -12,9 +12,14 @@ static bool want_page_poisoning __read_mostly; > > > > static int __init early_page_poison_param(char *buf) > > { > > +#ifdef CONFIG_INIT_ALL_HEAP > > + want_page_poisoning = true; > > + return 0; > > +#else > > if (!buf) > > return -EINVAL; > > return strtobool(buf, &want_page_poisoning); > > +#endif > > } > > early_param("page_poison", early_page_poison_param); > > IIUC this is only called if page_poison is passed on the command line, > so want_page_poisoning won't be set automatically unless that's passed. > > Presumably you want to initialize it at definition with: > > static bool want_page_poisoning __read_mostly = IS_ENABLED(CONFIG_INIT_ALL_HEAP); Yes, I've misunderstood how boot parameters work. Thanks for the suggestions! We'd better look into wiping allocations without POISON_SLAB though. > with: > > #ifndef CONFIG_INIT_ALL_HEAP > static int __init early_page_poison_param(char *buf) > { > ... > } > early_param("page_poison", early_page_poison_param); > #endif > > ... so that it can't be disabled? > > Thanks, > Mark. -- 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 ^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2019-04-09 17:06 UTC | newest] Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-04-08 17:04 [PATCH v3 0/2] RFC: introduce CONFIG_INIT_ALL_MEMORY Alexander Potapenko 2019-04-08 17:04 ` [PATCH v3 1/2] initmem: introduce CONFIG_INIT_ALL_MEMORY and CONFIG_INIT_ALL_STACK Alexander Potapenko 2019-04-08 22:15 ` Randy Dunlap 2019-04-09 8:29 ` Alexander Potapenko 2019-04-09 8:37 ` Masahiro Yamada 2019-04-09 9:02 ` Alexander Potapenko 2019-04-09 9:03 ` Alexander Potapenko 2019-04-09 17:06 ` Kees Cook 2019-04-08 17:04 ` [PATCH v3 2/2] initmem: introduce CONFIG_INIT_ALL_HEAP Alexander Potapenko 2019-04-08 17:39 ` Jann Horn 2019-04-09 9:32 ` Mark Rutland 2019-04-09 9:53 ` Alexander Potapenko
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).