mm/slab: fix unbounded recursion in free path with memalloc profiling

This is a follow-up fix after the recent discussion [1]. See patch 2 for the detailed description on the bug.

Based on slab/for-next-fixes (72bb229f916) and will be soon available at git.kernel.org [2].

Instead preventing cycles by bumping up the allocation size of obj_exts arrays, it introduces a new kmalloc type called KMALLOC_NO_OBJ_EXT and disallow formation of cycles between kmalloc types when allocating obj_exts arrays. obj_exts arrays of normal kmalloc caches are served from KMALLOC_NO_OBJ_EXT caches (that don't have obj_exts), and all other obj_exts arrays are served from normal kmalloc caches.

I tried to reuse SLAB_ALLOC_NO_RECURSE to make kmalloc_slab() select KMALLOC_NO_OBJ_EXT, but it was not great because it does not allow sheaves for those caches. So I introduced a new slab alloc flag SLAB_ALLOC_NO_OBJ_EXT.

To avoid huge confusion, I had to decouple "disallowing sheaves" semantics from SLAB_NO_OBJ_EXT and introduced SLAB_NO_SHEAVES.

While this cannot be directly backported to v6.18 and v6.12 due to lack of SLAB_ALLOC_* flags and kmalloc_flags(), I don't this will be particularily challenging to backport it. Instead of a new slab alloc flag, we can use __GFP_NO_OBJ_EXT to select KMALLOC_NO_OBJ_EXT as kmalloc caches don't have sheaves in v6.18 anyway.

[1] https://lore.kernel.org/linux-mm/[email protected]

[2] https://git.kernel.org/pub/scm/linux/kernel/git/harry/linux.git/log/?h=kmalloc-no-objext-v2r2

[ "v2r2" because after exposing v2r1 I adjusted some of Vlastimil's
feedback and fixed a memory leak due to bootstrapping twice when kmalloc caches are aliased ]

---
Changes in v2:
- Fix a memory leak when kmalloc caches are aliased (patch 1). This is included as part of this series as patch 3 makes it easier to trigger the leak.

- Fix a warning in MEM_ALLOC_PROFILING_DEBUG by dropping mark_obj_codetag_empty() for obj_exts. Sashiko raised this and I reproduced it and confirmed that the warnings are gone.
https://sashiko.dev/#/patchset/20260702-kmalloc-no-objext-v1-0-167175008538%40kernel.org

- Added R-b from Vlastimil on patch 2, thanks!
- Adjusted Vlastimil's feedback on RFC v1
- Link to v1: https://lore.kernel.org/r/[email protected]

---
Harry Yoo (Oracle) (3):
mm/slab: fix a memory leak due to bootstrapping sheaves twice mm/slab: decouple SLAB_NO_SHEAVES from SLAB_NO_OBJ_EXT mm/slab: prevent unbounded recursion in free path with new kmalloc type

 include/linux/slab.h |  16 +++++++-
 mm/slab.h            |  15 +++++--
 mm/slab_common.c     |  18 ++++++++-
 mm/slub.c            | 109 +++++++++++++++++++++------------------------------
 4 files changed, 88 insertions(+), 70 deletions(-)

---
base-commit: 72bb229f9161a1efcd5df32141b69fcc6ae81a13
change-id: 20260702-kmalloc-no-objext-2619c1e06083

Best regards,

Harry Yoo (Oracle) <[email protected]>

When kmalloc caches are aliased, multiple cache pointers reference the same kmem_cache. As a result, iterating over kmalloc indices and bootstrapping sheaves can bootstrap the same cache more than once and leak memory.

Currently, this could happen when the architecture specifies minimum alignment for slab caches that is larger than ARCH_KMALLOC_MINALIGN.

Bootstrap sheaves only when the cache does not have them already. Add a warning when bootstrap_cache_sheaves() is called for a cache that already has sheaves enabled.

---
 mm/slub.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

index 65febe957886..f9461a0c47d3 100644
@@ -8497,6 +8497,8 @@ static void __init bootstrap_cache_sheaves(struct kmem_cache *s)
 	bool failed = false;
 	int node, cpu;
 
+	VM_WARN_ON_ONCE(cache_has_sheaves(s));
+
 	capacity = calculate_sheaf_capacity(s, &empty_args);
 
 	/* capacity can be 0 due to debugging or SLUB_TINY */

@@ -8548,8 +8550,11 @@ static void __init bootstrap_kmalloc_sheaves(void)
 
 	for (type = KMALLOC_NORMAL; type <= KMALLOC_PARTITION_END; type++) {
 		for (int idx = 0; idx < KMALLOC_SHIFT_HIGH + 1; idx++) {
-			if (kmalloc_caches[type][idx])
-				bootstrap_cache_sheaves(kmalloc_caches[type][idx]);
+			struct kmem_cache *s = kmalloc_caches[type][idx];
+
+			/* Do not bootstrap twice when caches are aliased */
+			if (s && !cache_has_sheaves(s))
+				bootstrap_cache_sheaves(s);
 		}
 	}
 }

2.53.0

Bootstrap caches are created with SLAB_NO_OBJ_EXT to disallow sheaves and obj_exts.

To allow disabling obj_exts while allowing sheaves, decouple SLAB_NO_SHEAVES from SLAB_NO_OBJ_EXT. Bootstrap caches now have both SLAB_NO_SHEAVES and SLAB_NO_OBJ_EXT.

No functional change intended.

---
 include/linux/slab.h | 13 +++++++++++--
 mm/slub.c            | 10 ++++++----
 2 files changed, 17 insertions(+), 6 deletions(-)

index 51f03f18c9a7..08d7b6c9c4d6 100644
@@ -58,10 +58,13 @@ enum _slab_flag_bits {
 #endif
 	_SLAB_OBJECT_POISON,
 	_SLAB_CMPXCHG_DOUBLE,
+#ifdef CONFIG_SLAB_OBJ_EXT
 	_SLAB_NO_OBJ_EXT,
-#if defined(CONFIG_SLAB_OBJ_EXT) && defined(CONFIG_64BIT)
+#ifdef CONFIG_64BIT
 	_SLAB_OBJ_EXT_IN_OBJ,
 #endif
+#endif
+	_SLAB_NO_SHEAVES,
 	_SLAB_FLAGS_LAST_BIT
 };
 

@@ -239,8 +242,14 @@ enum _slab_flag_bits {
 #endif
 #define SLAB_TEMPORARY		SLAB_RECLAIM_ACCOUNT	/* Objects are short-lived */
 
-/* Slab created using create_boot_cache */
+/* Slab caches without obj_exts array */
+#ifdef CONFIG_SLAB_OBJ_EXT
 #define SLAB_NO_OBJ_EXT		__SLAB_FLAG_BIT(_SLAB_NO_OBJ_EXT)
+#else
+#define SLAB_NO_OBJ_EXT		__SLAB_FLAG_UNUSED
+#endif
+
+#define SLAB_NO_SHEAVES		__SLAB_FLAG_BIT(_SLAB_NO_SHEAVES)
 
 #if defined(CONFIG_SLAB_OBJ_EXT) && defined(CONFIG_64BIT)
 #define SLAB_OBJ_EXT_IN_OBJ	__SLAB_FLAG_BIT(_SLAB_OBJ_EXT_IN_OBJ)

index f9461a0c47d3..abe748b7dddb 100644
@@ -7799,12 +7799,12 @@ static unsigned int calculate_sheaf_capacity(struct kmem_cache *s,
 		return 0;
 
 	/*
-	 * Bootstrap caches can't have sheaves for now (SLAB_NO_OBJ_EXT).
+	 * Bootstrap caches can't have sheaves for now (SLAB_NO_SHEAVES).
 	 * SLAB_NOLEAKTRACE caches (e.g., kmemleak's object_cache) must not
 	 * have sheaves to avoid recursion when sheaf allocation triggers
 	 * kmemleak tracking.
 	 */
-	if (s->flags & (SLAB_NO_OBJ_EXT | SLAB_NOLEAKTRACE))
+	if (s->flags & (SLAB_NO_SHEAVES | SLAB_NOLEAKTRACE))
 		return 0;
 
 	/*

@@ -8586,7 +8586,8 @@ void __init kmem_cache_init(void)
 
 	create_boot_cache(kmem_cache_node, "kmem_cache_node",
 			sizeof(struct kmem_cache_node),
-			SLAB_HWCACHE_ALIGN | SLAB_NO_OBJ_EXT, 0, 0);
+			SLAB_HWCACHE_ALIGN | SLAB_NO_SHEAVES | SLAB_NO_OBJ_EXT,
+			0, 0);
 
 	hotplug_node_notifier(slab_memory_callback, SLAB_CALLBACK_PRI);
 

@@ -8596,7 +8597,8 @@ void __init kmem_cache_init(void)
 	create_boot_cache(kmem_cache, "kmem_cache",
 			offsetof(struct kmem_cache, per_node) +
 				nr_node_ids * sizeof(struct kmem_cache_per_node_ptrs),
-			SLAB_HWCACHE_ALIGN | SLAB_NO_OBJ_EXT, 0, 0);
+			SLAB_HWCACHE_ALIGN | SLAB_NO_SHEAVES | SLAB_NO_OBJ_EXT,
+			0, 0);
 
 	kmem_cache = bootstrap(&boot_kmem_cache);
 	kmem_cache_node = bootstrap(&boot_kmem_cache_node);

2.53.0

Commit 280ea9c3154b ("mm/slab: avoid allocating slabobj_ext array from its own slab") avoided recursive allocation of obj_exts from kmalloc caches of the same size, by bumping the obj_exts array's allocation size whenever the array size equals the size of the object being allocated.

However, as reported by Danielle Costantino and Shakeel Butt, even slabs from kmalloc caches of different sizes can form a cycle by allocating obj_exts arrays from each other [1]:

What happened: a KMALLOC_NORMAL slab's obj_exts array (used by allocation profiling / memcg accounting) is itself kmalloc()'d from a KMALLOC_NORMAL cache, so the "slab holds another slab's obj_exts array" relation can form cycles. With sizeof(struct slabobj_ext) == 16 and the host's geometry:

- kmalloc-512 has 64 objects/slab -> array is 64*16 == 1024 bytes,

    served from kmalloc-1k;

- kmalloc-1k has 32 objects/slab -> array is 32*16 == 512 bytes, served from kmalloc-512.

A kmalloc-512 slab and a kmalloc-1k slab therefore hold each other's obj_exts array. Discarding one frees the other's array, which empties and discards that slab, which frees the first's array, and so on:
__free_slab() -> free_slab_obj_exts() -> kfree() -> discard_slab() -> __free_slab() recurses along the cycle until the stack is exhausted.

With memory allocation profiling, this allows unbounded recursion in the free path and led to a stack overflow on a production host in the Meta fleet [1]:

  BUG: TASK stack guard page was hit
  Oops: stack guard page
  RIP: 0010:kfree+0x8/0x5d0
  Call Trace:
   __free_slab+0x66/0xc0
   kfree+0x3f0/0x5d0
   ... ( ~125x __free_slab <-> kfree ) ...
   <kernel driver freeing a resource>
   do_syscall_64

It is proposed [1] to resolve this issue by always serving the obj_exts array allocation from kmalloc caches (or large kmalloc) of sizes larger than the object size. However, as pointed out by Vlastimil Babka [2], this can waste an excessive amount of memory as slabs from large kmalloc sizes (e.g. kmalloc-8k) generally need obj_exts arrays much smaller than the object size.

Therefore, rather than bumping the size, let us take a different approach; disallow formation of cycles between kmalloc types when allocating obj_exts arrays. Currently, all obj_exts arrays are served from normal kmalloc caches. Cycles cannot be created if obj_exts arrays of normal kmalloc caches are served from a special kmalloc type that can never have obj_exts arrays.

To achieve this, create a new kmalloc type called KMALLOC_NO_OBJ_EXT. KMALLOC_NO_OBJ_EXT caches are created when CONFIG_SLAB_OBJ_EXT is enabled, and they have SLAB_NO_OBJ_EXT flag to prevent allocation of obj_exts arrays. They remain unused until allocation of obj_exts arrays for normal kmalloc caches happens.

Sheaf bootstrapping for KMALLOC_NO_OBJ_EXT caches now must be deferred because allocation of a barn can trigger obj_exts array allocation of normal kmalloc caches when the KMALLOC_NO_OBJ_EXT cache for that size is not ready yet. For simplicity, perform bootstrapping of sheaves for all kmalloc caches later.

Introduce a new slab alloc flag, SLAB_ALLOC_NO_OBJ_EXT, to prevent allocation of obj_exts arrays, and let kmalloc_slab() override the type to KMALLOC_NO_OBJ_EXT when specified. Note that kmalloc_type() remains unchanged because kmalloc_flags() bypasses the kmalloc fastpath.

Do not pass SLAB_ALLOC_NO_RECURSE to kmalloc_flags() in alloc_slab_obj_exts() and instead use SLAB_ALLOC_NO_OBJ_EXT only when the objects are allocated from normal kmalloc caches. While this prevents unbounded recursive allocation of obj_exts, it allows KMALLOC_NO_OBJ_EXT caches to have sheaves.

Since sheaf allocations specify SLAB_ALLOC_NO_RECURSE that prevents allocation of both sheaves and obj_exts arrays, the recursion depth is bounded.

obj_exts arrays for non-kmalloc-normal caches can now have a valid tag. Do not call mark_obj_codetag_empty() when freeing an obj_exts array to avoid false warnings. KMALLOC_NO_OBJ_EXT don't need this as they never allocate those arrays.

---
 include/linux/slab.h |  3 ++
 mm/slab.h            | 15 +++++++--
 mm/slab_common.c     | 18 ++++++++++-
 mm/slub.c            | 90 +++++++++++++++++++---------------------------------
 4 files changed, 64 insertions(+), 62 deletions(-)

index 08d7b6c9c4d6..0c1d13773523 100644
@@ -721,6 +721,9 @@ enum kmalloc_cache_type {
 #endif
 #ifdef CONFIG_MEMCG
 	KMALLOC_CGROUP,
+#endif
+#ifdef CONFIG_SLAB_OBJ_EXT
+	KMALLOC_NO_OBJ_EXT,
 #endif
 	NR_KMALLOC_TYPES
 };

index 281a65233795..7bce162699b7 100644
@@ -22,6 +22,7 @@
 #define SLAB_ALLOC_NOLOCK	0x01 /* a kmalloc_nolock() allocation */
 #define SLAB_ALLOC_NEW_SLAB	0x02 /* a flag for alloc_slab_obj_exts() */
 #define SLAB_ALLOC_NO_RECURSE	0x04 /* prevent kmalloc() recursion */
+#define SLAB_ALLOC_NO_OBJ_EXT	0x08 /* prevent obj_exts array allocation */
 
 static inline bool alloc_flags_allow_spinning(const unsigned int alloc_flags)
 {

@@ -386,12 +387,19 @@ static inline unsigned int size_index_elem(unsigned int bytes)
  * KMALLOC_MAX_CACHE_SIZE and the caller must check that.
  */
 static inline struct kmem_cache *
-kmalloc_slab(size_t size, kmem_buckets *b, gfp_t flags, kmalloc_token_t token)
+kmalloc_slab(size_t size, kmem_buckets *b, gfp_t flags, kmalloc_token_t token,
+	     unsigned int alloc_flags)
 {
 	unsigned int index;
+	enum kmalloc_cache_type type = kmalloc_type(flags, token);
+
+#ifdef CONFIG_SLAB_OBJ_EXT
+	if (alloc_flags & SLAB_ALLOC_NO_OBJ_EXT)
+		type = KMALLOC_NO_OBJ_EXT;
+#endif
 
 	if (!b)
-		b = &kmalloc_caches[kmalloc_type(flags, token)];
+		b = &kmalloc_caches[type];
 	if (size <= 192)
 		index = kmalloc_size_index[size_index_elem(size)];
 	else

@@ -426,7 +434,8 @@ static inline bool is_kmalloc_normal(struct kmem_cache *s)
 {
 	if (!is_kmalloc_cache(s))
 		return false;
-	return !(s->flags & (SLAB_CACHE_DMA|SLAB_ACCOUNT|SLAB_RECLAIM_ACCOUNT));
+
+	return !(s->flags & (SLAB_CACHE_DMA|SLAB_ACCOUNT|SLAB_RECLAIM_ACCOUNT|SLAB_NO_OBJ_EXT));
 }
 
 bool __kfree_rcu_sheaf(struct kmem_cache *s, void *obj);

index b6426d7ceec9..7f262134d0f2 100644
@@ -783,11 +783,15 @@ u8 kmalloc_size_index[24] __ro_after_init = {
 size_t kmalloc_size_roundup(size_t size)
 {
 	if (size && size <= KMALLOC_MAX_CACHE_SIZE) {
+		struct kmem_cache *s;
+
 		/*
 		 * The flags don't matter since size_index is common to all.
 		 * Neither does the caller for just getting ->object_size.
 		 */
-		return kmalloc_slab(size, NULL, GFP_KERNEL, __kmalloc_token(0))->object_size;
+		s = kmalloc_slab(size, NULL, GFP_KERNEL, __kmalloc_token(0),
+				 SLAB_ALLOC_DEFAULT);
+		return s->object_size;
 	}
 
 	/* Above the smaller buckets, size is a multiple of page size. */

@@ -843,6 +847,12 @@ EXPORT_SYMBOL(kmalloc_size_roundup);
 #define KMALLOC_PARTITION_NAME(N, sz)
 #endif
 
+#ifdef CONFIG_SLAB_OBJ_EXT
+#define KMALLOC_NO_OBJ_EXT_NAME(sz) .name[KMALLOC_NO_OBJ_EXT] = "kmalloc-no-objext-" #sz,
+#else
+#define KMALLOC_NO_OBJ_EXT_NAME(sz)
+#endif
+
 #define INIT_KMALLOC_INFO(__size, __short_size)			\
 {								\
 	.name[KMALLOC_NORMAL]  = "kmalloc-" #__short_size,	\

@@ -850,6 +860,7 @@ EXPORT_SYMBOL(kmalloc_size_roundup);
 	KMALLOC_CGROUP_NAME(__short_size)			\
 	KMALLOC_DMA_NAME(__short_size)				\
 	KMALLOC_PARTITION_NAME(KMALLOC_PARTITION_CACHES_NR, __short_size)	\
+	KMALLOC_NO_OBJ_EXT_NAME(__short_size)			\
 	.size = __size,						\
 }
 

@@ -966,6 +977,11 @@ new_kmalloc_cache(int idx, enum kmalloc_cache_type type)
 		flags |= SLAB_NO_MERGE;
 #endif
 
+#ifdef CONFIG_SLAB_OBJ_EXT
+	if (type == KMALLOC_NO_OBJ_EXT)
+		flags |= SLAB_NO_OBJ_EXT | SLAB_NO_MERGE;
+#endif
+
 	/*
 	 * If CONFIG_MEMCG is enabled, disable cache merging for
 	 * KMALLOC_NORMAL caches.

2.53.0

Since we agreed to investigate later whether it makes sense to start allocating all obj_ext allocations from the new type, I wonder if for now we should not create them at all if they won't be used - if anything to avoid polluting slabinfo. AFAIU that means only create them with mem alloc profiling is config-enabled and mem_profiling_support == true? (Or probably also with SLUB_TINY && MEMCG, as nonsensical as it is...)

That's fair point!

assuming mem_profiling_support == true means not "never"...

I'll instead check "is any kmalloc type w/ higher priority than KMALLOC_CGROUP aliased with KMALLOC_NORMAL?", which would be

IS_ENABLED(CONFIG_MEMCG) &&
(KMALLOC_DMA == KMALLOC_NORMAL || KMALLOC_RECLAIM == KMALLOC_NORMAL),

Cheers,
Harry / Hyeonggon