| Message ID | 20190408170418.148554-3-glider@google.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | RFC: introduce CONFIG_INIT_ALL_MEMORY | expand |
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?
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.
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.
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
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(+)