slab: Fix obj_ext is mistakenly considered NULL due to race condition
Subsystems (2)
PATCH v2 1/1 slab: Fix obj_ext is mistakenly considered NULL due to race condition
From: Hao Ge <[email protected]>
If two competing threads enter alloc_slab_obj_exts(), if the process that allocates the vector wins cmpxchg(), and the other thread mistakenly assume slab->obj_ext is still empty due to its own allocation failure. This will then trigger warnings enforced by CONFIG_MEM_ALLOC_PROFILING_DEBUG checks in the subsequent free path.
Therefore, let's add an additional check when the process that allocates the vector loses the cmpxchg()
---
v2: Revise the solution according to Harry's suggestion.
Add Suggested-by: Harry Yoo <[email protected]>
--- mm/slub.c | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-)
mm/slub.c +11 -5
@@ -2052,9 +2052,9 @@ static inline void mark_objexts_empty(struct slabobj_ext *obj_exts) } } -static inline void mark_failed_objexts_alloc (struct slab *slab) +static inline bool mark_failed_objexts_alloc (struct slab *slab) { -cmpxchg (&slab->obj_exts,0 ,OBJEXTS_ALLOC_FAIL ); +return cmpxchg (&slab->obj_exts,0 ,OBJEXTS_ALLOC_FAIL ) ==0 ; } static inline void handle_failed_objexts_alloc(unsigned long obj_exts,
@@ -2076,7 +2076,7 @@ static inline void handle_failed_objexts_alloc(unsigned long obj_exts,#else /* CONFIG_MEM_ALLOC_PROFILING_DEBUG */ static inline void mark_objexts_empty (struct slabobj_ext *obj_exts) {} -static inline void mark_failed_objexts_alloc (struct slab *slab) {} +static inline bool mark_failed_objexts_alloc (struct slab *slab) {return false; }static inline void handle_failed_objexts_alloc (unsigned long obj_exts,struct slabobj_ext *vec,unsigned int objects) {}
@@ -2124,8 +2124,14 @@ int alloc_slab_obj_exts(struct slab *slab, struct kmem_cache *s, slab_nid(slab)); }if (!vec) { -/* Mark vectors which failed to allocate */ -mark_failed_objexts_alloc (slab); +/* +* Try to mark vectors which failed to allocate +* If this operation fails, there may be a racing process +* that has already completed the allocation. +*/ +if (!mark_failed_objexts_alloc (slab) && +slab_obj_exts (slab)) +return 0 ;return -ENOMEM ; }
Signature · 1 line
2.25.1
Quoted 9 lines · Hao Ge
From: Hao Ge <[email protected]>
If two competing threads enter
alloc_slab_obj_exts(), if the process that allocates the vector winscmpxchg(), and the other thread mistakenly assumeslab->obj_extis still empty due to its own allocation failure. This will then trigger warnings enforced byCONFIG_MEM_ALLOC_PROFILING_DEBUGchecks in the subsequent free path.
Quoted 5 lines
Therefore, let's add an additional check when the process that allocates the vector loses the
cmpxchg()Suggested-by Harry Yoo <[email protected]>Signed-off-by Hao Ge <[email protected]>
Quoted 52 lines
---
v2: Revise the solution according to Harry's suggestion.
Add Suggested-by: Harry Yoo <[email protected]>--- mm/slub.c | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-)mm/slub.c +11
-6@@ -2052,9 +2052,9 @@ static inline void mark_objexts_empty(struct slabobj_ext *obj_exts) } } -static inline void mark_failed_objexts_alloc (struct slab *slab) +static inline bool mark_failed_objexts_alloc (struct slab *slab) { -cmpxchg (&slab->obj_exts,0 ,OBJEXTS_ALLOC_FAIL ); +return cmpxchg (&slab->obj_exts,0 ,OBJEXTS_ALLOC_FAIL ) ==0 ; } static inline void handle_failed_objexts_alloc(unsigned long obj_exts,@@ -2076,7 +2076,7 @@ static inline void handle_failed_objexts_alloc(unsigned long obj_exts,#else /* CONFIG_MEM_ALLOC_PROFILING_DEBUG */ static inline void mark_objexts_empty (struct slabobj_ext *obj_exts) {} -static inline void mark_failed_objexts_alloc (struct slab *slab) {} +static inline bool mark_failed_objexts_alloc (struct slab *slab) {return false; }static inline void handle_failed_objexts_alloc (unsigned long obj_exts,struct slabobj_ext *vec,unsigned int objects) {}@@ -2124,8 +2124,14 @@ int alloc_slab_obj_exts(struct slab *slab, struct kmem_cache *s, slab_nid(slab)); }if (!vec) { -/* Mark vectors which failed to allocate */ -mark_failed_objexts_alloc (slab); +/* +* Try to mark vectors which failed to allocate +* If this operation fails, there may be a racing process +* that has already completed the allocation. +*/ +if (!mark_failed_objexts_alloc (slab) && +slab_obj_exts (slab)) +return 0 ;return -ENOMEM ; } --2.25.1
Quoted 5 lines · Hao Ge
From: Hao Ge <[email protected]>
If two competing threads enter
alloc_slab_obj_exts(), if the process that allocates the vector winscmpxchg(), and the other thread mistakenly assumeslab->obj_extis still empty due to its own allocation failure.
Massaging this a little bit:
"If two competing threads enter alloc_slab_obj_exts(), and the one that
allocates the vector wins the cmpxchg(), the other thread that failed allocation mistakenly assumes that slab->obj_exts is still empty due to its own allocation failure."
Quoted 6 lines
This
will then trigger warnings enforced byCONFIG_MEM_ALLOC_PROFILING_DEBUGchecks in the subsequent free path.Therefore, let's add an additional check when the process that allocates the vector loses the
cmpxchg()
You mean "when the process that failed to allocate the vector loses the cmpxchg()"?
Quoted 36 lines
Suggested-by Harry Yoo <[email protected]>Signed-off-by Hao Ge <[email protected]>---
v2: Revise the solution according to Harry's suggestion.
Add Suggested-by: Harry Yoo <[email protected]>
---mm/slub.c | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-)mm/slub.c +3
-3@@ -2052,9 +2052,9 @@ static inline void mark_objexts_empty(struct slabobj_ext *obj_exts) } } -static inline void mark_failed_objexts_alloc (struct slab *slab) +static inline bool mark_failed_objexts_alloc (struct slab *slab) { -cmpxchg (&slab->obj_exts,0 ,OBJEXTS_ALLOC_FAIL ); +return cmpxchg (&slab->obj_exts,0 ,OBJEXTS_ALLOC_FAIL ) ==0 ; } static inline void handle_failed_objexts_alloc(unsigned long obj_exts,@@ -2076,7 +2076,7 @@ static inline void handle_failed_objexts_alloc(unsigned long obj_exts,#else /* CONFIG_MEM_ALLOC_PROFILING_DEBUG */ static inline void mark_objexts_empty (struct slabobj_ext *obj_exts) {} -static inline void mark_failed_objexts_alloc (struct slab *slab) {} +static inline bool mark_failed_objexts_alloc (struct slab *slab) {return false; }static inline void handle_failed_objexts_alloc (unsigned long obj_exts,struct slabobj_ext *vec,unsigned int objects) {}
mm/slub.c int alloc_slab_obj_exts(struct slab *slab, struct kmem_cache *s, · line 2128
Quoted 7 lines
mm/slub.c +2
-2@@ -2124,8 +2124,14 @@ int alloc_slab_obj_exts(struct slab *slab, struct kmem_cache *s, slab_nid(slab)); }if (!vec) { -/* Mark vectors which failed to allocate */ -mark_failed_objexts_alloc (slab); +/* +* Try to mark vectors which failed to allocate
nit:
^ missing period (.) here
With the comments resolved,
Quoted 11 lines
mm/slub.c +6
-0+* If this operation fails, there may be a racing process +* that has already completed the allocation. +*/ +if (!mark_failed_objexts_alloc (slab) && +slab_obj_exts (slab)) +return 0 ;return -ENOMEM ; }Signature · 1 line
2.25.1
Signature · 2 lines
Cheers, Harry / Hyeonggon
Hi Harry
Quoted 13 lines · Harry Yoo
Quoted 5 lines · Hao Ge
From: Hao Ge <[email protected]>
If two competing threads enter
alloc_slab_obj_exts(), if the process that allocates the vector winscmpxchg(), and the other thread mistakenly assumeslab->obj_extis still empty due to its own allocation failure.Massaging this a little bit:
"If two competing threads enter
alloc_slab_obj_exts(), and the one that
allocates the vector wins thecmpxchg(), the other thread that failed allocation mistakenly assumes thatslab->obj_extsis still empty due to its own allocation failure."
Quoted 8 lines
Quoted 6 lines
This
will then trigger warnings enforced byCONFIG_MEM_ALLOC_PROFILING_DEBUGchecks in the subsequent free path.Therefore, let's add an additional check when the process that allocates the vector loses the
cmpxchg()You mean "when the process that failed to allocate the vector loses the
cmpxchg()"?
Yes, I apologize for not being clear enough in my description here.
Quoted 44 lines
Quoted 36 lines
Suggested-by Harry Yoo <[email protected]>Signed-off-by Hao Ge <[email protected]>---
v2: Revise the solution according to Harry's suggestion.
Add Suggested-by: Harry Yoo <[email protected]>
---mm/slub.c | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-)mm/slub.c +3
-3@@ -2052,9 +2052,9 @@ static inline void mark_objexts_empty(struct slabobj_ext *obj_exts) } } -static inline void mark_failed_objexts_alloc (struct slab *slab) +static inline bool mark_failed_objexts_alloc (struct slab *slab) { -cmpxchg (&slab->obj_exts,0 ,OBJEXTS_ALLOC_FAIL ); +return cmpxchg (&slab->obj_exts,0 ,OBJEXTS_ALLOC_FAIL ) ==0 ; } static inline void handle_failed_objexts_alloc(unsigned long obj_exts,@@ -2076,7 +2076,7 @@ static inline void handle_failed_objexts_alloc(unsigned long obj_exts,#else /* CONFIG_MEM_ALLOC_PROFILING_DEBUG */ static inline void mark_objexts_empty (struct slabobj_ext *obj_exts) {} -static inline void mark_failed_objexts_alloc (struct slab *slab) {} +static inline bool mark_failed_objexts_alloc (struct slab *slab) {return false; }static inline void handle_failed_objexts_alloc (unsigned long obj_exts,struct slabobj_ext *vec,unsigned int objects) {}mm/slub.c int alloc_slab_obj_exts(struct slab *slab, struct kmem_cache *s, · line 2128
Quoted 7 lines
mm/slub.c +2
-2@@ -2124,8 +2124,14 @@ int alloc_slab_obj_exts(struct slab *slab, struct kmem_cache *s, slab_nid(slab)); }if (!vec) { -/* Mark vectors which failed to allocate */ -mark_failed_objexts_alloc (slab); +/* +* Try to mark vectors which failed to allocate nit:
Quoted 6 lines
^ missing period (.) hereWith the comments resolved,
Reviewed-by Harry Yoo <[email protected]>
A period is indeed missing here.
Hi Vlastimil
Thank you for adding V2 to your tree. Now, should I resubmit V3,
or can you assist with making these modifications in your tree?
Quoted 12 lines
Quoted 12 lines
mm/slub.c +6
-0+* If this operation fails, there may be a racing process +* that has already completed the allocation. +*/ +if (!mark_failed_objexts_alloc (slab) && +slab_obj_exts (slab)) +return 0 ;return -ENOMEM ; }Signature · 1 line
2.25.1
Quoted 7 lines · Hao Ge
Hi Harry
Hi VlastimilThank you for adding V2 to your tree. Now, should I resubmit V3,
or can you assist with making these modifications in your tree?
Massaged it a bit more and now have this:
commit b4bdf6770cadb8bffcf3dce7ad7a346979f79ede
Author: Hao Ge <[email protected]> Date: Thu Oct 23 22:33:13 2025 +0800
slab: Fix obj_ext mistakenly considered NULL due to race condition
If two competing threads enter alloc_slab_obj_exts(), and the one that allocates the vector wins the cmpxchg(), the other thread that failed allocation mistakenly assumes that slab->obj_exts is still empty due to its own allocation failure. This will then trigger warnings with CONFIG_MEM_ALLOC_PROFILING_DEBUG checks in the subsequent free path.
Therefore, let's check the result of cmpxchg() to see if marking the allocation as failed was successful. If it wasn't, check whether the winning side has succeeded its allocation (it might have been also marking it as failed) and if yes, return success.
mm/slub.c +11 -5
@@ -2052,9 +2052,9 @@ static inline void mark_objexts_empty(struct slabobj_ext *obj_exts) } } -static inline void mark_failed_objexts_alloc (struct slab *slab) +static inline bool mark_failed_objexts_alloc (struct slab *slab) { -cmpxchg (&slab->obj_exts,0 ,OBJEXTS_ALLOC_FAIL ); +return cmpxchg (&slab->obj_exts,0 ,OBJEXTS_ALLOC_FAIL ) ==0 ; } static inline void handle_failed_objexts_alloc(unsigned long obj_exts,
@@ -2076,7 +2076,7 @@ static inline void handle_failed_objexts_alloc(unsigned long obj_exts,#else /* CONFIG_MEM_ALLOC_PROFILING_DEBUG */ static inline void mark_objexts_empty (struct slabobj_ext *obj_exts) {} -static inline void mark_failed_objexts_alloc (struct slab *slab) {} +static inline bool mark_failed_objexts_alloc (struct slab *slab) {return false; }static inline void handle_failed_objexts_alloc (unsigned long obj_exts,struct slabobj_ext *vec,unsigned int objects) {}
@@ -2124,8 +2124,14 @@ int alloc_slab_obj_exts(struct slab *slab, struct kmem_cache *s, slab_nid(slab)); }if (!vec) { -/* Mark vectors which failed to allocate */ -mark_failed_objexts_alloc (slab); +/* +* Try to mark vectors which failed to allocate. +* If this operation fails, there may be a racing process +* that has already completed the allocation. +*/ +if (!mark_failed_objexts_alloc (slab) && +slab_obj_exts (slab)) +return 0 ;return -ENOMEM ; }
Hi Vlastimil
Quoted 70 lines · Vlastimil Babka
Quoted 7 lines · Hao Ge
Hi Harry
Hi VlastimilThank you for adding V2 to your tree. Now, should I resubmit V3,
or can you assist with making these modifications in your tree?
Massaged it a bit more and now have this:
commit
b4bdf6770cadb8bffcf3dce7ad7a346979f79edeAuthor: Hao Ge <[email protected]> Date: Thu Oct 23 22:33:13 2025 +0800slab: Fix
obj_extmistakenly considered NULL due to race conditionIf two competing threads enter
alloc_slab_obj_exts(), and the one that allocates the vector wins thecmpxchg(), the other thread that failed allocation mistakenly assumes thatslab->obj_extsis still empty due to its own allocation failure. This will then trigger warnings withCONFIG_MEM_ALLOC_PROFILING_DEBUGchecks in the subsequent free path.Therefore, let's check the result of
cmpxchg()to see if marking the allocation as failed was successful. If it wasn't, check whether the winning side has succeeded its allocation (it might have been also marking it as failed) and if yes, return success.Suggested-by Harry Yoo <[email protected]>Signed-off-by Hao Ge <[email protected]>Reviewed-by Suren Baghdasaryan <[email protected]>Reviewed-by Harry Yoo <[email protected]>Signed-off-by Vlastimil Babka <[email protected]>mm/slub.c +4
-5@@ -2052,9 +2052,9 @@ static inline void mark_objexts_empty(struct slabobj_ext *obj_exts) } } -static inline void mark_failed_objexts_alloc (struct slab *slab) +static inline bool mark_failed_objexts_alloc (struct slab *slab) { -cmpxchg (&slab->obj_exts,0 ,OBJEXTS_ALLOC_FAIL ); +return cmpxchg (&slab->obj_exts,0 ,OBJEXTS_ALLOC_FAIL ) ==0 ; } static inline void handle_failed_objexts_alloc(unsigned long obj_exts,@@ -2076,7 +2076,7 @@ static inline void handle_failed_objexts_alloc(unsigned long obj_exts,#else /* CONFIG_MEM_ALLOC_PROFILING_DEBUG */ static inline void mark_objexts_empty (struct slabobj_ext *obj_exts) {} -static inline void mark_failed_objexts_alloc (struct slab *slab) {} +static inline bool mark_failed_objexts_alloc (struct slab *slab) {return false; }static inline void handle_failed_objexts_alloc (unsigned long obj_exts,struct slabobj_ext *vec,unsigned int objects) {}@@ -2124,8 +2124,14 @@ int alloc_slab_obj_exts(struct slab *slab, struct kmem_cache *s, slab_nid(slab)); }if (!vec) { -/* Mark vectors which failed to allocate */ -mark_failed_objexts_alloc (slab); +/*
mm/slub.c int alloc_slab_obj_exts(struct slab *slab, struct kmem_cache *s, · line 2137
Quoted 10 lines
mm/slub.c +7
-0+* Try to mark vectors which failed to allocate. +* If this operation fails, there may be a racing process +* that has already completed the allocation. +*/ +if (!mark_failed_objexts_alloc (slab) && +slab_obj_exts (slab)) +return 0 ;return -ENOMEM ; }
Thank you very much for your help,
and I sincerely apologize for the actual trouble I have caused you.
Quoted 4 lines · Hao Ge
Quoted 2 lines
}Thank you very much for your help,
and I sincerely apologize for the actual trouble I have caused you.
No trouble at all, this is all normal review/maintenance process :) Thank you for the fixes!