linux-kernel - Re: [PATCH] slab: Avoid race on slab->obj_exts in alloc_slab_obj

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <556352a6-70dc-4709-a0d2-038e2cd4fd88@linux.dev>
Date: Fri, 17 Oct 2025 14:42:56 +0800
From: Hao Ge <hao.ge@...ux.dev>
To: Harry Yoo <harry.yoo@...cle.com>
Cc: Vlastimil Babka <vbabka@...e.cz>,
 Andrew Morton <akpm@...ux-foundation.org>, Christoph Lameter
 <cl@...two.org>, David Rientjes <rientjes@...gle.com>,
 Roman Gushchin <roman.gushchin@...ux.dev>,
 Suren Baghdasaryan <surenb@...gle.com>, Shakeel Butt
 <shakeel.butt@...ux.dev>, linux-mm@...ck.org, linux-kernel@...r.kernel.org,
 Hao Ge <gehao@...inos.cn>
Subject: Re: [PATCH] slab: Avoid race on slab->obj_exts in alloc_slab_obj_exts

Hi Harry


Thank you for your quick response.


On 2025/10/17 14:05, Harry Yoo wrote:
> On Fri, Oct 17, 2025 at 12:57:49PM +0800, Hao Ge wrote:
>> From: Hao Ge <gehao@...inos.cn>
>>
>> In the alloc_slab_obj_exts function, there is a race condition
>> between the successful allocation of slab->obj_exts and its
>> setting to OBJEXTS_ALLOC_FAIL due to allocation failure.
>>
>> When two threads are both allocating objects from the same slab,
>> they both end up entering the alloc_slab_obj_exts function because
>> the slab has no obj_exts (allocated yet).
>>
>> And One call succeeds in allocation, but the racing one overwrites
>> our obj_ext with OBJEXTS_ALLOC_FAIL. The threads that successfully
>> allocated will have prepare_slab_obj_exts_hook() return
>> slab_obj_exts(slab) + obj_to_index(s, slab, p), where slab_obj_exts(slab)
>> already sees OBJEXTS_ALLOC_FAIL and thus it returns an offset based
>> on the zero address.
>>
>> And then it will call alloc_tag_add, where the member codetag_ref *ref
>> of obj_exts will be referenced.Thus, a NULL pointer dereference occurs,
>> leading to a panic.
>>
>> In order to avoid that, for the case of allocation failure where
>> OBJEXTS_ALLOC_FAIL is assigned, we use cmpxchg to handle this assignment.
>>
>> Thanks for Vlastimil and Suren's help with debugging.
>>
>> Fixes: f7381b911640 ("slab: mark slab->obj_exts allocation failures unconditionally")
> I think we should add Cc: stable as well?
> We need an explicit Cc: stable to backport mm patches to -stable.
Oh sorry, I missed this.
>
>> Signed-off-by: Hao Ge <gehao@...inos.cn>
>> ---
>>   mm/slub.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/mm/slub.c b/mm/slub.c
>> index 2e4340c75be2..9e6361796e34 100644
>> --- a/mm/slub.c
>> +++ b/mm/slub.c
>> @@ -2054,7 +2054,7 @@ static inline void mark_objexts_empty(struct slabobj_ext *obj_exts)
>>   
>>   static inline void mark_failed_objexts_alloc(struct slab *slab)
>>   {
>> -	slab->obj_exts = OBJEXTS_ALLOC_FAIL;
>> +	cmpxchg(&slab->obj_exts, 0, OBJEXTS_ALLOC_FAIL);
>>   }
> A silly question:
>
> If mark_failed_objexts_alloc() succeeds and a concurrent
> alloc_slab_obj_exts() loses, should we retry cmpxchg() in
> alloc_slab_obj_exts()?

Great point.

We could modify it like this, perhaps?

  static inline void mark_failed_objexts_alloc(struct slab *slab)
  {
+       unsigned long old_exts = READ_ONCE(slab->obj_exts);
+       if( old_exts == 0 )
+               cmpxchg(&slab->obj_exts, 0, OBJEXTS_ALLOC_FAIL);
  }

Do you have any better suggestions on your end?

>
>> -- 
>> 2.25.1