All of lore.kernel.org
 help / color / mirror / Atom feed
From: Hyeonggon Yoo <42.hyeyoo@gmail.com>
To: kernel test robot <lkp@intel.com>
Cc: Vlastimil Babka <vbabka@suse.cz>,
	kbuild-all@lists.01.org, David Rientjes <rientjes@google.com>,
	Christoph Lameter <cl@linux-foundation.org>,
	Joonsoo Kim <iamjoonsoo.kim@lge.com>,
	Pekka Enberg <penberg@kernel.org>,
	Roman Gushchin <roman.gushchin@linux.dev>,
	Andrew Morton <akpm@linux-foundation.org>,
	Linux Memory Management List <linux-mm@kvack.org>,
	patches@lists.linux.dev, linux-kernel@vger.kernel.org,
	Oliver Glitta <glittao@gmail.com>,
	Faiyaz Mohammed <faiyazm@codeaurora.org>,
	Dmitry Vyukov <dvyukov@google.com>,
	Eric Dumazet <edumazet@google.com>,
	Jarkko Sakkinen <jarkko@kernel.org>,
	Johannes Berg <johannes.berg@intel.com>,
	Yury Norov <yury.norov@gmail.com>, Arnd Bergmann <arnd@arndb.de>,
	James Bottomley <James.Bottomley@hansenpartnership.com>,
	Matteo Croce <mcroce@microsoft.com>,
	Marco Elver <elver@google.com>,
	Andrey Konovalov <andreyknvl@gmail.com>,
	Imran Khan <imran.f.khan@oracle.com>,
	Zqiang <qiang.zhang@windriver.com>
Subject: [PATCH v3] mm/slub: initialize stack depot in boot process
Date: Tue, 1 Mar 2022 08:51:56 +0000	[thread overview]
Message-ID: <Yh3erDM9anwFXkD8@ip-172-31-19-208.ap-northeast-1.compute.internal> (raw)
In-Reply-To: <202203011512.U0o5cAx4-lkp@intel.com>

commit ba10d4b46655 ("mm/slub: use stackdepot to save stack trace in
objects") initializes stack depot in cache creation if SLAB_STORE_USER
flag is set.

This can make kernel crash because a cache can be crashed in various
contexts. For example if user sets slub_debug=U, kernel crashes
because create_boot_cache() calls stack_depot_init(), which tries to
allocate hash table using memblock_alloc() if slab is not available.
But memblock is also not available at that time.

This patch solves the problem by initializing stack depot early
in boot process if SLAB_STORE_USER debug flag is set globally
or the flag is set for at least one cache.

[ elver@google.com: initialize stack depot depending on slub_debug
  parameter instead of allowing stack_depot_init() to be called
  during kmem_cache_init() for simplicity. ]

[ vbabka@suse.cz: parse slub_debug parameter in setup_slub_debug()
  and initialize stack depot in stack_depot_early_init(). ]

[ lkp@intel.com: Fix build error. ]

Link: https://lore.kernel.org/all/YhyeaP8lrzKgKm5A@ip-172-31-19-208.ap-northeast-1.compute.internal/
Fixes: ba10d4b46655 ("mm/slub: use stackdepot to save stack trace in objects")
Signed-off-by: Hyeonggon Yoo <42.hyeyoo@gmail.com>
---
 include/linux/slab.h       |  1 +
 include/linux/stackdepot.h |  3 ++-
 mm/slab.c                  |  5 +++++
 mm/slob.c                  |  5 +++++
 mm/slub.c                  | 19 ++++++++++++++++---
 5 files changed, 29 insertions(+), 4 deletions(-)

diff --git a/include/linux/slab.h b/include/linux/slab.h
index 37bde99b74af..d2b0f8f9e5e6 100644
--- a/include/linux/slab.h
+++ b/include/linux/slab.h
@@ -762,6 +762,7 @@ extern void kvfree_sensitive(const void *addr, size_t len);
 
 unsigned int kmem_cache_size(struct kmem_cache *s);
 void __init kmem_cache_init_late(void);
+int __init slab_stack_depot_init(void);
 
 #if defined(CONFIG_SMP) && defined(CONFIG_SLAB)
 int slab_prepare_cpu(unsigned int cpu);
diff --git a/include/linux/stackdepot.h b/include/linux/stackdepot.h
index 17f992fe6355..a813a2673c48 100644
--- a/include/linux/stackdepot.h
+++ b/include/linux/stackdepot.h
@@ -12,6 +12,7 @@
 #define _LINUX_STACKDEPOT_H
 
 #include <linux/gfp.h>
+#include <linux/slab.h>
 
 typedef u32 depot_stack_handle_t;
 
@@ -32,7 +33,7 @@ int stack_depot_init(void);
 #ifdef CONFIG_STACKDEPOT_ALWAYS_INIT
 static inline int stack_depot_early_init(void)	{ return stack_depot_init(); }
 #else
-static inline int stack_depot_early_init(void)	{ return 0; }
+static inline int stack_depot_early_init(void)	{ return slab_stack_depot_init(); }
 #endif
 
 depot_stack_handle_t stack_depot_save(unsigned long *entries,
diff --git a/mm/slab.c b/mm/slab.c
index ddf5737c63d9..c7f929665fbe 100644
--- a/mm/slab.c
+++ b/mm/slab.c
@@ -1196,6 +1196,11 @@ static void __init set_up_node(struct kmem_cache *cachep, int index)
 	}
 }
 
+int __init slab_stack_depot_init(void)
+{
+	return 0;
+}
+
 /*
  * Initialisation.  Called after the page allocator have been initialised and
  * before smp_init().
diff --git a/mm/slob.c b/mm/slob.c
index 60c5842215f1..7597c219f061 100644
--- a/mm/slob.c
+++ b/mm/slob.c
@@ -725,3 +725,8 @@ void __init kmem_cache_init_late(void)
 {
 	slab_state = FULL;
 }
+
+int __init slab_stack_depot_init(void)
+{
+	return 0;
+}
diff --git a/mm/slub.c b/mm/slub.c
index a74afe59a403..8f130f917977 100644
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -646,6 +646,16 @@ static slab_flags_t slub_debug;
 
 static char *slub_debug_string;
 static int disable_higher_order_debug;
+static bool __initdata init_stack_depot;
+
+int __init slab_stack_depot_init(void)
+{
+#ifdef CONFIG_STACKDEPOT
+	if (init_stack_depot)
+		stack_depot_init();
+#endif
+	return 0;
+}
 
 /*
  * slub is about to manipulate internal object metadata.  This memory lies
@@ -1531,6 +1541,8 @@ static int __init setup_slub_debug(char *str)
 			global_slub_debug_changed = true;
 		} else {
 			slab_list_specified = true;
+			if (flags & SLAB_STORE_USER)
+				init_stack_depot = true;
 		}
 	}
 
@@ -1546,6 +1558,10 @@ static int __init setup_slub_debug(char *str)
 			global_flags = slub_debug;
 		slub_debug_string = saved_str;
 	}
+
+	if (global_flags & SLAB_STORE_USER)
+		init_stack_depot = true;
+
 out:
 	slub_debug = global_flags;
 	if (slub_debug != 0 || slub_debug_string)
@@ -4221,9 +4237,6 @@ static int kmem_cache_open(struct kmem_cache *s, slab_flags_t flags)
 	s->remote_node_defrag_ratio = 1000;
 #endif
 
-	if (s->flags & SLAB_STORE_USER && IS_ENABLED(CONFIG_STACKDEPOT))
-		stack_depot_init();
-
 	/* Initialize the pre-computed randomized freelist if slab is up */
 	if (slab_state >= UP) {
 		if (init_cache_random_seq(s))
-- 
2.33.1

WARNING: multiple messages have this Message-ID (diff)
From: Hyeonggon Yoo <42.hyeyoo@gmail.com>
To: kbuild-all@lists.01.org
Subject: [PATCH v3] mm/slub: initialize stack depot in boot process
Date: Tue, 01 Mar 2022 08:51:56 +0000	[thread overview]
Message-ID: <Yh3erDM9anwFXkD8@ip-172-31-19-208.ap-northeast-1.compute.internal> (raw)
In-Reply-To: <202203011512.U0o5cAx4-lkp@intel.com>

[-- Attachment #1: Type: text/plain, Size: 4802 bytes --]

commit ba10d4b46655 ("mm/slub: use stackdepot to save stack trace in
objects") initializes stack depot in cache creation if SLAB_STORE_USER
flag is set.

This can make kernel crash because a cache can be crashed in various
contexts. For example if user sets slub_debug=U, kernel crashes
because create_boot_cache() calls stack_depot_init(), which tries to
allocate hash table using memblock_alloc() if slab is not available.
But memblock is also not available at that time.

This patch solves the problem by initializing stack depot early
in boot process if SLAB_STORE_USER debug flag is set globally
or the flag is set for at least one cache.

[ elver(a)google.com: initialize stack depot depending on slub_debug
  parameter instead of allowing stack_depot_init() to be called
  during kmem_cache_init() for simplicity. ]

[ vbabka(a)suse.cz: parse slub_debug parameter in setup_slub_debug()
  and initialize stack depot in stack_depot_early_init(). ]

[ lkp(a)intel.com: Fix build error. ]

Link: https://lore.kernel.org/all/YhyeaP8lrzKgKm5A(a)ip-172-31-19-208.ap-northeast-1.compute.internal/
Fixes: ba10d4b46655 ("mm/slub: use stackdepot to save stack trace in objects")
Signed-off-by: Hyeonggon Yoo <42.hyeyoo@gmail.com>
---
 include/linux/slab.h       |  1 +
 include/linux/stackdepot.h |  3 ++-
 mm/slab.c                  |  5 +++++
 mm/slob.c                  |  5 +++++
 mm/slub.c                  | 19 ++++++++++++++++---
 5 files changed, 29 insertions(+), 4 deletions(-)

diff --git a/include/linux/slab.h b/include/linux/slab.h
index 37bde99b74af..d2b0f8f9e5e6 100644
--- a/include/linux/slab.h
+++ b/include/linux/slab.h
@@ -762,6 +762,7 @@ extern void kvfree_sensitive(const void *addr, size_t len);
 
 unsigned int kmem_cache_size(struct kmem_cache *s);
 void __init kmem_cache_init_late(void);
+int __init slab_stack_depot_init(void);
 
 #if defined(CONFIG_SMP) && defined(CONFIG_SLAB)
 int slab_prepare_cpu(unsigned int cpu);
diff --git a/include/linux/stackdepot.h b/include/linux/stackdepot.h
index 17f992fe6355..a813a2673c48 100644
--- a/include/linux/stackdepot.h
+++ b/include/linux/stackdepot.h
@@ -12,6 +12,7 @@
 #define _LINUX_STACKDEPOT_H
 
 #include <linux/gfp.h>
+#include <linux/slab.h>
 
 typedef u32 depot_stack_handle_t;
 
@@ -32,7 +33,7 @@ int stack_depot_init(void);
 #ifdef CONFIG_STACKDEPOT_ALWAYS_INIT
 static inline int stack_depot_early_init(void)	{ return stack_depot_init(); }
 #else
-static inline int stack_depot_early_init(void)	{ return 0; }
+static inline int stack_depot_early_init(void)	{ return slab_stack_depot_init(); }
 #endif
 
 depot_stack_handle_t stack_depot_save(unsigned long *entries,
diff --git a/mm/slab.c b/mm/slab.c
index ddf5737c63d9..c7f929665fbe 100644
--- a/mm/slab.c
+++ b/mm/slab.c
@@ -1196,6 +1196,11 @@ static void __init set_up_node(struct kmem_cache *cachep, int index)
 	}
 }
 
+int __init slab_stack_depot_init(void)
+{
+	return 0;
+}
+
 /*
  * Initialisation.  Called after the page allocator have been initialised and
  * before smp_init().
diff --git a/mm/slob.c b/mm/slob.c
index 60c5842215f1..7597c219f061 100644
--- a/mm/slob.c
+++ b/mm/slob.c
@@ -725,3 +725,8 @@ void __init kmem_cache_init_late(void)
 {
 	slab_state = FULL;
 }
+
+int __init slab_stack_depot_init(void)
+{
+	return 0;
+}
diff --git a/mm/slub.c b/mm/slub.c
index a74afe59a403..8f130f917977 100644
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -646,6 +646,16 @@ static slab_flags_t slub_debug;
 
 static char *slub_debug_string;
 static int disable_higher_order_debug;
+static bool __initdata init_stack_depot;
+
+int __init slab_stack_depot_init(void)
+{
+#ifdef CONFIG_STACKDEPOT
+	if (init_stack_depot)
+		stack_depot_init();
+#endif
+	return 0;
+}
 
 /*
  * slub is about to manipulate internal object metadata.  This memory lies
@@ -1531,6 +1541,8 @@ static int __init setup_slub_debug(char *str)
 			global_slub_debug_changed = true;
 		} else {
 			slab_list_specified = true;
+			if (flags & SLAB_STORE_USER)
+				init_stack_depot = true;
 		}
 	}
 
@@ -1546,6 +1558,10 @@ static int __init setup_slub_debug(char *str)
 			global_flags = slub_debug;
 		slub_debug_string = saved_str;
 	}
+
+	if (global_flags & SLAB_STORE_USER)
+		init_stack_depot = true;
+
 out:
 	slub_debug = global_flags;
 	if (slub_debug != 0 || slub_debug_string)
@@ -4221,9 +4237,6 @@ static int kmem_cache_open(struct kmem_cache *s, slab_flags_t flags)
 	s->remote_node_defrag_ratio = 1000;
 #endif
 
-	if (s->flags & SLAB_STORE_USER && IS_ENABLED(CONFIG_STACKDEPOT))
-		stack_depot_init();
-
 	/* Initialize the pre-computed randomized freelist if slab is up */
 	if (slab_state >= UP) {
 		if (init_cache_random_seq(s))
-- 
2.33.1

  reply	other threads:[~2022-03-01  8:52 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-03-01  3:36 [PATCH v2] mm/slub: initialize stack depot in boot process Hyeonggon Yoo
2022-03-01  3:53 ` Hyeonggon Yoo
2022-03-01  7:00 ` kernel test robot
2022-03-01  7:11 ` kernel test robot
2022-03-01  8:51   ` Hyeonggon Yoo [this message]
2022-03-01  8:51     ` [PATCH v3] " Hyeonggon Yoo
2022-03-01  9:14     ` Vlastimil Babka
2022-03-01  9:14       ` Vlastimil Babka
2022-03-01  9:29       ` Hyeonggon Yoo
2022-03-01  9:29         ` Hyeonggon Yoo

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=Yh3erDM9anwFXkD8@ip-172-31-19-208.ap-northeast-1.compute.internal \
    --to=42.hyeyoo@gmail.com \
    --cc=James.Bottomley@hansenpartnership.com \
    --cc=akpm@linux-foundation.org \
    --cc=andreyknvl@gmail.com \
    --cc=arnd@arndb.de \
    --cc=cl@linux-foundation.org \
    --cc=dvyukov@google.com \
    --cc=edumazet@google.com \
    --cc=elver@google.com \
    --cc=faiyazm@codeaurora.org \
    --cc=glittao@gmail.com \
    --cc=iamjoonsoo.kim@lge.com \
    --cc=imran.f.khan@oracle.com \
    --cc=jarkko@kernel.org \
    --cc=johannes.berg@intel.com \
    --cc=kbuild-all@lists.01.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=lkp@intel.com \
    --cc=mcroce@microsoft.com \
    --cc=patches@lists.linux.dev \
    --cc=penberg@kernel.org \
    --cc=qiang.zhang@windriver.com \
    --cc=rientjes@google.com \
    --cc=roman.gushchin@linux.dev \
    --cc=vbabka@suse.cz \
    --cc=yury.norov@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.