[PATCH v2] slab: Fix obj_ext is mistakenly considered NULL due to race condition

[PATCH v2] slab: Fix obj_ext is mistakenly considered NULL due to race condition

[Hao Ge]

From: Hao Ge <gehao@kylinos.cn>

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

Suggested-by: Harry Yoo <harry.yoo@oracle.com>
Signed-off-by: Hao Ge <gehao@kylinos.cn>
---
v2: Revise the solution according to Harry's suggestion.
    Add Suggested-by: Harry Yoo <harry.yoo@oracle.com>
---
 mm/slub.c | 16 +++++++++++-----
 1 file changed, 11 insertions(+), 5 deletions(-)

diff --git a/mm/slub.c b/mm/slub.c
index d4403341c9df..d7bfec6c0171 100644
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -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

Re: [PATCH v2] slab: Fix obj_ext is mistakenly considered NULL due to race condition

[Suren Baghdasaryan]

On Thu, Oct 23, 2025 at 7:34 AM Hao Ge <hao.ge@linux.dev> wrote:
>
> From: Hao Ge <gehao@kylinos.cn>
>
> 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()
>
> Suggested-by: Harry Yoo <harry.yoo@oracle.com>
> Signed-off-by: Hao Ge <gehao@kylinos.cn>

Reviewed-by: Suren Baghdasaryan <surenb@google.com>

> ---
> v2: Revise the solution according to Harry's suggestion.
>     Add Suggested-by: Harry Yoo <harry.yoo@oracle.com>
> ---
>  mm/slub.c | 16 +++++++++++-----
>  1 file changed, 11 insertions(+), 5 deletions(-)
>
> diff --git a/mm/slub.c b/mm/slub.c
> index d4403341c9df..d7bfec6c0171 100644
> --- a/mm/slub.c
> +++ b/mm/slub.c
> @@ -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
>

Re: [PATCH v2] slab: Fix obj_ext is mistakenly considered NULL due to race condition

[Harry Yoo]

On Thu, Oct 23, 2025 at 10:33:13PM +0800, Hao Ge wrote:
> From: Hao Ge <gehao@kylinos.cn>
> 
> 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.

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."

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

You mean "when the process that failed to allocate the vector loses the
cmpxchg()"?

> Suggested-by: Harry Yoo <harry.yoo@oracle.com>
> Signed-off-by: Hao Ge <gehao@kylinos.cn>
> ---
> v2: Revise the solution according to Harry's suggestion.
>     Add Suggested-by: Harry Yoo <harry.yoo@oracle.com>
> ---
>
>  mm/slub.c | 16 +++++++++++-----
>  1 file changed, 11 insertions(+), 5 deletions(-)
> 
> diff --git a/mm/slub.c b/mm/slub.c
> index d4403341c9df..d7bfec6c0171 100644
> --- a/mm/slub.c
> +++ b/mm/slub.c
> @@ -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

nit:
                                                               ^ missing
							       period
							       (.) here

With the comments resolved,
Reviewed-by: Harry Yoo <harry.yoo@oracle.com>

> +		 * 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

-- 
Cheers,
Harry / Hyeonggon

Re: [PATCH v2] slab: Fix obj_ext is mistakenly considered NULL due to race condition

[Hao Ge]

Hi Harry


On 2025/10/24 16:54, Harry Yoo wrote:
> On Thu, Oct 23, 2025 at 10:33:13PM +0800, Hao Ge wrote:
>> From: Hao Ge <gehao@kylinos.cn>
>>
>> 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.
> 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."
>
>> 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()
> 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.

>> Suggested-by: Harry Yoo <harry.yoo@oracle.com>
>> Signed-off-by: Hao Ge <gehao@kylinos.cn>
>> ---
>> v2: Revise the solution according to Harry's suggestion.
>>      Add Suggested-by: Harry Yoo <harry.yoo@oracle.com>
>> ---
>>
>>   mm/slub.c | 16 +++++++++++-----
>>   1 file changed, 11 insertions(+), 5 deletions(-)
>>
>> diff --git a/mm/slub.c b/mm/slub.c
>> index d4403341c9df..d7bfec6c0171 100644
>> --- a/mm/slub.c
>> +++ b/mm/slub.c
>> @@ -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
> nit:
>                                                                 ^ missing
> 							       period
> 							       (.) here
>
> With the comments resolved,
> Reviewed-by: Harry Yoo <harry.yoo@oracle.com>


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?


>
>> +		 * 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

Re: [PATCH v2] slab: Fix obj_ext is mistakenly considered NULL due to race condition

[Vlastimil Babka]

On 10/24/25 11:27, Hao Ge wrote:
> Hi Harry
> 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?

Massaged it a bit more and now have this:

commit b4bdf6770cadb8bffcf3dce7ad7a346979f79ede
Author: Hao Ge <gehao@kylinos.cn>
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.
    
    Suggested-by: Harry Yoo <harry.yoo@oracle.com>
    Signed-off-by: Hao Ge <gehao@kylinos.cn>
    Link: https://patch.msgid.link/20251023143313.1327968-1-hao.ge@linux.dev
    Reviewed-by: Suren Baghdasaryan <surenb@google.com>
    Reviewed-by: Harry Yoo <harry.yoo@oracle.com>
    Signed-off-by: Vlastimil Babka <vbabka@suse.cz>

diff --git a/mm/slub.c b/mm/slub.c
index 87a1d2f9de0d..d4367f25b20d 100644
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -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;
 	}

Re: [PATCH v2] slab: Fix obj_ext is mistakenly considered NULL due to race condition

[Hao Ge]

Hi Vlastimil


> On Oct 24, 2025, at 17:40, Vlastimil Babka <vbabka@suse.cz> wrote:
> 
> On 10/24/25 11:27, Hao Ge wrote:
>> Hi Harry
>> 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?
> 
> Massaged it a bit more and now have this:
> 
> commit b4bdf6770cadb8bffcf3dce7ad7a346979f79ede
> Author: Hao Ge <gehao@kylinos.cn>
> 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.
> 
>    Suggested-by: Harry Yoo <harry.yoo@oracle.com>
>    Signed-off-by: Hao Ge <gehao@kylinos.cn>
>    Link: https://patch.msgid.link/20251023143313.1327968-1-hao.ge@linux.dev
>    Reviewed-by: Suren Baghdasaryan <surenb@google.com>
>    Reviewed-by: Harry Yoo <harry.yoo@oracle.com>
>    Signed-off-by: Vlastimil Babka <vbabka@suse.cz>
> 
> diff --git a/mm/slub.c b/mm/slub.c
> index 87a1d2f9de0d..d4367f25b20d 100644
> --- a/mm/slub.c
> +++ b/mm/slub.c
> @@ -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;
>    }
> 
Thank you very much for your help, 
and I sincerely apologize for the actual trouble I have caused you.

Re: [PATCH v2] slab: Fix obj_ext is mistakenly considered NULL due to race condition

[Vlastimil Babka]

On 10/24/25 12:06, Hao Ge wrote:
>>    }
>> 
> 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!