[v5,2/3] mm: init: report memory auto-initialization features at boot time
diff mbox series

Message ID 20190529123812.43089-3-glider@google.com
State New
Headers show
Series
  • [v5,1/3] mm: security: introduce init_on_alloc=1 and init_on_free=1 boot options
Related show

Commit Message

Alexander Potapenko May 29, 2019, 12:38 p.m. UTC
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(+)

Comments

Kees Cook May 29, 2019, 6:43 p.m. UTC | #1
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
>
Andrew Morton June 1, 2019, 1:18 a.m. UTC | #2
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?
Alexander Potapenko June 3, 2019, 9:24 a.m. UTC | #3
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?
Kees Cook June 4, 2019, 3:14 a.m. UTC | #4
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.)
Kaiwan N Billimoria June 4, 2019, 6:01 a.m. UTC | #5
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
>
Alexander Potapenko June 4, 2019, 3:06 p.m. UTC | #6
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
> >
Kaiwan N Billimoria June 5, 2019, 2:58 a.m. UTC | #7
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

Patch
diff mbox series

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();