| Message ID | 20190529123812.43089-3-glider@google.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v5,1/3] mm: security: introduce init_on_alloc=1 and init_on_free=1 boot options | expand |
On Wed, May 29, 2019 at 02:38:11PM +0200, Alexander Potapenko wrote: > Print the currently enabled stack and heap initialization modes. > > The possible options for stack are: > - "all" for CONFIG_INIT_STACK_ALL; > - "byref_all" for CONFIG_GCC_PLUGIN_STRUCTLEAK_BYREF_ALL; > - "byref" for CONFIG_GCC_PLUGIN_STRUCTLEAK_BYREF; > - "__user" for CONFIG_GCC_PLUGIN_STRUCTLEAK_USER; > - "off" otherwise. > > Depending on the values of init_on_alloc and init_on_free boottime > options we also report "heap alloc" and "heap free" as "on"/"off". > > In the init_on_free mode initializing pages at boot time may take some > time, so print a notice about that as well. > > Signed-off-by: Alexander Potapenko <glider@google.com> > Suggested-by: Kees Cook <keescook@chromium.org> Looks good to me! Acked-by: Kees Cook <keescook@chromium.org> > To: Andrew Morton <akpm@linux-foundation.org> > To: Christoph Lameter <cl@linux.com> > Cc: Dmitry Vyukov <dvyukov@google.com> > Cc: James Morris <jmorris@namei.org> > Cc: Jann Horn <jannh@google.com> > Cc: Kostya Serebryany <kcc@google.com> > Cc: Laura Abbott <labbott@redhat.com> > Cc: Mark Rutland <mark.rutland@arm.com> > Cc: Masahiro Yamada <yamada.masahiro@socionext.com> > Cc: Matthew Wilcox <willy@infradead.org> > Cc: Nick Desaulniers <ndesaulniers@google.com> > Cc: Randy Dunlap <rdunlap@infradead.org> > Cc: Sandeep Patil <sspatil@android.com> > Cc: "Serge E. Hallyn" <serge@hallyn.com> > Cc: Souptick Joarder <jrdr.linux@gmail.com> > Cc: Marco Elver <elver@google.com> > Cc: kernel-hardening@lists.openwall.com > Cc: linux-mm@kvack.org > Cc: linux-security-module@vger.kernel.org > --- > init/main.c | 24 ++++++++++++++++++++++++ > 1 file changed, 24 insertions(+) > > diff --git a/init/main.c b/init/main.c > index 66a196c5e4c3..9d63ff1d48f3 100644 > --- a/init/main.c > +++ b/init/main.c > @@ -520,6 +520,29 @@ static inline void initcall_debug_enable(void) > } > #endif > > +/* Report memory auto-initialization states for this boot. */ > +void __init report_meminit(void) > +{ > + const char *stack; > + > + if (IS_ENABLED(CONFIG_INIT_STACK_ALL)) > + stack = "all"; > + else if (IS_ENABLED(CONFIG_GCC_PLUGIN_STRUCTLEAK_BYREF_ALL)) > + stack = "byref_all"; > + else if (IS_ENABLED(CONFIG_GCC_PLUGIN_STRUCTLEAK_BYREF)) > + stack = "byref"; > + else if (IS_ENABLED(CONFIG_GCC_PLUGIN_STRUCTLEAK_USER)) > + stack = "__user"; > + else > + stack = "off"; > + > + pr_info("mem auto-init: stack:%s, heap alloc:%s, heap free:%s\n", > + stack, want_init_on_alloc(GFP_KERNEL) ? "on" : "off", > + want_init_on_free() ? "on" : "off"); > + if (want_init_on_free()) > + pr_info("Clearing system memory may take some time...\n"); > +} > + > /* > * Set up kernel memory allocators > */ > @@ -530,6 +553,7 @@ static void __init mm_init(void) > * bigger than MAX_ORDER unless SPARSEMEM. > */ > page_ext_init_flatmem(); > + report_meminit(); > mem_init(); > kmem_cache_init(); > pgtable_init(); > -- > 2.22.0.rc1.257.g3120a18244-goog >
On Wed, 29 May 2019 14:38:11 +0200 Alexander Potapenko <glider@google.com> wrote: > Print the currently enabled stack and heap initialization modes. > > The possible options for stack are: > - "all" for CONFIG_INIT_STACK_ALL; > - "byref_all" for CONFIG_GCC_PLUGIN_STRUCTLEAK_BYREF_ALL; > - "byref" for CONFIG_GCC_PLUGIN_STRUCTLEAK_BYREF; > - "__user" for CONFIG_GCC_PLUGIN_STRUCTLEAK_USER; > - "off" otherwise. > > Depending on the values of init_on_alloc and init_on_free boottime > options we also report "heap alloc" and "heap free" as "on"/"off". Why? Please fully describe the benefit to users so that others can judge the desirability of the patch. And so they can review it effectively, etc. Always! > In the init_on_free mode initializing pages at boot time may take some > time, so print a notice about that as well. How much time?
On Sat, Jun 1, 2019 at 3:18 AM Andrew Morton <akpm@linux-foundation.org> wrote: > > On Wed, 29 May 2019 14:38:11 +0200 Alexander Potapenko <glider@google.com> wrote: > > > Print the currently enabled stack and heap initialization modes. > > > > The possible options for stack are: > > - "all" for CONFIG_INIT_STACK_ALL; > > - "byref_all" for CONFIG_GCC_PLUGIN_STRUCTLEAK_BYREF_ALL; > > - "byref" for CONFIG_GCC_PLUGIN_STRUCTLEAK_BYREF; > > - "__user" for CONFIG_GCC_PLUGIN_STRUCTLEAK_USER; > > - "off" otherwise. > > > > Depending on the values of init_on_alloc and init_on_free boottime > > options we also report "heap alloc" and "heap free" as "on"/"off". > > Why? > > Please fully describe the benefit to users so that others can judge the > desirability of the patch. And so they can review it effectively, etc. I'm going to update the description with the following passage: Print the currently enabled stack and heap initialization modes. Stack initialization is enabled by a config flag, while heap initialization is configured at boot time with defaults being set in the config. It's more convenient for the user to have all information about these hardening measures in one place. Does this make sense? > Always! > > > In the init_on_free mode initializing pages at boot time may take some > > time, so print a notice about that as well. > > How much time? I've seen pauses up to 1 second, not actually sure they're worth a separate line in the log. Kees, how long were the delays in your case?
On Mon, Jun 03, 2019 at 11:24:49AM +0200, Alexander Potapenko wrote: > On Sat, Jun 1, 2019 at 3:18 AM Andrew Morton <akpm@linux-foundation.org> wrote: > > > > On Wed, 29 May 2019 14:38:11 +0200 Alexander Potapenko <glider@google.com> wrote: > > > > > Print the currently enabled stack and heap initialization modes. > > > > > > The possible options for stack are: > > > - "all" for CONFIG_INIT_STACK_ALL; > > > - "byref_all" for CONFIG_GCC_PLUGIN_STRUCTLEAK_BYREF_ALL; > > > - "byref" for CONFIG_GCC_PLUGIN_STRUCTLEAK_BYREF; > > > - "__user" for CONFIG_GCC_PLUGIN_STRUCTLEAK_USER; > > > - "off" otherwise. > > > > > > Depending on the values of init_on_alloc and init_on_free boottime > > > options we also report "heap alloc" and "heap free" as "on"/"off". > > > > Why? > > > > Please fully describe the benefit to users so that others can judge the > > desirability of the patch. And so they can review it effectively, etc. > I'm going to update the description with the following passage: > > Print the currently enabled stack and heap initialization modes. > > Stack initialization is enabled by a config flag, while heap > initialization is configured at boot time with defaults being set > in the config. It's more convenient for the user to have all information > about these hardening measures in one place. > > Does this make sense? > > Always! > > > > > In the init_on_free mode initializing pages at boot time may take some > > > time, so print a notice about that as well. > > > > How much time? > I've seen pauses up to 1 second, not actually sure they're worth a > separate line in the log. > Kees, how long were the delays in your case? I didn't measure it, but I think it was something like 0.5 second per GB. I noticed because normally boot flashes by. With init_on_free it pauses for no apparent reason, which is why I suggested the note. (I mean *I* knew why it was pausing, but it might surprise someone who sets init_on_free=1 without really thinking about what's about to happen at boot.)
On Tue, Jun 4, 2019 at 8:44 AM Kees Cook <keescook@chromium.org> wrote: > > On Mon, Jun 03, 2019 at 11:24:49AM +0200, Alexander Potapenko wrote: > > On Sat, Jun 1, 2019 at 3:18 AM Andrew Morton <akpm@linux-foundation.org> wrote: > > > > > > On Wed, 29 May 2019 14:38:11 +0200 Alexander Potapenko <glider@google.com> wrote: > > > > > > > Print the currently enabled stack and heap initialization modes. > > > > > > > > The possible options for stack are: > > > > - "all" for CONFIG_INIT_STACK_ALL; > > > > - "byref_all" for CONFIG_GCC_PLUGIN_STRUCTLEAK_BYREF_ALL; > > > > - "byref" for CONFIG_GCC_PLUGIN_STRUCTLEAK_BYREF; > > > > - "__user" for CONFIG_GCC_PLUGIN_STRUCTLEAK_USER; > > > > - "off" otherwise. > > > > > > > > Depending on the values of init_on_alloc and init_on_free boottime > > > > options we also report "heap alloc" and "heap free" as "on"/"off". > > > > > > Why? > > > > > > Please fully describe the benefit to users so that others can judge the > > > desirability of the patch. And so they can review it effectively, etc. > > I'm going to update the description with the following passage: > > > > Print the currently enabled stack and heap initialization modes. > > > > Stack initialization is enabled by a config flag, while heap > > initialization is configured at boot time with defaults being set > > in the config. It's more convenient for the user to have all information > > about these hardening measures in one place. > > > > Does this make sense? > > > Always! > > > > > > > In the init_on_free mode initializing pages at boot time may take some > > > > time, so print a notice about that as well. > > > > > > How much time? > > I've seen pauses up to 1 second, not actually sure they're worth a > > separate line in the log. > > Kees, how long were the delays in your case? > > I didn't measure it, but I think it was something like 0.5 second per GB. > I noticed because normally boot flashes by. With init_on_free it pauses > for no apparent reason, which is why I suggested the note. (I mean *I* > knew why it was pausing, but it might surprise someone who sets > init_on_free=1 without really thinking about what's about to happen at > boot.) (Pardon the gmail client) How about: - if (want_init_on_free()) - pr_info("Clearing system memory may take some time...\n"); + if (want_init_on_free()) + pr_info("meminit: clearing system memory may take some time...\n"); or even + if (want_init_on_free()) + pr_info("meminit (init_on_free == 1): clearing system memory may take some time...\n"); or some combo thereof? -- Kaiwan > > -- > Kees Cook >
On Tue, Jun 4, 2019 at 8:01 AM Kaiwan N Billimoria <kaiwan@kaiwantech.com> wrote: > > On Tue, Jun 4, 2019 at 8:44 AM Kees Cook <keescook@chromium.org> wrote: > > > > On Mon, Jun 03, 2019 at 11:24:49AM +0200, Alexander Potapenko wrote: > > > On Sat, Jun 1, 2019 at 3:18 AM Andrew Morton <akpm@linux-foundation.org> wrote: > > > > > > > > On Wed, 29 May 2019 14:38:11 +0200 Alexander Potapenko <glider@google.com> wrote: > > > > > > > > > Print the currently enabled stack and heap initialization modes. > > > > > > > > > > The possible options for stack are: > > > > > - "all" for CONFIG_INIT_STACK_ALL; > > > > > - "byref_all" for CONFIG_GCC_PLUGIN_STRUCTLEAK_BYREF_ALL; > > > > > - "byref" for CONFIG_GCC_PLUGIN_STRUCTLEAK_BYREF; > > > > > - "__user" for CONFIG_GCC_PLUGIN_STRUCTLEAK_USER; > > > > > - "off" otherwise. > > > > > > > > > > Depending on the values of init_on_alloc and init_on_free boottime > > > > > options we also report "heap alloc" and "heap free" as "on"/"off". > > > > > > > > Why? > > > > > > > > Please fully describe the benefit to users so that others can judge the > > > > desirability of the patch. And so they can review it effectively, etc. > > > I'm going to update the description with the following passage: > > > > > > Print the currently enabled stack and heap initialization modes. > > > > > > Stack initialization is enabled by a config flag, while heap > > > initialization is configured at boot time with defaults being set > > > in the config. It's more convenient for the user to have all information > > > about these hardening measures in one place. > > > > > > Does this make sense? > > > > Always! > > > > > > > > > In the init_on_free mode initializing pages at boot time may take some > > > > > time, so print a notice about that as well. > > > > > > > > How much time? > > > I've seen pauses up to 1 second, not actually sure they're worth a > > > separate line in the log. > > > Kees, how long were the delays in your case? > > > > I didn't measure it, but I think it was something like 0.5 second per GB. > > I noticed because normally boot flashes by. With init_on_free it pauses > > for no apparent reason, which is why I suggested the note. (I mean *I* > > knew why it was pausing, but it might surprise someone who sets > > init_on_free=1 without really thinking about what's about to happen at > > boot.) > > (Pardon the gmail client) > How about: > - if (want_init_on_free()) > - pr_info("Clearing system memory may take some time...\n"); > + if (want_init_on_free()) > + pr_info("meminit: clearing system memory may take some > time...\n"); Yes, adding a prefix may give the users better understanding of who's clearing the memory. We should stick to the same prefix as before though, i.e. "mem auto-init" > > or even > > + if (want_init_on_free()) > + pr_info("meminit (init_on_free == 1): clearing system > memory may take some time...\n"); > > or some combo thereof? > > -- > Kaiwan > > > > -- > > Kees Cook > >
On Tue, Jun 4, 2019 at 8:36 PM Alexander Potapenko <glider@google.com> wrote: > > On Tue, Jun 4, 2019 at 8:01 AM Kaiwan N Billimoria > <kaiwan@kaiwantech.com> wrote: > > > > On Tue, Jun 4, 2019 at 8:44 AM Kees Cook <keescook@chromium.org> wrote: > > > > > > On Mon, Jun 03, 2019 at 11:24:49AM +0200, Alexander Potapenko wrote: > > > > On Sat, Jun 1, 2019 at 3:18 AM Andrew Morton <akpm@linux-foundation.org> wrote: > > > > > > > > > > On Wed, 29 May 2019 14:38:11 +0200 Alexander Potapenko <glider@google.com> wrote: > > > > > > > > > > > Print the currently enabled stack and heap initialization modes. > > > > > > > > > > > > The possible options for stack are: > > > > > > - "all" for CONFIG_INIT_STACK_ALL; > > > > > > - "byref_all" for CONFIG_GCC_PLUGIN_STRUCTLEAK_BYREF_ALL; > > > > > > - "byref" for CONFIG_GCC_PLUGIN_STRUCTLEAK_BYREF; > > > > > > - "__user" for CONFIG_GCC_PLUGIN_STRUCTLEAK_USER; > > > > > > - "off" otherwise. > > > > > > > > > > > > Depending on the values of init_on_alloc and init_on_free boottime > > > > > > options we also report "heap alloc" and "heap free" as "on"/"off". > > > > > > > > > > Why? > > > > > > > > > > Please fully describe the benefit to users so that others can judge the > > > > > desirability of the patch. And so they can review it effectively, etc. > > > > I'm going to update the description with the following passage: > > > > > > > > Print the currently enabled stack and heap initialization modes. > > > > > > > > Stack initialization is enabled by a config flag, while heap > > > > initialization is configured at boot time with defaults being set > > > > in the config. It's more convenient for the user to have all information > > > > about these hardening measures in one place. > > > > > > > > Does this make sense? > > > > > Always! > > > > > > > > > > > In the init_on_free mode initializing pages at boot time may take some > > > > > > time, so print a notice about that as well. > > > > > > > > > > How much time? > > > > I've seen pauses up to 1 second, not actually sure they're worth a > > > > separate line in the log. > > > > Kees, how long were the delays in your case? > > > > > > I didn't measure it, but I think it was something like 0.5 second per GB. > > > I noticed because normally boot flashes by. With init_on_free it pauses > > > for no apparent reason, which is why I suggested the note. (I mean *I* > > > knew why it was pausing, but it might surprise someone who sets > > > init_on_free=1 without really thinking about what's about to happen at > > > boot.) > > > > (Pardon the gmail client) > > How about: > > - if (want_init_on_free()) > > - pr_info("Clearing system memory may take some time...\n"); > > + if (want_init_on_free()) > > + pr_info("meminit: clearing system memory may take some > > time...\n"); > Yes, adding a prefix may give the users better understanding of who's > clearing the memory. > We should stick to the same prefix as before though, i.e. "mem auto-init" True, agreed. -- Kaiwan
diff --git a/init/main.c b/init/main.c index 66a196c5e4c3..9d63ff1d48f3 100644 --- a/init/main.c +++ b/init/main.c @@ -520,6 +520,29 @@ static inline void initcall_debug_enable(void) } #endif +/* Report memory auto-initialization states for this boot. */ +void __init report_meminit(void) +{ + const char *stack; + + if (IS_ENABLED(CONFIG_INIT_STACK_ALL)) + stack = "all"; + else if (IS_ENABLED(CONFIG_GCC_PLUGIN_STRUCTLEAK_BYREF_ALL)) + stack = "byref_all"; + else if (IS_ENABLED(CONFIG_GCC_PLUGIN_STRUCTLEAK_BYREF)) + stack = "byref"; + else if (IS_ENABLED(CONFIG_GCC_PLUGIN_STRUCTLEAK_USER)) + stack = "__user"; + else + stack = "off"; + + pr_info("mem auto-init: stack:%s, heap alloc:%s, heap free:%s\n", + stack, want_init_on_alloc(GFP_KERNEL) ? "on" : "off", + want_init_on_free() ? "on" : "off"); + if (want_init_on_free()) + pr_info("Clearing system memory may take some time...\n"); +} + /* * Set up kernel memory allocators */ @@ -530,6 +553,7 @@ static void __init mm_init(void) * bigger than MAX_ORDER unless SPARSEMEM. */ page_ext_init_flatmem(); + report_meminit(); mem_init(); kmem_cache_init(); pgtable_init();
Print the currently enabled stack and heap initialization modes. The possible options for stack are: - "all" for CONFIG_INIT_STACK_ALL; - "byref_all" for CONFIG_GCC_PLUGIN_STRUCTLEAK_BYREF_ALL; - "byref" for CONFIG_GCC_PLUGIN_STRUCTLEAK_BYREF; - "__user" for CONFIG_GCC_PLUGIN_STRUCTLEAK_USER; - "off" otherwise. Depending on the values of init_on_alloc and init_on_free boottime options we also report "heap alloc" and "heap free" as "on"/"off". In the init_on_free mode initializing pages at boot time may take some time, so print a notice about that as well. Signed-off-by: Alexander Potapenko <glider@google.com> Suggested-by: Kees Cook <keescook@chromium.org> To: Andrew Morton <akpm@linux-foundation.org> To: Christoph Lameter <cl@linux.com> Cc: Dmitry Vyukov <dvyukov@google.com> Cc: James Morris <jmorris@namei.org> Cc: Jann Horn <jannh@google.com> Cc: Kostya Serebryany <kcc@google.com> Cc: Laura Abbott <labbott@redhat.com> Cc: Mark Rutland <mark.rutland@arm.com> Cc: Masahiro Yamada <yamada.masahiro@socionext.com> Cc: Matthew Wilcox <willy@infradead.org> Cc: Nick Desaulniers <ndesaulniers@google.com> Cc: Randy Dunlap <rdunlap@infradead.org> Cc: Sandeep Patil <sspatil@android.com> Cc: "Serge E. Hallyn" <serge@hallyn.com> Cc: Souptick Joarder <jrdr.linux@gmail.com> Cc: Marco Elver <elver@google.com> Cc: kernel-hardening@lists.openwall.com Cc: linux-mm@kvack.org Cc: linux-security-module@vger.kernel.org --- init/main.c | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+)