[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <aPHcqbQkPV--NCt8@hyeyoo>
Date: Fri, 17 Oct 2025 15:05:29 +0900
From: Harry Yoo <harry.yoo@...cle.com>
To: Hao Ge <hao.ge@...ux.dev>
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
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.
> 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()?
> --
> 2.25.1
--
Cheers,
Harry / Hyeonggon
Powered by blists - more mailing lists