mm/slab: fix unbounded recursion in free path with memalloc profiling
Subsystems (4)
Harry Yoo (Oracle)
maintainerv3 cover mm/slab: fix unbounded recursion in free path with memalloc profiling
This is a follow-up fix after the recent discussion [1]. See patch 4 for the detailed description on the bug.
Based on slab/for-next-fixes (af9ea231c0b45) and is 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-v3r1
To: Vlastimil Babka <[email protected]>
To: Andrew Morton <[email protected]>
To: Hao Li <[email protected]>
To: Christoph Lameter <[email protected]>
To: David Rientjes <[email protected]>
To: Roman Gushchin <[email protected]>
To: Suren Baghdasaryan <[email protected]>
To: Hao Ge <[email protected]>
To: Kees Cook <[email protected]>
To: Pedro Falcato <[email protected]>
To: Shakeel Butt <[email protected]>
To: Danielle Constantino <[email protected]>
To: Liam R. Howlett <[email protected]>
---
Changes in v3 (range-diff included at the end):
- Do not create kmalloc-no-objext-* caches unless required (Vlastimil)
- Introduce mem_alloc_profiling_permanently_disabled() in patch 3,
to avoid creating KMALLOC_NO_OBJ_EXT caches when mem_profiling is
permanently disabled.
- Make KMALLOC_NO_OBJ_EXT an alias for KMALLOC_NORMAL if
CONFIG_SLAB_OBJ_EXT is not defined and avoid ifdefs (refactoring).
- Link to v2: https://lore.kernel.org/r/[email protected]
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]
---
$ b4 prep --compare-to v2
1: 26e19124ce7a = 1: 701534464f8e mm/slab: fix a memory leak due to bootstrapping sheaves twice
2: a4a3a61cea59 = 2: 8c23ba4a36c9 mm/slab: decouple SLAB_NO_SHEAVES from SLAB_NO_OBJ_EXT
-: ------------ > 3: c285dbfbf6fb lib/alloc_tag: introduce mem_alloc_profiling_permanently_disabled()
3: 836ba88ba786 ! 4: 8d7f1e6dd334 mm/slab: prevent unbounded recursion in free path with new kmalloc type
@@ Commit message
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.
+ KMALLOC_NO_OBJ_EXT caches are created with SLAB_NO_OBJ_EXT flag when
+ either 1) memory allocation profiling is not permanently disabled,
+ or 2) kmalloc types with a priority higher than KMALLOC_CGROUP are
+ aliased with KMALLOC_NORMAL.
Sheaf bootstrapping for KMALLOC_NO_OBJ_EXT caches now must be deferred
because allocation of a barn can trigger obj_exts array allocation of
@@ Commit message
Signed-off-by: Harry Yoo (Oracle) <[email protected]>
## include/linux/slab.h ##
+@@ include/linux/slab.h: enum kmalloc_cache_type {
+ #endif
+ #ifndef CONFIG_MEMCG
+ KMALLOC_CGROUP = KMALLOC_NORMAL,
++#endif
++#ifndef CONFIG_SLAB_OBJ_EXT
++ KMALLOC_NO_OBJ_EXT = KMALLOC_NORMAL,
+ #endif
+ KMALLOC_PARTITION_START = KMALLOC_NORMAL,
+ KMALLOC_PARTITION_END = KMALLOC_PARTITION_START + KMALLOC_PARTITION_CACHES_NR,
@@ include/linux/slab.h: enum kmalloc_cache_type {
#endif
#ifdef CONFIG_MEMCG
@@ mm/slab.h: static inline unsigned int size_index_elem(unsigned int bytes)
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)];
@@ mm/slab.h: static inline bool is_kmalloc_normal(struct kmem_cache *s)
}
bool __kfree_rcu_sheaf(struct kmem_cache *s, void *obj);
+@@ mm/slab.h: static inline void metadata_access_disable(void)
+ kasan_enable_current();
+ }
+
++/*
++ * Return true if KMALLOC_NORMAL caches may need obj_exts arrays.
++ *
++ * Memory allocation profiling requires obj_exts for all caches.
++ * Memcg usually doesn't need them for normal kmalloc caches, but kmalloc types
++ * with a priority higher than KMALLOC_CGROUP can be aliased with KMALLOC_NORMAL.
++ */
++static inline bool need_kmalloc_no_objext(void)
++{
++ if (!mem_alloc_profiling_permanently_disabled())
++ return true;
++
++ if (!mem_cgroup_kmem_disabled() &&
++ (KMALLOC_NORMAL == KMALLOC_RECLAIM))
++ return true;
++
++ return false;
++}
++
+ #ifdef CONFIG_SLAB_OBJ_EXT
+
+ /*
## mm/slab_common.c ##
@@ mm/slab_common.c: u8 kmalloc_size_index[24] __ro_after_init = {
@@ mm/slab_common.c: EXPORT_SYMBOL(kmalloc_size_roundup);
}
@@ mm/slab_common.c: 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)
+ return;
+ }
+ flags |= SLAB_ACCOUNT;
++ } else if (IS_ENABLED(CONFIG_SLAB_OBJ_EXT) && type == KMALLOC_NO_OBJ_EXT) {
++ if (!need_kmalloc_no_objext()) {
++ kmalloc_caches[type][idx] = kmalloc_caches[KMALLOC_NORMAL][idx];
++ return;
++ }
+ flags |= SLAB_NO_OBJ_EXT | SLAB_NO_MERGE;
-+#endif
-+
- /*
- * If CONFIG_MEMCG is enabled, disable cache merging for
- * KMALLOC_NORMAL caches.
+ } else if (IS_ENABLED(CONFIG_ZONE_DMA) && (type == KMALLOC_DMA)) {
+ flags |= SLAB_CACHE_DMA;
+ }
## mm/slub.c ##
@@ mm/slub.c: static inline void init_slab_obj_exts(struct slab *slab)
@@ mm/slub.c: int alloc_slab_obj_exts(struct slab *slab, struct kmem_cache *s,
+ * However, normal kmalloc caches must allocate them from
+ * KMALLOC_NO_OBJ_EXT caches to prevent recursion.
+ */
-+ if (is_kmalloc_normal(s))
++ if (is_kmalloc_normal(s)) {
++ VM_WARN_ON_ONCE(!need_kmalloc_no_objext());
+ alloc_flags |= SLAB_ALLOC_NO_OBJ_EXT;
++ }
- sz = obj_exts_alloc_size(s, slab, gfp);
+ alloc_flags &= ~SLAB_ALLOC_NEW_SLAB;---
Harry Yoo (Oracle) (4):
mm/slab: fix a memory leak due to bootstrapping sheaves twice mm/slab: decouple SLAB_NO_SHEAVES from SLAB_NO_OBJ_EXT lib/alloc_tag: introduce mem_alloc_profiling_permanently_disabled() mm/slab: prevent unbounded recursion in free path with new kmalloc type
include/linux/alloc_tag.h | 3 ++ include/linux/slab.h | 19 +++++++- lib/alloc_tag.c | 9 ++++ mm/slab.h | 32 +++++++++++-- mm/slab_common.c | 19 +++++++- mm/slub.c | 111 ++++++++++++++++++++-------------------------- 6 files changed, 123 insertions(+), 70 deletions(-)
---
base-commit: 72bb229f9161a1efcd5df32141b69fcc6ae81a13
change-id: 20260702-kmalloc-no-objext-2619c1e06083
Best regards,
Signature · 1 line
Harry Yoo (Oracle) <[email protected]>
Harry Yoo (Oracle)
maintainerPATCH v3 1/4 mm/slab: fix a memory leak due to bootstrapping sheaves twice
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(-)
mm/slub.c +7 -2
@@ -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); } } }
Signature · 1 line
2.53.0
Re: [PATCH slab/for-next-fixes v3 1/4] mm/slab: fix a memory leak due to bootstrapping sheaves twice
Quoted 10 lines · Harry Yoo (Oracle)
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.
Quoted 7 lines
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.Fixes 913ffd3a1bf5 ("slab: handle kmalloc sheaves bootstrap")Signed-off-by Harry Yoo (Oracle) <[email protected]>
nit: Don't know if we really need a warning... I would just do an early return from bootstrap_cache_sheaves() if cache_has_sheaves() is true. But that's not critical.
Quoted 35 lines
--- mm/slub.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-)mm/slub.c +7
-3@@ -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
Harry Yoo
maintainerRe: [PATCH slab/for-next-fixes v3 1/4] mm/slab: fix a memory leak due to bootstrapping sheaves twice
Quoted 15 lines · Suren Baghdasaryan
Quoted 14 lines · Harry Yoo (Oracle)
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.
Quoted 5 lines
Quoted 3 lines
Fixes 913ffd3a1bf5 ("slab: handle kmalloc sheaves bootstrap")Signed-off-by Harry Yoo (Oracle) <[email protected]>nit: Don't know if we really need a warning...
Hehe, at least the warning allowed me to confirm that this is not an oversight and the patch fixed it :)
Quoted 3 lines
I would just do an
early return frombootstrap_cache_sheaves()ifcache_has_sheaves()is true. But that's not critical.
But yeah, no strong opinion on this.
Quoted 1 line
Reviewed-by Suren Baghdasaryan <[email protected]>
Thanks!
Signature · 2 lines
Cheers, Harry / Hyeonggon
Harry Yoo (Oracle)
maintainerPATCH v3 2/4 mm/slab: decouple SLAB_NO_SHEAVES from SLAB_NO_OBJ_EXT
In reply to Harry Yoo (Oracle)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(-)
include/linux/slab.h +11 -2
@@ -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)
mm/slub.c +6 -4
@@ -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);
Signature · 1 line
2.53.0
Re: [PATCH slab/for-next-fixes v3 2/4] mm/slab: decouple SLAB_NO_SHEAVES from SLAB_NO_OBJ_EXT
Quoted 8 lines · Harry Yoo (Oracle)
Bootstrap caches are created with
SLAB_NO_OBJ_EXTto disallow sheaves andobj_exts.To allow disabling
obj_extswhile allowing sheaves, decoupleSLAB_NO_SHEAVESfromSLAB_NO_OBJ_EXT. Bootstrap caches now have bothSLAB_NO_SHEAVESandSLAB_NO_OBJ_EXT.
Quoted 6 lines
No functional change intended.
Fixes e47c897a2949 ("slab: add sheaves to most caches")Reviewed-by Vlastimil Babka (SUSE) <[email protected]>Signed-off-by Harry Yoo (Oracle) <[email protected]>
Quoted 83 lines
--- include/linux/slab.h | 13 +++++++++++-- mm/slub.c | 10 ++++++---- 2 files changed, 17 insertions(+), 6 deletions(-)include/linux/slab.h +11
-2@@ -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)mm/slub.c +6
-5@@ -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
Harry Yoo (Oracle)
maintainerPATCH v3 3/4 lib/alloc_tag: introduce mem_alloc_profiling_permanently_disabled()
In reply to Harry Yoo (Oracle)mem_alloc_profiling_enabled() tells whether memalloc profiling is currently enabled. However, even when this function returns false, it can be enabled later.
However, this is not enough. Some optimizations can be applied only when memalloc profiling is permanently disabled. For example, to skip the creation of KMALLOC_NO_OBJ_EXT caches at boot time, mem_profiling must be set to "never", "0" w/ debugging on, or have been shutdown so that it can no longer be enabled.
Introduce mem_alloc_profiling_permanently_disabled() for this purpose.
--- include/linux/alloc_tag.h | 3 +++ lib/alloc_tag.c | 9 +++++++++ 2 files changed, 12 insertions(+)
include/linux/alloc_tag.h +3 -0
@@ -134,6 +134,8 @@ static inline bool mem_alloc_profiling_enabled(void) &mem_alloc_profiling_key); } +bool mem_alloc_profiling_permanently_disabled (void ); +static inline struct alloc_tag_counters alloc_tag_read (struct alloc_tag *tag) {struct alloc_tag_counters v = {0 ,0 };
@@ -239,6 +241,7 @@ static inline bool alloc_tag_is_inaccurate(struct alloc_tag *tag)#define DEFINE_ALLOC_TAG (_alloc_tag)static inline bool mem_alloc_profiling_enabled (void ) {return false; } +static inline bool mem_alloc_profiling_permanently_disabled (void ) {return true; }static inline void alloc_tag_add (union codetag_ref *ref,struct alloc_tag *tag,size_t bytes) {}static inline void alloc_tag_sub (union codetag_ref *ref,size_t bytes) {}
lib/alloc_tag.c +9 -0
@@ -26,6 +26,15 @@ static bool mem_profiling_support = true;static bool mem_profiling_support;#endif +/* +* Memory allocation profiling is permanently disabled and cannot be enabled. +* Must be called after setup_early_mem_profiling(). +*/ +bool mem_alloc_profiling_permanently_disabled (void ) +{ +return !mem_profiling_support; +} +static struct codetag_type *alloc_tag_cttype;#ifdef CONFIG_ARCH_MODULE_NEEDS_WEAK_PER_CPU
Signature · 1 line
2.53.0
Re: [PATCH slab/for-next-fixes v3 3/4] lib/alloc_tag: introduce mem_alloc_profiling_permanently_disabled()
Quoted 5 lines · Harry Yoo (Oracle)
mem_alloc_profiling_enabled()tells whether memalloc profiling is currently enabled. However, even when this function returns false, it can be enabled later.
Quoted 7 lines
However, this is not enough. Some optimizations can be applied only when memalloc profiling is permanently disabled. For example, to skip the creation of
KMALLOC_NO_OBJ_EXTcaches at boot time,mem_profilingmust be set to "never", "0" w/ debugging on, or have been shutdown so that it can no longer be enabled.Introduce
mem_alloc_profiling_permanently_disabled()for this purpose.
nit: I would call it mem_alloc_profiling_supported() but this name would do.
Quoted 4 lines
Fixes 4b8736964640 ("mm/slab: add allocation accounting into slab allocation and free paths")Signed-off-by Harry Yoo (Oracle) <[email protected]>
Quoted 50 lines
--- include/linux/alloc_tag.h | 3 +++ lib/alloc_tag.c | 9 +++++++++ 2 files changed, 12 insertions(+)include/linux/alloc_tag.h +3
-0@@ -134,6 +134,8 @@ static inline bool mem_alloc_profiling_enabled(void) &mem_alloc_profiling_key); } +bool mem_alloc_profiling_permanently_disabled (void ); +static inline struct alloc_tag_counters alloc_tag_read (struct alloc_tag *tag) {struct alloc_tag_counters v = {0 ,0 };@@ -239,6 +241,7 @@ static inline bool alloc_tag_is_inaccurate(struct alloc_tag *tag)#define DEFINE_ALLOC_TAG (_alloc_tag)static inline bool mem_alloc_profiling_enabled (void ) {return false; } +static inline bool mem_alloc_profiling_permanently_disabled (void ) {return true; }static inline void alloc_tag_add (union codetag_ref *ref,struct alloc_tag *tag,size_t bytes) {}static inline void alloc_tag_sub (union codetag_ref *ref,size_t bytes) {}lib/alloc_tag.c +9
-1@@ -26,6 +26,15 @@ static bool mem_profiling_support = true;static bool mem_profiling_support;#endif +/* +* Memory allocation profiling is permanently disabled and cannot be enabled. +* Must be called after setup_early_mem_profiling(). +*/ +bool mem_alloc_profiling_permanently_disabled (void ) +{ +return !mem_profiling_support; +} +static struct codetag_type *alloc_tag_cttype;#ifdef CONFIG_ARCH_MODULE_NEEDS_WEAK_PER_CPU --2.53.0
Vlastimil Babka (SUSE)
maintainerRe: [PATCH slab/for-next-fixes v3 3/4] lib/alloc_tag: introduce mem_alloc_profiling_permanently_disabled()
Quoted 12 lines · Suren Baghdasaryan
Quoted 11 lines · Harry Yoo (Oracle)
mem_alloc_profiling_enabled()tells whether memalloc profiling is currently enabled. However, even when this function returns false, it can be enabled later.However, this is not enough. Some optimizations can be applied only when memalloc profiling is permanently disabled. For example, to skip the creation of
KMALLOC_NO_OBJ_EXTcaches at boot time,mem_profilingmust be set to "never", "0" w/ debugging on, or have been shutdown so that it can no longer be enabled.
Quoted 3 lines
Quoted 1 line
Introduce
mem_alloc_profiling_permanently_disabled()for this purpose.nit: I would call it
mem_alloc_profiling_supported()but this name would do.
I wouldn't like that name, because _supported to me sounds like those CONFIG_ARCH_SUPPORTS... things and not something that was just disabled. I suggested mem_alloc_profiling_unavailable(), but it's not ideal either, naming is hard.
Quoted 57 lines
Quoted 4 lines
Fixes 4b8736964640 ("mm/slab: add allocation accounting into slab allocation and free paths")Signed-off-by Harry Yoo (Oracle) <[email protected]>Quoted 50 lines
--- include/linux/alloc_tag.h | 3 +++ lib/alloc_tag.c | 9 +++++++++ 2 files changed, 12 insertions(+)include/linux/alloc_tag.h +3
-0@@ -134,6 +134,8 @@ static inline bool mem_alloc_profiling_enabled(void) &mem_alloc_profiling_key); } +bool mem_alloc_profiling_permanently_disabled (void ); +static inline struct alloc_tag_counters alloc_tag_read (struct alloc_tag *tag) {struct alloc_tag_counters v = {0 ,0 };@@ -239,6 +241,7 @@ static inline bool alloc_tag_is_inaccurate(struct alloc_tag *tag)#define DEFINE_ALLOC_TAG (_alloc_tag)static inline bool mem_alloc_profiling_enabled (void ) {return false; } +static inline bool mem_alloc_profiling_permanently_disabled (void ) {return true; }static inline void alloc_tag_add (union codetag_ref *ref,struct alloc_tag *tag,size_t bytes) {}static inline void alloc_tag_sub (union codetag_ref *ref,size_t bytes) {}lib/alloc_tag.c +9
-1@@ -26,6 +26,15 @@ static bool mem_profiling_support = true;static bool mem_profiling_support;#endif +/* +* Memory allocation profiling is permanently disabled and cannot be enabled. +* Must be called after setup_early_mem_profiling(). +*/ +bool mem_alloc_profiling_permanently_disabled (void ) +{ +return !mem_profiling_support; +} +static struct codetag_type *alloc_tag_cttype;#ifdef CONFIG_ARCH_MODULE_NEEDS_WEAK_PER_CPU --2.53.0
Harry Yoo
maintainerRe: [PATCH slab/for-next-fixes v3 3/4] lib/alloc_tag: introduce mem_alloc_profiling_permanently_disabled()
Quoted 13 lines · Vlastimil Babka (SUSE)
Quoted 12 lines · Suren Baghdasaryan
Quoted 11 lines · Harry Yoo (Oracle)
mem_alloc_profiling_enabled()tells whether memalloc profiling is currently enabled. However, even when this function returns false, it can be enabled later.However, this is not enough. Some optimizations can be applied only when memalloc profiling is permanently disabled. For example, to skip the creation of
KMALLOC_NO_OBJ_EXTcaches at boot time,mem_profilingmust be set to "never", "0" w/ debugging on, or have been shutdown so that it can no longer be enabled.
Quoted 6 lines
Quoted 3 lines
Quoted 1 line
Introduce
mem_alloc_profiling_permanently_disabled()for this purpose.nit: I would call it
mem_alloc_profiling_supported()but this name would do.I wouldn't like that name, because
_supportedto me sounds like thoseCONFIG_ARCH_SUPPORTS... things and not something that was just disabled.
Agreed!
Actually, last year I wasn't sure what "_support" in mem_profiling_support means and had to look at the code while fixing a bug [1].
This year I forgot what it meant and had to look at the code again :)
Quoted 1 line
I suggested
mem_alloc_profiling_unavailable(), but it's not ideal either,
Hehe, in terms of 'the meaning is obvious without reading the code', it's not very straightforward...
Quoted 1 line
naming is hard.
Agreed!
[1]
https://lore.kernel.org/linux-mm/[email protected]
Signature · 2 lines
Cheers, Harry / Hyeonggon
Suren Baghdasaryan
maintainerRe: [PATCH slab/for-next-fixes v3 3/4] lib/alloc_tag: introduce mem_alloc_profiling_permanently_disabled()
Quoted 32 lines · Harry Yoo
Quoted 13 lines · Vlastimil Babka (SUSE)
Quoted 12 lines · Suren Baghdasaryan
Quoted 11 lines · Harry Yoo (Oracle)
mem_alloc_profiling_enabled()tells whether memalloc profiling is currently enabled. However, even when this function returns false, it can be enabled later.However, this is not enough. Some optimizations can be applied only when memalloc profiling is permanently disabled. For example, to skip the creation of
KMALLOC_NO_OBJ_EXTcaches at boot time,mem_profilingmust be set to "never", "0" w/ debugging on, or have been shutdown so that it can no longer be enabled.Quoted 6 lines
Quoted 3 lines
Quoted 1 line
Introduce
mem_alloc_profiling_permanently_disabled()for this purpose.nit: I would call it
mem_alloc_profiling_supported()but this name would do.I wouldn't like that name, because
_supportedto me sounds like thoseCONFIG_ARCH_SUPPORTS... things and not something that was just disabled.Agreed!
Actually, last year I wasn't sure what "
_support" inmem_profiling_supportmeans and had to look at the code while fixing a bug [1].This year I forgot what it meant and had to look at the code again :)
Quoted 8 lines
Quoted 1 line
I suggested
mem_alloc_profiling_unavailable(), but it's not ideal either,Hehe, in terms of 'the meaning is obvious without reading the code', it's not very straightforward...
Quoted 1 line
naming is hard.
Agreed!
I'll let you pick one :)
Quoted 8 lines
[1]
https://lore.kernel.org/linux-mm/[email protected]--
Cheers,
Harry / Hyeonggon
Harry Yoo
maintainerRe: [PATCH slab/for-next-fixes v3 3/4] lib/alloc_tag: introduce mem_alloc_profiling_permanently_disabled()
Quoted 32 lines · Suren Baghdasaryan
Quoted 31 lines · Harry Yoo
Quoted 13 lines · Vlastimil Babka (SUSE)
Quoted 12 lines · Suren Baghdasaryan
Quoted 11 lines · Harry Yoo (Oracle)
mem_alloc_profiling_enabled()tells whether memalloc profiling is currently enabled. However, even when this function returns false, it can be enabled later.However, this is not enough. Some optimizations can be applied only when memalloc profiling is permanently disabled. For example, to skip the creation of
KMALLOC_NO_OBJ_EXTcaches at boot time,mem_profilingmust be set to "never", "0" w/ debugging on, or have been shutdown so that it can no longer be enabled.Quoted 6 lines
Quoted 3 lines
Quoted 1 line
Introduce
mem_alloc_profiling_permanently_disabled()for this purpose.nit: I would call it
mem_alloc_profiling_supported()but this name would do.I wouldn't like that name, because
_supportedto me sounds like thoseCONFIG_ARCH_SUPPORTS... things and not something that was just disabled.Agreed!
Actually, last year I wasn't sure what "
_support" inmem_profiling_supportmeans and had to look at the code while fixing a bug [1].This year I forgot what it meant and had to look at the code again :)
Quoted 1 line
I suggested
mem_alloc_profiling_unavailable(), but it's not ideal either,
Quoted 8 lines
Quoted 6 lines
Hehe, in terms of 'the meaning is obvious without reading the code', it's not very straightforward...
Quoted 1 line
naming is hard.
Agreed!
I'll let you pick one :)
Haha, I'm honored! ;)
Signature · 2 lines
Cheers, Harry / Hyeonggon
Harry Yoo (Oracle)
maintainerPATCH v3 4/4 mm/slab: prevent unbounded recursion in free path with new kmalloc type
In reply to Harry Yoo (Oracle)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 with SLAB_NO_OBJ_EXT flag when either 1) memory allocation profiling is not permanently disabled, or 2) kmalloc types with a priority higher than KMALLOC_CGROUP are aliased with KMALLOC_NORMAL.
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 | 6 ++++ mm/slab.h | 32 ++++++++++++++++-- mm/slab_common.c | 19 ++++++++++- mm/slub.c | 92 +++++++++++++++++++--------------------------------- 4 files changed, 87 insertions(+), 62 deletions(-)
include/linux/slab.h +6 -0
@@ -708,6 +708,9 @@ enum kmalloc_cache_type {#endif #ifndef CONFIG_MEMCG KMALLOC_CGROUP = KMALLOC_NORMAL, +#endif +#ifndef CONFIG_SLAB_OBJ_EXT +KMALLOC_NO_OBJ_EXT =KMALLOC_NORMAL , #endifKMALLOC_PARTITION_START =KMALLOC_NORMAL , KMALLOC_PARTITION_END = KMALLOC_PARTITION_START + KMALLOC_PARTITION_CACHES_NR,
@@ -721,6 +724,9 @@ enum kmalloc_cache_type {#endif #ifdef CONFIG_MEMCG KMALLOC_CGROUP, +#endif +#ifdef CONFIG_SLAB_OBJ_EXT +KMALLOC_NO_OBJ_EXT , #endifNR_KMALLOC_TYPES };
mm/slab.h +29 -3
@@ -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,17 @@ 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); + +if (alloc_flags &SLAB_ALLOC_NO_OBJ_EXT ) + type =KMALLOC_NO_OBJ_EXT ;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 +432,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);
@@ -529,6 +536,25 @@ static inline void metadata_access_disable(void)kasan_enable_current (); } +/* +* Return true if KMALLOC_NORMAL caches may need obj_exts arrays. +* +* Memory allocation profiling requires obj_exts for all caches. +* Memcg usually doesn't need them for normal kmalloc caches, but kmalloc types +* with a priority higher than KMALLOC_CGROUP can be aliased with KMALLOC_NORMAL. +*/ +static inline bool need_kmalloc_no_objext (void ) +{ +if (!mem_alloc_profiling_permanently_disabled ()) +return true; + +if (!mem_cgroup_kmem_disabled () && + (KMALLOC_NORMAL ==KMALLOC_RECLAIM )) +return true; + +return false; +} +#ifdef CONFIG_SLAB_OBJ_EXT /*
mm/slab_common.c +18 -1
@@ -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, \ }
@@ -957,6 +968,12 @@ new_kmalloc_cache(int idx, enum kmalloc_cache_type type)return ; } flags |=SLAB_ACCOUNT ; + }else if (IS_ENABLED (CONFIG_SLAB_OBJ_EXT ) && type ==KMALLOC_NO_OBJ_EXT ) { +if (!need_kmalloc_no_objext ()) { + kmalloc_caches[type][idx] = kmalloc_caches[KMALLOC_NORMAL ][idx]; +return ; + } + flags |=SLAB_NO_OBJ_EXT |SLAB_NO_MERGE ; }else if (IS_ENABLED (CONFIG_ZONE_DMA ) && (type ==KMALLOC_DMA )) { flags |=SLAB_CACHE_DMA ; }
mm/slub.c +34 -58
150 lines not on this page · raw message
Signature · 1 line
2.53.0
Suren Baghdasaryan
maintainerRe: [PATCH slab/for-next-fixes v3 4/4] mm/slab: prevent unbounded recursion in free path with new kmalloc type
Quoted 141 lines · Harry Yoo (Oracle)
Commit
280ea9c3154b("mm/slab: avoid allocatingslabobj_extarray from its own slab") avoided recursive allocation ofobj_extsfrom kmalloc caches of the same size, by bumping theobj_extsarray'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_extsarrays from each other [1]:What happened: a
KMALLOC_NORMALslab'sobj_extsarray (used by allocation profiling / memcg accounting) is itself kmalloc()'d from aKMALLOC_NORMALcache, so the "slab holds another slab'sobj_extsarray" relation can form cycles. Withsizeof(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_extsarray. 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_64It is proposed [1] to resolve this issue by always serving the
obj_extsarray 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 needobj_extsarrays 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_extsarrays. Currently, allobj_extsarrays are served from normal kmalloc caches. Cycles cannot be created ifobj_extsarrays of normal kmalloc caches are served from a special kmalloc type that can never haveobj_extsarrays.To achieve this, create a new kmalloc type called
KMALLOC_NO_OBJ_EXT.KMALLOC_NO_OBJ_EXTcaches are created withSLAB_NO_OBJ_EXTflag when either 1) memory allocation profiling is not permanently disabled, or 2) kmalloc types with a priority higher thanKMALLOC_CGROUPare aliased withKMALLOC_NORMAL.Sheaf bootstrapping for
KMALLOC_NO_OBJ_EXTcaches now must be deferred because allocation of a barn can triggerobj_extsarray allocation of normal kmalloc caches when theKMALLOC_NO_OBJ_EXTcache 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 ofobj_extsarrays, and letkmalloc_slab()override the type toKMALLOC_NO_OBJ_EXTwhen specified. Note thatkmalloc_type()remains unchanged becausekmalloc_flags()bypasses the kmalloc fastpath.Do not pass
SLAB_ALLOC_NO_RECURSEtokmalloc_flags()inalloc_slab_obj_exts()and instead useSLAB_ALLOC_NO_OBJ_EXTonly when the objects are allocated from normal kmalloc caches. While this prevents unbounded recursive allocation ofobj_exts, it allowsKMALLOC_NO_OBJ_EXTcaches to have sheaves.Since sheaf allocations specify
SLAB_ALLOC_NO_RECURSEthat prevents allocation of both sheaves andobj_extsarrays, the recursion depth is bounded.
obj_extsarrays for non-kmalloc-normal caches can now have a valid tag. Do not callmark_obj_codetag_empty()when freeing anobj_extsarray to avoid false warnings.KMALLOC_NO_OBJ_EXTdon't need this as they never allocate those arrays.Reported-by Danielle Costantino <[email protected]>Reported-by Shakeel Butt <[email protected]>Fixes 4b8736964640 ("mm/slab: add allocation accounting into slab allocation and free paths")Signed-off-by Harry Yoo (Oracle) <[email protected]>--- include/linux/slab.h | 6 ++++ mm/slab.h | 32 ++++++++++++++++-- mm/slab_common.c | 19 ++++++++++- mm/slub.c | 92 +++++++++++++++++++--------------------------------- 4 files changed, 87 insertions(+), 62 deletions(-)include/linux/slab.h +6
-0@@ -708,6 +708,9 @@ enum kmalloc_cache_type {#endif #ifndef CONFIG_MEMCG KMALLOC_CGROUP = KMALLOC_NORMAL, +#endif +#ifndef CONFIG_SLAB_OBJ_EXT +KMALLOC_NO_OBJ_EXT =KMALLOC_NORMAL , #endifKMALLOC_PARTITION_START =KMALLOC_NORMAL , KMALLOC_PARTITION_END = KMALLOC_PARTITION_START + KMALLOC_PARTITION_CACHES_NR,@@ -721,6 +724,9 @@ enum kmalloc_cache_type {#endif #ifdef CONFIG_MEMCG KMALLOC_CGROUP, +#endif +#ifdef CONFIG_SLAB_OBJ_EXT +KMALLOC_NO_OBJ_EXT , #endifNR_KMALLOC_TYPES };mm/slab.h +1
-0@@ -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,17 @@ static inline unsigned int size_index_elem(unsigned int bytes)* KMALLOC_MAX_CACHE_SIZE and the caller must check that. */
mm/slab.h static inline unsigned int size_index_elem(unsigned int bytes) · line 397
Quoted 10 lines
mm/slab.h +6
-1static 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); + +if (alloc_flags &SLAB_ALLOC_NO_OBJ_EXT ) + type =KMALLOC_NO_OBJ_EXT ;
Why not let kmalloc_type() handle alloc_flags? Other users (there are only 4 of them) can pass SLAB_ALLOC_DEFAULT. That seems cleaner to me and more robust.
Quoted 86 lines
mm/slab.h +22
-2if (!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 +432,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);@@ -529,6 +536,25 @@ static inline void metadata_access_disable(void)kasan_enable_current (); } +/* +* Return true if KMALLOC_NORMAL caches may need obj_exts arrays. +* +* Memory allocation profiling requires obj_exts for all caches. +* Memcg usually doesn't need them for normal kmalloc caches, but kmalloc types +* with a priority higher than KMALLOC_CGROUP can be aliased with KMALLOC_NORMAL. +*/ +static inline bool need_kmalloc_no_objext (void ) +{ +if (!mem_alloc_profiling_permanently_disabled ()) +return true; + +if (!mem_cgroup_kmem_disabled () && + (KMALLOC_NORMAL ==KMALLOC_RECLAIM )) +return true; + +return false; +} +#ifdef CONFIG_SLAB_OBJ_EXT /* mm/slab_common.c +12
-1@@ -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, \ }
mm/slab_common.c new_kmalloc_cache(int idx, enum kmalloc_cache_type type) · line 971
Quoted 4 lines
mm/slab_common.c +1
-0@@ -957,6 +968,12 @@ new_kmalloc_cache(int idx, enum kmalloc_cache_type type)return ; } flags |=SLAB_ACCOUNT ; + }else if (IS_ENABLED (CONFIG_SLAB_OBJ_EXT ) && type ==KMALLOC_NO_OBJ_EXT ) {
Hmm, you have to check IS_ENABLED(CONFIG_SLAB_OBJ_EXT) here because KMALLOC_NO_OBJ_EXT can be aliased with KMALLOC_NORMAL... Could we instead have a helper function like this (maybe with a better name):
#ifdef CONFIG_SLAB_OBJ_EXT
bool is_kmalloc_no_obj_ext_type(type) { return type == KMALLOC_NO_OBJ_EXT; } #else
bool is_kmalloc_no_obj_ext_type(type) { return false; }
#endif
?
mm/slab_common.c new_kmalloc_cache(int idx, enum kmalloc_cache_type type) · line 973
Quoted 2 lines
mm/slab_common.c +2
-0+if (!need_kmalloc_no_objext ()) { + kmalloc_caches[type][idx] = kmalloc_caches[KMALLOC_NORMAL ][idx];
Could kmalloc_caches[KMALLOC_NORMAL][idx] be NULL here?
In general why do we special-case and do an early exit here? Can we do instead:
if (need_kmalloc_no_objext())
flags |= SLAB_NO_OBJ_EXT | SLAB_NO_MERGE;
and use the common path?
Quoted 58 lines
mm/slab_common.c +3
-0+return ; + } + flags |=SLAB_NO_OBJ_EXT |SLAB_NO_MERGE ; }else if (IS_ENABLED (CONFIG_ZONE_DMA ) && (type ==KMALLOC_DMA )) { flags |=SLAB_CACHE_DMA ; }mm/slub.c +0
-37@@ -2123,42 +2123,6 @@ static inline void init_slab_obj_exts(struct slab *slab) slab->obj_exts =0 ; } -/* -* Calculate the allocation size for slabobj_ext array. -* -* When memory allocation profiling is enabled, the obj_exts array -* could be allocated from the same slab cache it's being allocated for. -* This would prevent the slab from ever being freed because it would -* always contain at least one allocated object (its own obj_exts array). -* -* To avoid this, increase the allocation size when we detect the array -* may come from the same cache, forcing it to use a different cache. -*/ -static inline size_t obj_exts_alloc_size (struct kmem_cache *s, -struct slab *slab,gfp_t gfp) -{ -size_t sz =sizeof (struct slabobj_ext ) * slab->objects; -struct kmem_cache *obj_exts_cache; - -if (sz >KMALLOC_MAX_CACHE_SIZE ) -return sz; - -if (!is_kmalloc_normal (s)) -return sz; - - obj_exts_cache =kmalloc_slab (sz,NULL , gfp,__kmalloc_token (0 )); -/* -* We can't simply compare s with obj_exts_cache, because partitioned kmalloc -* caches have multiple caches per size, selected by caller address or type. -* Since caller address or type may differ between kmalloc_slab() and actual -* allocation, bump size when sizes are equal. -*/ -if (s->object_size == obj_exts_cache->object_size) -return obj_exts_cache->object_size +1 ; - -return sz; -} -int alloc_slab_obj_exts (struct slab *slab,struct kmem_cache *s,gfp_t gfp,unsigned int alloc_flags) {@@ -2168,14 +2132,20 @@ int alloc_slab_obj_exts(struct slab *slab, struct kmem_cache *s,unsigned long new_exts;unsigned long old_exts;struct slabobj_ext *vec; -size_t sz;
mm/slub.c int alloc_slab_obj_exts(struct slab *slab, struct kmem_cache *s, · line 2141
Quoted 10 lines
mm/slub.c +5
-3+size_t sz =sizeof (struct slabobj_ext ) * slab->objects; gfp &= ~OBJCGS_CLEAR_MASK ; -/* Prevent recursive extension vector allocation */ - alloc_flags |=SLAB_ALLOC_NO_RECURSE ; - alloc_flags &= ~SLAB_ALLOC_NEW_SLAB ; +/* +* In most cases, obj_exts arrays are allocated from normal kmalloc. +* However, normal kmalloc caches must allocate them from +* KMALLOC_NO_OBJ_EXT caches to prevent recursion.
For debugging it would have been convenient to allocate all obj_ext vectors from dedicated caches... Maybe we can do that for CONFIG_DEBUG_VM or someday when we add CONFIG_OBJ_EXT_DEBUG? Anyway, not really a complaint but a wish.
Quoted 105 lines
mm/slub.c +29
-19+*/ +if (is_kmalloc_normal (s)) { +VM_WARN_ON_ONCE (!need_kmalloc_no_objext ()); + alloc_flags |=SLAB_ALLOC_NO_OBJ_EXT ; + } - sz =obj_exts_alloc_size (s, slab, gfp); + alloc_flags &= ~SLAB_ALLOC_NEW_SLAB ;/* This will use kmalloc_nolock() if alloc_flags say so */ vec =kmalloc_flags (sz, gfp | __GFP_ZERO, alloc_flags,slab_nid (slab));@@ -2193,8 +2163,21 @@ int alloc_slab_obj_exts(struct slab *slab, struct kmem_cache *s,return -ENOMEM ; } -VM_WARN_ON_ONCE (virt_to_slab (vec) !=NULL && -virt_to_slab (vec)->slab_cache == s); +if (IS_ENABLED (CONFIG_DEBUG_VM )) { +struct kmem_cache *exts_cache; +struct slab *exts_slab; + + exts_slab =virt_to_slab (vec); +if (exts_slab) { +/* +* The vector must be allocated from either normal or +* KMALLOC_NO_OBJ_EXT kmalloc caches to avoid cycles. +*/ + exts_cache = exts_slab->slab_cache; +WARN_ON_ONCE (!is_kmalloc_normal (exts_cache) && + !(exts_cache->flags &SLAB_NO_OBJ_EXT )); + } + } new_exts = (unsigned long )vec;#ifdef CONFIG_MEMCG @@ -2217,7 +2200,6 @@ int alloc_slab_obj_exts(struct slab *slab, struct kmem_cache *s,* assign slabobj_exts in parallel. In this case the existing * objcg vector should be reused. */ -mark_obj_codetag_empty (vec);if (unlikely (!allow_spin))kfree_nolock (vec);else @@ -2253,14 +2235,6 @@ static inline void free_slab_obj_exts(struct slab *slab, bool allow_spin)return ; } -/* -* obj_exts was created with SLAB_ALLOC_NO_RECURSE flag, therefore its -* corresponding extension will be NULL. alloc_tag_sub() will throw a -* warning if slab has extensions but the extension of an object is -* NULL, therefore replace NULL with CODETAG_EMPTY to indicate that -* the extension for obj_exts is expected to be NULL. -*/ -mark_obj_codetag_empty (obj_exts);if (allow_spin)kfree (obj_exts);else @@ -5356,7 +5330,7 @@ void *__do_kmalloc_node(kmem_buckets *b, gfp_t flags, int node,if (unlikely (!size))return ZERO_SIZE_PTR ; - s =kmalloc_slab (size, b, flags, token); + s =kmalloc_slab (size, b, flags, token, ac->alloc_flags); ret =slab_alloc_node (s, flags, node, ac); ret =kasan_kmalloc (s, ret, size, flags);@@ -5419,7 +5393,9 @@ static void *__kmalloc_nolock_noprof(DECL_TOKEN_PARAMS(size, token), gfp_t gfp_f retry:if (unlikely (size >KMALLOC_MAX_CACHE_SIZE ))return NULL ; - s =kmalloc_slab (size,NULL , gfp_flags,PASS_TOKEN_PARAM (token)); + + s =kmalloc_slab (size,NULL , gfp_flags,PASS_TOKEN_PARAM (token), + ac->alloc_flags);if (!(s->flags & __CMPXCHG_DOUBLE) && !kmem_cache_debug (s))/* @@ -7979,10 +7955,10 @@ static int calculate_sizes(struct kmem_cache_args *args, struct kmem_cache *s) s->allocflags |= __GFP_RECLAIMABLE;/* -* For KMALLOC_NORMAL caches we enable sheaves later by -* bootstrap_kmalloc_sheaves() to avoid recursion +* For kmalloc caches we enable sheaves later by +* bootstrap_kmalloc_sheaves() to avoid recursion. */ -if (!is_kmalloc_normal (s)) +if (!is_kmalloc_cache (s)) s->sheaf_capacity =calculate_sheaf_capacity (s, args);/* @@ -8548,7 +8524,7 @@ static void __init bootstrap_kmalloc_sheaves(void) {enum kmalloc_cache_type type; -for (type =KMALLOC_NORMAL ; type <=KMALLOC_PARTITION_END ; type++) { +for (type =KMALLOC_NORMAL ; type <NR_KMALLOC_TYPES ; type++) {for (int idx =0 ; idx <KMALLOC_SHIFT_HIGH +1 ; idx++) {struct kmem_cache *s = kmalloc_caches[type][idx]; --2.53.0
Harry Yoo
maintainerRe: [PATCH slab/for-next-fixes v3 4/4] mm/slab: prevent unbounded recursion in free path with new kmalloc type
Quoted 14 lines · Suren Baghdasaryan
mm/slab.h static inline unsigned int size_index_elem(unsigned int bytes) · line 397
Quoted 13 lines · Harry Yoo (Oracle)
mm/slab.h +6
-1@@ -386,12 +387,17 @@ 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); + +if (alloc_flags &SLAB_ALLOC_NO_OBJ_EXT ) + type =KMALLOC_NO_OBJ_EXT ;
Hi Suren, thanks for the reviews.
It's indeed helpful to have an eye for those bugfixes.
Quoted 1 line
Why not let
kmalloc_type()handlealloc_flags?
Good point!
Quoted 3 lines
Other users (there are
only 4 of them) can passSLAB_ALLOC_DEFAULT. That seems cleaner to me and more robust.
Hmm, there was a reason... *checks notes*, oh, there is no note. IIRC I was afraid of exposing SLAB_ALLOC_* flags to arbitrary users.
should probably fine as long as it's not used in
kmalloc/kmem_cache_alloc() APIs, not sure.
Quoted 11 lines
Quoted 11 lines
mm/slab.h +1
-1if (!b) - b = &kmalloc_caches[kmalloc_type (flags, token)]; + b = &kmalloc_caches[type];if (size <=192 ) index = kmalloc_size_index[size_index_elem (size)];else mm/slab_common.c +0
-0@@ -957,6 +968,12 @@ new_kmalloc_cache(int idx, enum kmalloc_cache_type type)
Quoted 8 lines
mm/slab_common.c new_kmalloc_cache(int idx, enum kmalloc_cache_type type) · line 971
Quoted 4 lines
mm/slab_common.c +1
-0return ; } flags |=SLAB_ACCOUNT ; + }else if (IS_ENABLED (CONFIG_SLAB_OBJ_EXT ) && type ==KMALLOC_NO_OBJ_EXT ) {Hmm, you have to check
IS_ENABLED(CONFIG_SLAB_OBJ_EXT)here becauseKMALLOC_NO_OBJ_EXTcan be aliased withKMALLOC_NORMAL... Could we instead have a helper function like this (maybe with a better name):
Hmm that's fine, but I think that bit should not be part of -stable fixes at least. Here I tried to make it consistent with KMALLOC_RECLAIM and KMALLOC_DMA :)
Quoted 6 lines
#ifdef
CONFIG_SLAB_OBJ_EXT
boolis_kmalloc_no_obj_ext_type(type){ return type ==KMALLOC_NO_OBJ_EXT; } #else
boolis_kmalloc_no_obj_ext_type(type){ return false; }
#endif
?
is_kmalloc_no_obj_ext_type(),kmalloc_type_is_no_obj_ext(),is_kmalloc_type_no_obj_ext(),
...
naming is hard, ugh :)
Quoted 5 lines
mm/slab_common.c new_kmalloc_cache(int idx, enum kmalloc_cache_type type) · line 973
Quoted 3 lines
mm/slab_common.c +2
-0+if (!need_kmalloc_no_objext ()) { + kmalloc_caches[type][idx] = kmalloc_caches[KMALLOC_NORMAL ][idx];Could kmalloc_caches[KMALLOC_NORMAL][idx] be NULL here?
No. KMALLOC_NORMAL caches are created before all other kmalloc caches.
IIRC checking if kmalloc_caches[KMALLOC_NORMAL][idx] is NULL was added by commit 963e84b0f262 ("mm/slab: limit kmalloc() minimum alignment to dma_get_cache_alignment()") to avoid creating kmalloc caches of same type and size due to minimum alignment.
Quoted 8 lines
In general why do we special-case and do an early exit here?
Can we do instead:
if (
need_kmalloc_no_objext())flags |= SLAB_NO_OBJ_EXT | SLAB_NO_MERGE;and use the common path?
Hmm, but even without SLAB_NO_MERGE, we often end up not merging kmalloc caches e.g.) because of non-zero s->usersize.
I think that's why we do special-case and an early exit?
We could probably do some refactoring to change that, but in general I'm afraid of backporting refactoring work to -stable because I fear introducing very subtle behavioral changes that nobody would notice.
Quoted 17 lines
Quoted 17 lines
mm/slab_common.c +3
-0+return ; + } + flags |=SLAB_NO_OBJ_EXT |SLAB_NO_MERGE ; }else if (IS_ENABLED (CONFIG_ZONE_DMA ) && (type ==KMALLOC_DMA )) { flags |=SLAB_CACHE_DMA ; }mm/slub.c +1
-1@@ -2168,14 +2132,20 @@ int alloc_slab_obj_exts(struct slab *slab, struct kmem_cache *s,unsigned long new_exts;unsigned long old_exts;struct slabobj_ext *vec; -size_t sz; +size_t sz =sizeof (struct slabobj_ext ) * slab->objects;
Quoted 13 lines
mm/slub.c int alloc_slab_obj_exts(struct slab *slab, struct kmem_cache *s, · line 2141
Quoted 8 lines
mm/slub.c +4
-3gfp &= ~OBJCGS_CLEAR_MASK ; -/* Prevent recursive extension vector allocation */ - alloc_flags |=SLAB_ALLOC_NO_RECURSE ; - alloc_flags &= ~SLAB_ALLOC_NEW_SLAB ; +/* +* In most cases, obj_exts arrays are allocated from normal kmalloc. +* However, normal kmalloc caches must allocate them from +* KMALLOC_NO_OBJ_EXT caches to prevent recursion. For debugging it would have been convenient to allocate all
obj_extvectors from dedicated caches... Maybe we can do that forCONFIG_DEBUG_VMor someday when we addCONFIG_OBJ_EXT_DEBUG? Anyway, not really a complaint but a wish.
Vlastimil and I had a conversation on always (even w/o debug options) having dedicated caches (primarily to separate lifetime and for simplicity), it would be interesting to explore.
https://lore.kernel.org/all/[email protected]
Signature · 2 lines
Cheers, Harry / Hyeonggon
Vlastimil Babka (SUSE)
maintainerRe: [PATCH slab/for-next-fixes v3 4/4] mm/slab: prevent unbounded recursion in free path with new kmalloc type
Quoted 22 lines · Harry Yoo
Quoted 14 lines · Suren Baghdasaryan
mm/slab.h static inline unsigned int size_index_elem(unsigned int bytes) · line 397
Quoted 13 lines · Harry Yoo (Oracle)
mm/slab.h +6
-1@@ -386,12 +387,17 @@ 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); + +if (alloc_flags &SLAB_ALLOC_NO_OBJ_EXT ) + type =KMALLOC_NO_OBJ_EXT ;Hi Suren, thanks for the reviews.
It's indeed helpful to have an eye for those bugfixes.Quoted 1 line
Why not let
kmalloc_type()handlealloc_flags?
Quoted 8 lines
Good point!
Quoted 3 lines
Other users (there are
only 4 of them) can passSLAB_ALLOC_DEFAULT. That seems cleaner to me and more robust.Hmm, there was a reason... *checks notes*, oh, there is no note. IIRC I was afraid of exposing SLAB_ALLOC_* flags to arbitrary users.
Hm good point, they are in mm/slab.h and kmalloc_type() needs to be in the public include/linux/slab.h. Given its usage in __builtin_constant_p(size) paths of kmalloc[_node] I would rather not complicate it (even if a hardcoded SLAB_ALLOC_DEFAULT should eliminate the extra code). So I'd rather keep KMALLOC_NO_OBJ_EXT handling hidden in mm/ as long as it's possible.
Quoted 23 lines
should probably fine as long as it's not used in
kmalloc/kmem_cache_alloc() APIs, not sure.Quoted 11 lines
Quoted 11 lines
mm/slab.h +1
-1if (!b) - b = &kmalloc_caches[kmalloc_type (flags, token)]; + b = &kmalloc_caches[type];if (size <=192 ) index = kmalloc_size_index[size_index_elem (size)];else mm/slab_common.c +0
-0@@ -957,6 +968,12 @@ new_kmalloc_cache(int idx, enum kmalloc_cache_type type)Quoted 8 lines
mm/slab_common.c new_kmalloc_cache(int idx, enum kmalloc_cache_type type) · line 971
Quoted 4 lines
mm/slab_common.c +1
-0return ; } flags |=SLAB_ACCOUNT ; + }else if (IS_ENABLED (CONFIG_SLAB_OBJ_EXT ) && type ==KMALLOC_NO_OBJ_EXT ) {Hmm, you have to check
IS_ENABLED(CONFIG_SLAB_OBJ_EXT)here becauseKMALLOC_NO_OBJ_EXTcan be aliased withKMALLOC_NORMAL... Could we instead have a helper function like this (maybe with a better name):
Quoted 3 lines
Hmm that's fine, but I think that bit should not be part of -stable fixes at least. Here I tried to make it consistent with
KMALLOC_RECLAIMandKMALLOC_DMA:)
Agreed.
Quoted 37 lines
Quoted 6 lines
#ifdef
CONFIG_SLAB_OBJ_EXT
boolis_kmalloc_no_obj_ext_type(type){ return type ==KMALLOC_NO_OBJ_EXT; } #else
boolis_kmalloc_no_obj_ext_type(type){ return false; }
#endif
?
is_kmalloc_no_obj_ext_type(),kmalloc_type_is_no_obj_ext(),is_kmalloc_type_no_obj_ext(),
...naming is hard, ugh :)
Quoted 5 lines
mm/slab_common.c new_kmalloc_cache(int idx, enum kmalloc_cache_type type) · line 973
Quoted 3 lines
mm/slab_common.c +2
-0+if (!need_kmalloc_no_objext ()) { + kmalloc_caches[type][idx] = kmalloc_caches[KMALLOC_NORMAL ][idx];Could kmalloc_caches[KMALLOC_NORMAL][idx] be NULL here?
No.
KMALLOC_NORMALcaches are created before all other kmalloc caches.IIRC checking if kmalloc_caches[KMALLOC_NORMAL][idx] is NULL was added by commit
963e84b0f262("mm/slab: limitkmalloc()minimum alignment todma_get_cache_alignment()") to avoid creating kmalloc caches of same type and size due to minimum alignment.Quoted 8 lines
In general why do we special-case and do an early exit here?
Can we do instead:
if (
need_kmalloc_no_objext())flags |= SLAB_NO_OBJ_EXT | SLAB_NO_MERGE;and use the common path?
Quoted 8 lines
Hmm, but even without
SLAB_NO_MERGE, we often end up not merging kmalloc caches e.g.) because of non-zeros->usersize.I think that's why we do special-case and an early exit?
We could probably do some refactoring to change that, but in general I'm afraid of backporting refactoring work to -stable because I fear introducing very subtle behavioral changes that nobody would notice.
Agreed, let's keep hotfixes as minimal as possible and refactor later.
Quoted 37 lines
Quoted 17 lines
Quoted 17 lines
mm/slab_common.c +3
-0+return ; + } + flags |=SLAB_NO_OBJ_EXT |SLAB_NO_MERGE ; }else if (IS_ENABLED (CONFIG_ZONE_DMA ) && (type ==KMALLOC_DMA )) { flags |=SLAB_CACHE_DMA ; }mm/slub.c +1
-1@@ -2168,14 +2132,20 @@ int alloc_slab_obj_exts(struct slab *slab, struct kmem_cache *s,unsigned long new_exts;unsigned long old_exts;struct slabobj_ext *vec; -size_t sz; +size_t sz =sizeof (struct slabobj_ext ) * slab->objects;Quoted 13 lines
mm/slub.c int alloc_slab_obj_exts(struct slab *slab, struct kmem_cache *s, · line 2141
Quoted 8 lines
mm/slub.c +4
-3gfp &= ~OBJCGS_CLEAR_MASK ; -/* Prevent recursive extension vector allocation */ - alloc_flags |=SLAB_ALLOC_NO_RECURSE ; - alloc_flags &= ~SLAB_ALLOC_NEW_SLAB ; +/* +* In most cases, obj_exts arrays are allocated from normal kmalloc. +* However, normal kmalloc caches must allocate them from +* KMALLOC_NO_OBJ_EXT caches to prevent recursion. For debugging it would have been convenient to allocate all
obj_extvectors from dedicated caches... Maybe we can do that forCONFIG_DEBUG_VMor someday when we addCONFIG_OBJ_EXT_DEBUG? Anyway, not really a complaint but a wish.Vlastimil and I had a conversation on always (even w/o debug options) having dedicated caches (primarily to separate lifetime and for simplicity), it would be interesting to explore.
Re: [PATCH slab/for-next-fixes v3 4/4] mm/slab: prevent unbounded recursion in free path with new kmalloc type
Quoted 109 lines · Vlastimil Babka (SUSE)
Quoted 22 lines · Harry Yoo
Quoted 14 lines · Suren Baghdasaryan
mm/slab.h static inline unsigned int size_index_elem(unsigned int bytes) · line 397
Quoted 13 lines · Harry Yoo (Oracle)
mm/slab.h +6
-1@@ -386,12 +387,17 @@ 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); + +if (alloc_flags &SLAB_ALLOC_NO_OBJ_EXT ) + type =KMALLOC_NO_OBJ_EXT ;Hi Suren, thanks for the reviews.
It's indeed helpful to have an eye for those bugfixes.Quoted 1 line
Why not let
kmalloc_type()handlealloc_flags?Quoted 8 lines
Good point!
Quoted 3 lines
Other users (there are
only 4 of them) can passSLAB_ALLOC_DEFAULT. That seems cleaner to me and more robust.Hmm, there was a reason... *checks notes*, oh, there is no note. IIRC I was afraid of exposing SLAB_ALLOC_* flags to arbitrary users.
Hm good point, they are in
mm/slab.handkmalloc_type()needs to be in the publicinclude/linux/slab.h. Given its usage in__builtin_constant_p(size)paths of kmalloc[_node] I would rather not complicate it (even if a hardcodedSLAB_ALLOC_DEFAULTshould eliminate the extra code). So I'd rather keepKMALLOC_NO_OBJ_EXThandling hidden inmm/as long as it's possible.Quoted 23 lines
should probably fine as long as it's not used in
kmalloc/kmem_cache_alloc() APIs, not sure.Quoted 11 lines
Quoted 11 lines
mm/slab.h +1
-1if (!b) - b = &kmalloc_caches[kmalloc_type (flags, token)]; + b = &kmalloc_caches[type];if (size <=192 ) index = kmalloc_size_index[size_index_elem (size)];else mm/slab_common.c +0
-0@@ -957,6 +968,12 @@ new_kmalloc_cache(int idx, enum kmalloc_cache_type type)Quoted 8 lines
mm/slab_common.c new_kmalloc_cache(int idx, enum kmalloc_cache_type type) · line 971
Quoted 4 lines
mm/slab_common.c +1
-0return ; } flags |=SLAB_ACCOUNT ; + }else if (IS_ENABLED (CONFIG_SLAB_OBJ_EXT ) && type ==KMALLOC_NO_OBJ_EXT ) {Hmm, you have to check
IS_ENABLED(CONFIG_SLAB_OBJ_EXT)here becauseKMALLOC_NO_OBJ_EXTcan be aliased withKMALLOC_NORMAL... Could we instead have a helper function like this (maybe with a better name):Quoted 3 lines
Hmm that's fine, but I think that bit should not be part of -stable fixes at least. Here I tried to make it consistent with
KMALLOC_RECLAIMandKMALLOC_DMA:)Agreed.
Quoted 40 lines
Quoted 6 lines
#ifdef
CONFIG_SLAB_OBJ_EXT
boolis_kmalloc_no_obj_ext_type(type){ return type ==KMALLOC_NO_OBJ_EXT; } #else
boolis_kmalloc_no_obj_ext_type(type){ return false; }
#endif
?
is_kmalloc_no_obj_ext_type(),kmalloc_type_is_no_obj_ext(),is_kmalloc_type_no_obj_ext(),
...naming is hard, ugh :)
Quoted 5 lines
mm/slab_common.c new_kmalloc_cache(int idx, enum kmalloc_cache_type type) · line 973
Quoted 3 lines
mm/slab_common.c +2
-0+if (!need_kmalloc_no_objext ()) { + kmalloc_caches[type][idx] = kmalloc_caches[KMALLOC_NORMAL ][idx];Could kmalloc_caches[KMALLOC_NORMAL][idx] be NULL here?
No.
KMALLOC_NORMALcaches are created before all other kmalloc caches.IIRC checking if kmalloc_caches[KMALLOC_NORMAL][idx] is NULL was added by commit
963e84b0f262("mm/slab: limitkmalloc()minimum alignment todma_get_cache_alignment()") to avoid creating kmalloc caches of same type and size due to minimum alignment.Quoted 8 lines
In general why do we special-case and do an early exit here?
Can we do instead:
if (
need_kmalloc_no_objext())flags |= SLAB_NO_OBJ_EXT | SLAB_NO_MERGE;and use the common path?
Hmm, but even without
SLAB_NO_MERGE, we often end up not merging kmalloc caches e.g.) because of non-zeros->usersize.
Quoted 7 lines
Quoted 5 lines
I think that's why we do special-case and an early exit?
We could probably do some refactoring to change that, but in general I'm afraid of backporting refactoring work to -stable because I fear introducing very subtle behavioral changes that nobody would notice.
Agreed, let's keep hotfixes as minimal as possible and refactor later.
Sounds good. Feel free to add:
Quoted 39 lines
Quoted 38 lines
Quoted 18 lines
Quoted 18 lines
mm/slab_common.c +3
-0+return ; + } + flags |=SLAB_NO_OBJ_EXT |SLAB_NO_MERGE ; }else if (IS_ENABLED (CONFIG_ZONE_DMA ) && (type ==KMALLOC_DMA )) { flags |=SLAB_CACHE_DMA ; }mm/slub.c +1
-1@@ -2168,14 +2132,20 @@ int alloc_slab_obj_exts(struct slab *slab, struct kmem_cache *s,unsigned long new_exts;unsigned long old_exts;struct slabobj_ext *vec; -size_t sz; +size_t sz =sizeof (struct slabobj_ext ) * slab->objects;Quoted 13 lines
mm/slub.c int alloc_slab_obj_exts(struct slab *slab, struct kmem_cache *s, · line 2141
Quoted 8 lines
mm/slub.c +4
-3gfp &= ~OBJCGS_CLEAR_MASK ; -/* Prevent recursive extension vector allocation */ - alloc_flags |=SLAB_ALLOC_NO_RECURSE ; - alloc_flags &= ~SLAB_ALLOC_NEW_SLAB ; +/* +* In most cases, obj_exts arrays are allocated from normal kmalloc. +* However, normal kmalloc caches must allocate them from +* KMALLOC_NO_OBJ_EXT caches to prevent recursion. For debugging it would have been convenient to allocate all
obj_extvectors from dedicated caches... Maybe we can do that forCONFIG_DEBUG_VMor someday when we addCONFIG_OBJ_EXT_DEBUG? Anyway, not really a complaint but a wish.Vlastimil and I had a conversation on always (even w/o debug options) having dedicated caches (primarily to separate lifetime and for simplicity), it would be interesting to explore.
Vlastimil Babka (SUSE)
maintainerRe: [PATCH slab/for-next-fixes v3 4/4] mm/slab: prevent unbounded recursion in free path with new kmalloc type
In reply to Harry Yoo (Oracle)Quoted 8 lines · Harry Yoo (Oracle)
mm/slub.c +1
-1@@ -2168,14 +2132,20 @@ int alloc_slab_obj_exts(struct slab *slab, struct kmem_cache *s,unsigned long new_exts;unsigned long old_exts;struct slabobj_ext *vec; -size_t sz; +size_t sz =sizeof (struct slabobj_ext ) * slab->objects; gfp &= ~OBJCGS_CLEAR_MASK ;
mm/slub.c int alloc_slab_obj_exts(struct slab *slab, struct kmem_cache *s, · line 2144
Quoted 10 lines
mm/slub.c +7
-3-/* Prevent recursive extension vector allocation */ - alloc_flags |=SLAB_ALLOC_NO_RECURSE ; - alloc_flags &= ~SLAB_ALLOC_NEW_SLAB ; +/* +* In most cases, obj_exts arrays are allocated from normal kmalloc. +* However, normal kmalloc caches must allocate them from +* KMALLOC_NO_OBJ_EXT caches to prevent recursion. +*/ +if (is_kmalloc_normal (s)) { +VM_WARN_ON_ONCE (!need_kmalloc_no_objext ());
As discussed off-list, this can false positively trigger due to a race with shutdown_mem_profiling() so I'll remove it.
Quoted 8 lines
mm/slub.c +3
-1+ alloc_flags |=SLAB_ALLOC_NO_OBJ_EXT ; + } - sz =obj_exts_alloc_size (s, slab, gfp); + alloc_flags &= ~SLAB_ALLOC_NEW_SLAB ;/* This will use kmalloc_nolock() if alloc_flags say so */ vec =kmalloc_flags (sz, gfp | __GFP_ZERO, alloc_flags,slab_nid (slab));
Vlastimil Babka (SUSE)
maintainerQuoted 44 lines · Harry Yoo (Oracle)
This is a follow-up fix after the recent discussion [1]. See patch 4 for the detailed description on the bug.
Based on slab/for-next-fixes (
af9ea231c0b45) and is available at git.kernel.org [2].Instead preventing cycles by bumping up the allocation size of
obj_extsarrays, it introduces a new kmalloc type calledKMALLOC_NO_OBJ_EXTand disallow formation of cycles between kmalloc types when allocatingobj_extsarrays.obj_extsarrays of normal kmalloc caches are served fromKMALLOC_NO_OBJ_EXTcaches (that don't haveobj_exts), and all otherobj_extsarrays are served from normal kmalloc caches.I tried to reuse
SLAB_ALLOC_NO_RECURSEto makekmalloc_slab()selectKMALLOC_NO_OBJ_EXT, but it was not great because it does not allow sheaves for those caches. So I introduced a new slab alloc flagSLAB_ALLOC_NO_OBJ_EXT.To avoid huge confusion, I had to decouple "disallowing sheaves" semantics from
SLAB_NO_OBJ_EXTand introducedSLAB_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_EXTto selectKMALLOC_NO_OBJ_EXTas 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-v3r1
To: Vlastimil Babka <[email protected]>
To: Andrew Morton <[email protected]>
To: Hao Li <[email protected]>
To: Christoph Lameter <[email protected]>
To: David Rientjes <[email protected]>
To: Roman Gushchin <[email protected]>
To: Suren Baghdasaryan <[email protected]>
To: Hao Ge <[email protected]>
To: Kees Cook <[email protected]>
To: Pedro Falcato <[email protected]>
To: Shakeel Butt <[email protected]>
To: Danielle Constantino <[email protected]>
To: Liam R. Howlett <[email protected]>
Quoted 3 lines
Added to slab/for-next-fixes, thanks!
I've removed Fixes: and Cc: stable from patches 2 and 3 as I don't believe it's the way to handle the dependencies.
Harry Yoo
maintainerQuoted 3 lines · Vlastimil Babka (SUSE)
On 7/13/26 16:28, Harry Yoo (Oracle) wrote:
Added to slab/for-next-fixes, thanks!
Thanks a lot.
Huge thanks to Vlastimil and Suren for the reviews!
Quoted 2 lines
I've removed Fixes: and Cc: stable from patches 2 and 3 as I don't believe it's the way to handle the dependencies.
Ack!
*keeps learning how -stable works*
Signature · 2 lines
Cheers, Harry / Hyeonggon
Quoted 18 lines · Harry Yoo (Oracle)
This is a follow-up fix after the recent discussion [1]. See patch 4 for the detailed description on the bug.
Based on slab/for-next-fixes (
af9ea231c0b45) and is available at git.kernel.org [2].Instead preventing cycles by bumping up the allocation size of
obj_extsarrays, it introduces a new kmalloc type calledKMALLOC_NO_OBJ_EXTand disallow formation of cycles between kmalloc types when allocatingobj_extsarrays.obj_extsarrays of normal kmalloc caches are served fromKMALLOC_NO_OBJ_EXTcaches (that don't haveobj_exts), and all otherobj_extsarrays are served from normal kmalloc caches.I tried to reuse
SLAB_ALLOC_NO_RECURSEto makekmalloc_slab()selectKMALLOC_NO_OBJ_EXT, but it was not great because it does not allow sheaves for those caches. So I introduced a new slab alloc flagSLAB_ALLOC_NO_OBJ_EXT.
Quoted 8 lines
To avoid huge confusion, I had to decouple "disallowing sheaves" semantics from
SLAB_NO_OBJ_EXTand introducedSLAB_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_EXTto selectKMALLOC_NO_OBJ_EXTas kmalloc caches don't have sheaves in v6.18 anyway.
Hi Harry, are you planning to send the backports to stable once this merges into linus tree?
Harry Yoo
maintainerQuoted 22 lines · Shakeel Butt
Quoted 21 lines · Harry Yoo (Oracle)
This is a follow-up fix after the recent discussion [1]. See patch 4 for the detailed description on the bug.
Based on slab/for-next-fixes (
af9ea231c0b45) and is available at git.kernel.org [2].Instead preventing cycles by bumping up the allocation size of
obj_extsarrays, it introduces a new kmalloc type calledKMALLOC_NO_OBJ_EXTand disallow formation of cycles between kmalloc types when allocatingobj_extsarrays.obj_extsarrays of normal kmalloc caches are served fromKMALLOC_NO_OBJ_EXTcaches (that don't haveobj_exts), and all otherobj_extsarrays are served from normal kmalloc caches.I tried to reuse
SLAB_ALLOC_NO_RECURSEto makekmalloc_slab()selectKMALLOC_NO_OBJ_EXT, but it was not great because it does not allow sheaves for those caches. So I introduced a new slab alloc flagSLAB_ALLOC_NO_OBJ_EXT.To avoid huge confusion, I had to decouple "disallowing sheaves" semantics from
SLAB_NO_OBJ_EXTand introducedSLAB_NO_SHEAVES.
Quoted 7 lines
Quoted 5 lines
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_EXTto selectKMALLOC_NO_OBJ_EXTas kmalloc caches don't have sheaves in v6.18 anyway.Hi Harry,
Hi Shakeel,
Quoted 2 lines
are you planning to send the backports to stable once this merges into linus tree?
If somebody wants to give it a try, that would be great! Otherwise I will send backports although it might take some time as I'll be working on and off for a while :)
Signature · 2 lines
Cheers, Harry / Hyeonggon
Quoted 44 lines · Harry Yoo (Oracle)
This is a follow-up fix after the recent discussion [1]. See patch 4 for the detailed description on the bug.
Based on slab/for-next-fixes (
af9ea231c0b45) and is available at git.kernel.org [2].Instead preventing cycles by bumping up the allocation size of
obj_extsarrays, it introduces a new kmalloc type calledKMALLOC_NO_OBJ_EXTand disallow formation of cycles between kmalloc types when allocatingobj_extsarrays.obj_extsarrays of normal kmalloc caches are served fromKMALLOC_NO_OBJ_EXTcaches (that don't haveobj_exts), and all otherobj_extsarrays are served from normal kmalloc caches.I tried to reuse
SLAB_ALLOC_NO_RECURSEto makekmalloc_slab()selectKMALLOC_NO_OBJ_EXT, but it was not great because it does not allow sheaves for those caches. So I introduced a new slab alloc flagSLAB_ALLOC_NO_OBJ_EXT.To avoid huge confusion, I had to decouple "disallowing sheaves" semantics from
SLAB_NO_OBJ_EXTand introducedSLAB_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_EXTto selectKMALLOC_NO_OBJ_EXTas 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-v3r1
To: Vlastimil Babka <[email protected]>
To: Andrew Morton <[email protected]>
To: Hao Li <[email protected]>
To: Christoph Lameter <[email protected]>
To: David Rientjes <[email protected]>
To: Roman Gushchin <[email protected]>
To: Suren Baghdasaryan <[email protected]>
To: Hao Ge <[email protected]>
To: Kees Cook <[email protected]>
To: Pedro Falcato <[email protected]>
To: Shakeel Butt <[email protected]>
To: Danielle Constantino <[email protected]>
To: Liam R. Howlett <[email protected]>
Quoted 3 lines
I tested next-20260716 which has this series against next-20260707 which does not have the fix. The reproducer were able to trigger the leak/recursion on next-20260707 but not on next-20260716. So, you can add:
Hao Ge
reviewerQuoted 48 lines · Shakeel Butt
Quoted 44 lines · Harry Yoo (Oracle)
This is a follow-up fix after the recent discussion [1]. See patch 4 for the detailed description on the bug.
Based on slab/for-next-fixes (
af9ea231c0b45) and is available at git.kernel.org [2].Instead preventing cycles by bumping up the allocation size of
obj_extsarrays, it introduces a new kmalloc type calledKMALLOC_NO_OBJ_EXTand disallow formation of cycles between kmalloc types when allocatingobj_extsarrays.obj_extsarrays of normal kmalloc caches are served fromKMALLOC_NO_OBJ_EXTcaches (that don't haveobj_exts), and all otherobj_extsarrays are served from normal kmalloc caches.I tried to reuse
SLAB_ALLOC_NO_RECURSEto makekmalloc_slab()selectKMALLOC_NO_OBJ_EXT, but it was not great because it does not allow sheaves for those caches. So I introduced a new slab alloc flagSLAB_ALLOC_NO_OBJ_EXT.To avoid huge confusion, I had to decouple "disallowing sheaves" semantics from
SLAB_NO_OBJ_EXTand introducedSLAB_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_EXTto selectKMALLOC_NO_OBJ_EXTas 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-v3r1
To: Vlastimil Babka <[email protected]>
To: Andrew Morton <[email protected]>
To: Hao Li <[email protected]>
To: Christoph Lameter <[email protected]>
To: David Rientjes <[email protected]>
To: Roman Gushchin <[email protected]>
To: Suren Baghdasaryan <[email protected]>
To: Hao Ge <[email protected]>
To: Kees Cook <[email protected]>
To: Pedro Falcato <[email protected]>
To: Shakeel Butt <[email protected]>
To: Danielle Constantino <[email protected]>
To: Liam R. Howlett <[email protected]>Quoted 3 lines
Quoted 3 lines
I tested next-20260716 which has this series against next-20260707 which does not have the fix. The reproducer were able to trigger the leak/recursion on next-20260707 but not on next-20260716. So, you can add:
Hi Shakeel
It seems you have a consistent stable reproduction method or test script.
Would you mind sharing it if possible? I'm sorry if this was provided earlier and I overlooked it.
I'd like to thoroughly investigate and learn about this problem.
Thank you very much.
Thanks
Best Regards
Hao
Quoted 3 lines
Tested-by Shakeel Butt <[email protected]>
Quoted 57 lines · Hao Ge
Quoted 48 lines · Shakeel Butt
Quoted 44 lines · Harry Yoo (Oracle)
This is a follow-up fix after the recent discussion [1]. See patch 4 for the detailed description on the bug.
Based on slab/for-next-fixes (
af9ea231c0b45) and is available at git.kernel.org [2].Instead preventing cycles by bumping up the allocation size of
obj_extsarrays, it introduces a new kmalloc type calledKMALLOC_NO_OBJ_EXTand disallow formation of cycles between kmalloc types when allocatingobj_extsarrays.obj_extsarrays of normal kmalloc caches are served fromKMALLOC_NO_OBJ_EXTcaches (that don't haveobj_exts), and all otherobj_extsarrays are served from normal kmalloc caches.I tried to reuse
SLAB_ALLOC_NO_RECURSEto makekmalloc_slab()selectKMALLOC_NO_OBJ_EXT, but it was not great because it does not allow sheaves for those caches. So I introduced a new slab alloc flagSLAB_ALLOC_NO_OBJ_EXT.To avoid huge confusion, I had to decouple "disallowing sheaves" semantics from
SLAB_NO_OBJ_EXTand introducedSLAB_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_EXTto selectKMALLOC_NO_OBJ_EXTas 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-v3r1
To: Vlastimil Babka <[email protected]>
To: Andrew Morton <[email protected]>
To: Hao Li <[email protected]>
To: Christoph Lameter <[email protected]>
To: David Rientjes <[email protected]>
To: Roman Gushchin <[email protected]>
To: Suren Baghdasaryan <[email protected]>
To: Hao Ge <[email protected]>
To: Kees Cook <[email protected]>
To: Pedro Falcato <[email protected]>
To: Shakeel Butt <[email protected]>
To: Danielle Constantino <[email protected]>
To: Liam R. Howlett <[email protected]>Quoted 3 lines
Quoted 3 lines
I tested next-20260716 which has this series against next-20260707 which does not have the fix. The reproducer were able to trigger the leak/recursion on next-20260707 but not on next-20260716. So, you can add:
Hi Shakeel
Quoted 10 lines
It seems you have a consistent stable reproduction method or test script.
Would you mind sharing it if possible? I'm sorry if this was provided earlier and I overlooked it.
I'd like to thoroughly investigate and learn about this problem.
Thank you very much.
From dcb6ca5c051738d64f7a901402f5f257babbc385 Mon Sep 17 00:00:00 2001
From: Shakeel Butt <[email protected]>
Date: Fri, 17 Jul 2026 08:12:11 -0700
Subject: [PATCH] slub: repro unbounded recursion in slub objext array
allocation
--- repro_public/Makefile | 1 + repro_public/README.md | 128 +++++++++++++++++++++++++ repro_public/repro_objext.c | 180 ++++++++++++++++++++++++++++++++++++ repro_public/run_repro.sh | 26 ++++++ 4 files changed, 335 insertions(+) create mode 100644 repro_public/Makefile create mode 100644 repro_public/README.md create mode 100644 repro_public/repro_objext.c create mode 100755 repro_public/run_repro.sh
repro_public/Makefile new file +1 -0
@@ -0,0 +1 @@ +obj-m := repro_objext.o
repro_public/README.md new file +128 -0
128 lines not on this page · raw message
repro_public/repro_objext.c new file +180 -0
180 lines not on this page · raw message
repro_public/run_repro.sh new file +26 -0
@@ -0,0 +1,26 @@ +#!/bin/bash +# Branch reproducer driver. Arg1 = path to repro_objext.ko built against this kernel. +# All diagnostics go to /dev/kmsg so they survive in the dmesg dump (vng stdout is lossy). +set +e +KO="$1" +L(){ echo "BT| $*" > /dev/kmsg 2>/dev/null; } +k(){ awk '{print $1}' /sys/kernel/slab/$1/objects 2>/dev/null; } +g(){ cat /sys/kernel/slab/$1/$2 2>/dev/null; } +L "===== BRANCHTEST START =====" +L "uname=$(uname -r)" +L "cmdline=$(cat /proc/cmdline)" +echo 1 > /proc/sys/vm/mem_profiling 2>/dev/null +L "profiling_runtime=$(cat /proc/sys/vm/mem_profiling 2>/dev/null)" +L "GEOM k512 objs=$(g kmalloc-512 objs_per_slab) ord=$(g kmalloc-512 order) | k1k objs=$(g kmalloc-1k objs_per_slab) ord=$(g kmalloc-1k order) | k2k objs=$(g kmalloc-2k objs_per_slab) | k256 objs=$(g kmalloc-256 objs_per_slab)" +L "noobjext_caches=$(ls -d /sys/kernel/slab/*no-objext* 2>/dev/null | wc -l)" +L "BEFORE k512=$(k kmalloc-512) k1k=$(k kmalloc-1k)" +insmod "$KO" n=2000000 rounds=4 shrink=1 keep=0 shrink_iters=8 +L "insmod_rc=$?" +# extra external shrink storm to force the discard cascade +for i in $(seq 1 80); do for c in kmalloc-128 kmalloc-256 kmalloc-512 kmalloc-1k kmalloc-2k kmalloc-4k; do echo 1 > /sys/kernel/slab/$c/shrink 2>/dev/null; done; done +sleep 1 +L "AFTER k512=$(k kmalloc-512) k1k=$(k kmalloc-1k)" +L "===== BRANCHTEST END =====" +echo "=====DMESG DUMP=====" +dmesg | grep -E "BT\||REPRODUCER|stack guard|BUG:|Oops|Call Trace|Kernel panic|guard page" +echo "=====BRANCHTEST_DONE====="
Signature · 1 line
2.53.0-Meta
Hao Ge
reviewerHi Shakeel
Quoted 435 lines · Shakeel Butt
Quoted 56 lines · Hao Ge
Quoted 48 lines · Shakeel Butt
Quoted 44 lines · Harry Yoo (Oracle)
This is a follow-up fix after the recent discussion [1]. See patch 4 for the detailed description on the bug.
Based on slab/for-next-fixes (
af9ea231c0b45) and is available at git.kernel.org [2].Instead preventing cycles by bumping up the allocation size of
obj_extsarrays, it introduces a new kmalloc type calledKMALLOC_NO_OBJ_EXTand disallow formation of cycles between kmalloc types when allocatingobj_extsarrays.obj_extsarrays of normal kmalloc caches are served fromKMALLOC_NO_OBJ_EXTcaches (that don't haveobj_exts), and all otherobj_extsarrays are served from normal kmalloc caches.I tried to reuse
SLAB_ALLOC_NO_RECURSEto makekmalloc_slab()selectKMALLOC_NO_OBJ_EXT, but it was not great because it does not allow sheaves for those caches. So I introduced a new slab alloc flagSLAB_ALLOC_NO_OBJ_EXT.To avoid huge confusion, I had to decouple "disallowing sheaves" semantics from
SLAB_NO_OBJ_EXTand introducedSLAB_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_EXTto selectKMALLOC_NO_OBJ_EXTas 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-v3r1
To: Vlastimil Babka <[email protected]>
To: Andrew Morton <[email protected]>
To: Hao Li <[email protected]>
To: Christoph Lameter <[email protected]>
To: David Rientjes <[email protected]>
To: Roman Gushchin <[email protected]>
To: Suren Baghdasaryan <[email protected]>
To: Hao Ge <[email protected]>
To: Kees Cook <[email protected]>
To: Pedro Falcato <[email protected]>
To: Shakeel Butt <[email protected]>
To: Danielle Constantino <[email protected]>
To: Liam R. Howlett <[email protected]>Quoted 3 lines
Quoted 3 lines
I tested next-20260716 which has this series against next-20260707 which does not have the fix. The reproducer were able to trigger the leak/recursion on next-20260707 but not on next-20260716. So, you can add:
Hi Shakeel
Quoted 10 lines
It seems you have a consistent stable reproduction method or test script.
Would you mind sharing it if possible? I'm sorry if this was provided earlier and I overlooked it.
I'd like to thoroughly investigate and learn about this problem.
Thank you very much.
From
dcb6ca5c051738d64f7a901402f5f257babbc385Mon Sep 17 00:00:00 2001
From: Shakeel Butt <[email protected]>
Date: Fri, 17 Jul 2026 08:12:11 -0700
Subject: [PATCH] slub: repro unbounded recursion in slub objext array
allocationSigned-off-by Shakeel Butt <[email protected]>--- repro_public/Makefile | 1 + repro_public/README.md | 128 +++++++++++++++++++++++++ repro_public/repro_objext.c | 180 ++++++++++++++++++++++++++++++++++++ repro_public/run_repro.sh | 26 ++++++ 4 files changed, 335 insertions(+)create mode 100644 repro_public/Makefile
create mode 100644repro_public/README.md
create mode 100644repro_public/repro_objext.c
create mode 100755repro_public/run_repro.shrepro_public/Makefile new file +1
-0@@ -0,0 +1 @@ +obj-m := repro_objext.orepro_public/README.md new file +128
-0@@ -0,0 +1,128 @@ +# Reproducer: SLUB obj_exts free-recursion / cross-cycle leak + +With `CONFIG_MEM_ALLOC_PROFILING=y` and profiling enabled, every +`KMALLOC_NORMAL` slab gets a `slabobj_ext` (obj_exts) array that is itself +`kmalloc()`'d from a `KMALLOC_NORMAL` cache. `sizeof(struct slabobj_ext) == 16`, +so with the right slab geometry the arrays reference each other: + +- `kmalloc-512` with 64 objs/slab -> array is `64*16 = 1024` bytes -> `kmalloc-1k` +- `kmalloc-1k` with 32 objs/slab -> array is `32*16 = 512` bytes -> `kmalloc-512` + +A `kmalloc-512` slab and a `kmalloc-1k` slab then hold each other's obj_exts +array (a 2-cycle). Discarding one frees the other's array, which empties and +discards that slab, and so on: + +``` +__free_slab() -> unaccount_slab() -> free_slab_obj_exts() -> kfree() + -> discard_slab() -> __free_slab() ... +``` + +At production scale this recurses until the 16 KiB kernel stack overflows +(stack guard page). In a small VM it deterministically manifests as a *leak*: +the mutually-pinned slabs can never be reclaimed, so `kmalloc-512`/`kmalloc-1k` +retain thousands of unreclaimable objects after a shrink storm. Same root cause. + +Related upstream commits: +- `4b8736964640` ("mm/slab: add allocation accounting into slab allocation and + free paths") — introduced the obj_exts arrays (Fixes:). +- `280ea9c3154b` ("mm/slab: avoid allocating slabobj_ext array from its own + slab") — bumps the size only when the array would come from the *same* cache + (`object_size ==`); does NOT break the `kmalloc-512 <-> kmalloc-1k` cross + cycle, so it is insufficient. + +## Files + +- `repro_objext.c` — the reproducer kernel module. +- `Makefile` — out-of-tree module makefile (`obj-m := repro_objext.o`). +- `run_repro.sh` — in-guest driver: enables profiling, prints the slab + geometry, insmods the module, hammers `shrink`, and reports the leftover + object counts (all via `/dev/kmsg` so it survives in `dmesg`). + +## Requirements + +Kernel `.config`: + +``` +CONFIG_MEM_ALLOC_PROFILING=y # trigger: obj_exts on KMALLOC_NORMAL slabs +CONFIG_MEMCG=y # selects CONFIG_SLAB_OBJ_EXT=y +CONFIG_MODULES=y +CONFIG_MODULE_UNLOAD=y +# CONFIG_SLUB is the default allocator +``` + +Tools: a kernel build toolchain (`make`, `gcc`/clang, `binutils`, `bison`, +`flex`, `libelf-dev`, `libssl-dev`, `bc`, `pahole`) and either +[virtme-ng](https://github.com/arighi/virtme-ng) or `qemu-system-x86_64`. + +## Build + +```sh +# 1. Build the kernel under test (with the config options above). +make -C /path/to/kernel -j"$(nproc)" bzImage modules +# ('modules' is needed so Module.symvers includes mem_alloc_profiling_key, +# which kmalloc() references when profiling is compiled in.) + +# 2. Build the reproducer module against that kernel. +make -C /path/to/kernel M="$PWD" modules # produces repro_objext.ko +``` + +## Run + +Boot the kernel with two boot-cmdline parameters: + +``` +slub_min_objects=64 # forces kmalloc-512=64 objs/slab and + # kmalloc-1k=32 objs/slab -> the cross-cycle geometry +sysctl.vm.mem_profiling=1 # enables profiling at boot; a runtime + # `echo 1 > /proc/sys/vm/mem_profiling` is REJECTED + # unless CONFIG_MEM_ALLOC_PROFILING_ENABLED_BY_DEFAULT=y + # or this boot param is set. +``` + +With virtme-ng: + +```sh +cd /path/to/kernel +vng --cpus 8 -m 12G -a "slub_min_objects=64 sysctl.vm.mem_profiling=1" \ + -- bash /path/to/run_repro.sh /path/to/repro_objext.ko +``` + +Or, by hand inside any guest booted with the params above: + +```sh +insmod repro_objext.ko n=2000000 rounds=4 shrink=1 keep=0 shrink_iters=8 +for i in $(seq 1 80); do + for c in kmalloc-128 kmalloc-256 kmalloc-512 kmalloc-1k kmalloc-2k kmalloc-4k; do + echo 1 > /sys/kernel/slab/$c/shrink 2>/dev/null + done +done +cat /sys/kernel/slab/kmalloc-512/objects +cat /sys/kernel/slab/kmalloc-1k/objects +``` + +The module returns `-ECANCELED` on purpose (it does all its work in init and +then unloads itself); the `insmod: Operation canceled` message is expected. + +## Expected results + +Verify the geometry printed by the driver is `k512 objs=64` and `k1k objs=32` +and `profiling_runtime=1`; otherwise the cross-cycle is not exercised. + +- **Buggy kernel** (cross-cycle not fixed): after the shrink storm, + `kmalloc-512`/`kmalloc-1k` stay pinned at thousands of objects (e.g. ~8000), + far above the pre-insmod baseline. At production scale/geometry the same + condition overflows the kernel stack instead. +- **Fixed kernel**: object counts return to the pre-insmod baseline (a few + hundred); no leak, no recursion. + +## Module parameters + +| param | default | meaning | +|----------------|-----------|-----------------------------------------------------------| +| `n` | 2000000 | objects per size class per round | +| `rounds` | 4 | warm-up rounds to build obj_exts density | +| `keep` | 0 | keep 1-in-`keep` objects live per round (0 = free all) | +| `shrink` | 1 | hammer `kmem_cache_shrink` via sysfs after the churn | +| `shrink_iters` | 8 | shrink passes per round | +| `size_lo` | 384 | small request size (-> kmalloc-512) | +| `size_hi` | 768 | large request size (-> kmalloc-1k) |repro_public/repro_objext.c new file +180
-0@@ -0,0 +1,180 @@ +// SPDX-License-Identifier: GPL-2.0 +/* +* repro_objext.c — reproduce the SLUB obj_exts free-recursion stack overflow. +* +* Bug: with CONFIG_MEM_ALLOC_PROFILING=y and vm.mem_profiling on, +* every KMALLOC_NORMAL slab gets a slabobj_ext (obj_exts) vector allocated from +* another KMALLOC_NORMAL slab. sizeof(slabobj_ext)==16, so kmalloc-1k's vector +* (objs*16) lands in kmalloc-512 and kmalloc-512's vector lands in kmalloc-1k -> +* cross-referencing chains and a kmalloc-1k<->kmalloc-512 2-cycle. When many such +* slabs are discarded (per-cpu partial flush __put_partials, or kmem_cache_shrink), +* __free_slab()->unaccount_slab()->free_slab_obj_exts()->kfree(vector) empties the +* slab that holds the vector, which discards THAT slab, recursing without bound and +* overflowing the 16 KiB task stack (VMAP guard page). +* +* Boot: append `sysctl.vm.mem_profiling=1` (or =1,compressed) to the kernel cmdline, +* OR echo 1 > /proc/sys/vm/mem_profiling before insmod. +* +* WITHOUT fix: "BUG: TASK stack guard page was hit" recursion -> panic/hang/reboot; +* "REPRODUCER DONE" is NEVER printed (note: the overflow may also +* corrupt the console TX path, so the box may just die silently — +* treat a VM that never prints DONE / dies / reboots as a CRASH). +* WITH fix: no obj_exts on KMALLOC_NORMAL slabs -> no recursion -> +* "REPRODUCER DONE no-crash" printed, insmod returns (module unloads). +* +* Tunables (module params): +* n objects per size class per round (default 2,000,000) +* rounds warm-up rounds to build obj_exts density (default 4) +* keep keep 1-in-`keep` objects live after each round to leave slabs at low +* occupancy (denser cross-referenced partial slabs). 0 = free all. +* shrink after the churn, hammer kmem_cache_shrink via +* /sys/kernel/slab/<cache>/shrink to force the discard cascade. +* sizes_lo/sizes_hi the two kmalloc request sizes (default 768 -> kmalloc-1k, +* 384 -> kmalloc-512). +*/ +#include <linux/module.h> +#include <linux/slab.h> +#include <linux/vmalloc.h> +#include <linux/kernel.h> +#include <linux/fs.h> +#include <linux/uaccess.h> +#include <linux/delay.h> + +static unsigned long n =2000000 ; +module_param (n, ulong,0444 ); +MODULE_PARM_DESC (n,"objects per size class per round" ); + +static unsigned int rounds =4 ; +module_param (rounds, uint,0444 ); +MODULE_PARM_DESC (rounds,"warm-up rounds" ); + +static unsigned int keep; +module_param (keep, uint,0444 ); +MODULE_PARM_DESC (keep,"keep 1-in-keep objects live after each round (0=free all)" ); + +static unsigned int shrink =1 ; +module_param (shrink, uint,0444 ); +MODULE_PARM_DESC (shrink,"hammer kmem_cache_shrink via sysfs after churn" ); + +static unsigned int shrink_iters =8 ; +module_param (shrink_iters, uint,0444 ); +MODULE_PARM_DESC (shrink_iters,"how many shrink passes" ); + +static unsigned int size_lo =384 ;/* -> kmalloc-512 */ +module_param (size_lo, uint,0444 ); +static unsigned int size_hi =768 ;/* -> kmalloc-1k */ +module_param (size_hi, uint,0444 ); + +static void **a;/* kmalloc-1k (size_hi) */ +static void **b;/* kmalloc-512 (size_lo) */ + +static void echo_to (const char *path,const char *val) +{ +struct file *f; +loff_t pos =0 ; + + f =filp_open (path,O_WRONLY ,0 ); +if (IS_ERR (f)) +return ; +kernel_write (f, val,strlen (val), &pos); +filp_close (f,NULL ); +} + +/* Force kmem_cache_shrink on the two caches: walks partial lists and discards +* empty slabs -> discard_slab -> __free_slab -> free_slab_obj_exts -> kfree. +* If the freed obj_exts vectors cross-reference, the recursion fires here. */ +static void do_shrink (void ) +{ +unsigned int i; +char p1[64 ], p2[64 ]; + +snprintf (p1,sizeof (p1),"/sys/kernel/slab/kmalloc-1k/shrink" ); +snprintf (p2,sizeof (p2),"/sys/kernel/slab/kmalloc-512/shrink" ); +for (i =0 ; i < shrink_iters; i++) { +echo_to (p1,"1\n" ); +echo_to (p2,"1\n" ); + } +/* also the cg variants if present (harmless if absent) */ +echo_to ("/sys/kernel/slab/kmalloc-rnd-08-1k/shrink" ,"1\n" ); +echo_to ("/sys/kernel/slab/kmalloc-rnd-07-512/shrink" ,"1\n" ); +} + +static void do_round (unsigned long cnt,bool last) +{ +unsigned long i; + +for (i =0 ; i < cnt; i++) { + a[i] =kmalloc (size_hi,GFP_KERNEL ); + b[i] =kmalloc (size_lo,GFP_KERNEL ); +if (a[i]) + *(volatile char *)a[i] =1 ; +if (b[i]) + *(volatile char *)b[i] =1 ; + } +if (last) +pr_emerg ("REPRODUCER: allocated %lu+%lu objs, starting free-storm (keep=%u)\n" , + cnt, cnt, keep); + +/* Free-storm. If keep>0, leave 1-in-keep live to keep slabs partial & +* densely cross-referenced; those are freed only on the final round. */ +for (i =0 ; i < cnt; i++) { +if (keep && (i % keep) ==0 ) +continue ;/* leave this one live for now */ +kfree (a[i]); +kfree (b[i]); + } +} + +static int __init repro_init (void ) +{ +unsigned int r; +unsigned long i; + +pr_emerg ("REPRODUCER START n=%lu rounds=%u keep=%u shrink=%u sizes=%u/%u mem_profiling=%d\n" , + n, rounds, keep, shrink, size_lo, size_hi, +IS_ENABLED (CONFIG_MEM_ALLOC_PROFILING )); + + a =vmalloc (n *sizeof (void *)); + b =vmalloc (n *sizeof (void *)); +if (!a || !b) { +pr_emerg ("REPRODUCER: vmalloc failed, reduce n=\n" ); + goto out; + } + +for (r =0 ; r < rounds; r++) { +do_round (n, r == rounds -1 ); +if (shrink) +do_shrink (); + } + +/* Free everything that was kept live, then shrink hard: this is the +* discard-cascade moment most likely to hit the recursion. */ +if (keep) { +pr_emerg ("REPRODUCER: freeing kept-live objects then shrinking\n" ); +for (i =0 ; i < n; i++) { +if ((i % keep) ==0 ) { +kfree (a[i]); +kfree (b[i]); + } + } + } +if (shrink) { +pr_emerg ("REPRODUCER: final shrink storm\n" ); +do_shrink (); + } + +pr_emerg ("REPRODUCER DONE no-crash (fix present)\n" ); +out: +vfree (a); +vfree (b); +/* Return error so the module unloads itself; the test only needs init. */ +return -ECANCELED ; +} + +static void __exit repro_exit(void) { } + +module_init (repro_init); +module_exit (repro_exit); +MODULE_LICENSE ("GPL" ); +MODULE_DESCRIPTION ("Reproducer for SLUB obj_exts free-recursion stack overflow" ); +repro_public/run_repro.sh new file +16
-0@@ -0,0 +1,26 @@ +#!/bin/bash +# Branch reproducer driver. Arg1 = path to repro_objext.ko built against this kernel. +# All diagnostics go to /dev/kmsg so they survive in the dmesg dump (vng stdout is lossy). +set +e +KO="$1" +L(){ echo "BT| $*" > /dev/kmsg 2>/dev/null; } +k(){ awk '{print $1}' /sys/kernel/slab/$1/objects 2>/dev/null; } +g(){ cat /sys/kernel/slab/$1/$2 2>/dev/null; } +L "===== BRANCHTEST START =====" +L "uname=$(uname -r)" +L "cmdline=$(cat /proc/cmdline)" +echo 1 > /proc/sys/vm/mem_profiling 2>/dev/null +L "profiling_runtime=$(cat /proc/sys/vm/mem_profiling 2>/dev/null)" +L "GEOM k512 objs=$(g kmalloc-512 objs_per_slab) ord=$(g kmalloc-512 order) | k1k objs=$(g kmalloc-1k objs_per_slab) ord=$(g kmalloc-1k order) | k2k objs=$(g kmalloc-2k objs_per_slab) | k256 objs=$(g kmalloc-256 objs_per_slab)" +L "noobjext_caches=$(ls -d /sys/kernel/slab/*no-objext* 2>/dev/null | wc -l)" +L "BEFORE k512=$(k kmalloc-512) k1k=$(k kmalloc-1k)"
repro_public/run_repro.sh line 26
Quoted 10 lines
repro_public/run_repro.sh new file +10
-0+insmod "$KO" n=2000000 rounds=4 shrink=1 keep=0 shrink_iters=8 +L "insmod_rc=$?" +# extra external shrink storm to force the discard cascade +for i in $(seq 1 80); do for c in kmalloc-128 kmalloc-256 kmalloc-512 kmalloc-1k kmalloc-2k kmalloc-4k; do echo 1 > /sys/kernel/slab/$c/shrink 2>/dev/null; done; done +sleep 1 +L "AFTER k512=$(k kmalloc-512) k1k=$(k kmalloc-1k)" +L "===== BRANCHTEST END =====" +echo "=====DMESG DUMP=====" +dmesg | grep -E "BT\||REPRODUCER|stack guard|BUG:|Oops|Call Trace|Kernel panic|guard page" +echo "=====BRANCHTEST_DONE====="
Thanks for sharing the full reproducer. I'll study it and reproduce the issue,
which helps me understand the problem greatly.
Thanks
Best Regards
Hao
Harry Yoo
maintainerQuoted 49 lines · Shakeel Butt
Quoted 44 lines · Harry Yoo (Oracle)
This is a follow-up fix after the recent discussion [1]. See patch 4 for the detailed description on the bug.
Based on slab/for-next-fixes (
af9ea231c0b45) and is available at git.kernel.org [2].Instead preventing cycles by bumping up the allocation size of
obj_extsarrays, it introduces a new kmalloc type calledKMALLOC_NO_OBJ_EXTand disallow formation of cycles between kmalloc types when allocatingobj_extsarrays.obj_extsarrays of normal kmalloc caches are served fromKMALLOC_NO_OBJ_EXTcaches (that don't haveobj_exts), and all otherobj_extsarrays are served from normal kmalloc caches.I tried to reuse
SLAB_ALLOC_NO_RECURSEto makekmalloc_slab()selectKMALLOC_NO_OBJ_EXT, but it was not great because it does not allow sheaves for those caches. So I introduced a new slab alloc flagSLAB_ALLOC_NO_OBJ_EXT.To avoid huge confusion, I had to decouple "disallowing sheaves" semantics from
SLAB_NO_OBJ_EXTand introducedSLAB_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_EXTto selectKMALLOC_NO_OBJ_EXTas 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-v3r1
To: Vlastimil Babka <[email protected]>
To: Andrew Morton <[email protected]>
To: Hao Li <[email protected]>
To: Christoph Lameter <[email protected]>
To: David Rientjes <[email protected]>
To: Roman Gushchin <[email protected]>
To: Suren Baghdasaryan <[email protected]>
To: Hao Ge <[email protected]>
To: Kees Cook <[email protected]>
To: Pedro Falcato <[email protected]>
To: Shakeel Butt <[email protected]>
To: Danielle Constantino <[email protected]>
To: Liam R. Howlett <[email protected]>Quoted 3 lines
Quoted 5 lines
I tested next-20260716 which has this series against next-20260707 which does not have the fix. The reproducer were able to trigger the leak/recursion on next-20260707 but not on next-20260716. So, you can add:
Tested-by Shakeel Butt <[email protected]>
Thanks a lot for testing, Shakeel!
Signature · 2 lines
Cheers, Harry / Hyeonggon