From: Hao Ge 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. Conversely, in a race condition, if mark_failed_objexts_alloc wins the race, the other process (that previously succeeded in allocation) will lose the race. A null pointer dereference may occur in the following scenario: Thread1 Thead2 alloc_slab_obj_exts alloc_slab_obj_exts old_exts = READ_ONCE(slab->obj_exts) = 0 mark_failed_objexts_alloc(slab); cmpxchg(&slab->obj_exts, old_exts, new_exts) != old_exts kfree and return 0; alloc_tag_add -> a panic occurs. To fix this, introduce a retry mechanism for the cmpxchg() operation: 1. Add a 'retry' label at the point where READ_ONCE(slab->obj_exts) is invoked, ensuring the latest value is fetched during subsequent retries. 2. if cmpxchg() fails (indicating a concurrent update), jump back to "retry" to re-read old_exts and recheck the validity of the obj_exts allocated in this operation. Thanks for Vlastimil and Suren's help with debugging. Fixes: f7381b911640 ("slab: mark slab->obj_exts allocation failures unconditionally") Suggested-by: Suren Baghdasaryan Signed-off-by: Hao Ge --- v2: Incorporate handling for the scenario where, if mark_failed_objexts_alloc wins the race, the other process (that previously succeeded in allocation) will lose the race, based on Suren's suggestion. Add Suggested-by: Suren Baghdasaryan --- mm/slub.c | 20 +++++++++++++++++--- 1 file changed, 17 insertions(+), 3 deletions(-) diff --git a/mm/slub.c b/mm/slub.c index 2e4340c75be2..fd1b5dda3863 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); } static inline void handle_failed_objexts_alloc(unsigned long obj_exts, @@ -2136,6 +2136,7 @@ int alloc_slab_obj_exts(struct slab *slab, struct kmem_cache *s, #ifdef CONFIG_MEMCG new_exts |= MEMCG_DATA_OBJEXTS; #endif +retry: old_exts = READ_ONCE(slab->obj_exts); handle_failed_objexts_alloc(old_exts, vec, objects); if (new_slab) { @@ -2145,8 +2146,7 @@ int alloc_slab_obj_exts(struct slab *slab, struct kmem_cache *s, * be simply assigned. */ slab->obj_exts = new_exts; - } else if ((old_exts & ~OBJEXTS_FLAGS_MASK) || - cmpxchg(&slab->obj_exts, old_exts, new_exts) != old_exts) { + } else if (old_exts & ~OBJEXTS_FLAGS_MASK) { /* * If the slab is already in use, somebody can allocate and * assign slabobj_exts in parallel. In this case the existing @@ -2158,6 +2158,20 @@ int alloc_slab_obj_exts(struct slab *slab, struct kmem_cache *s, else kfree(vec); return 0; + } else if (cmpxchg(&slab->obj_exts, old_exts, new_exts) != old_exts) { + /* + * There are some abnormal scenarios caused by race conditions: + * + * Thread1 Thead2 + * alloc_slab_obj_exts alloc_slab_obj_exts + * old_exts = READ_ONCE(slab->obj_exts) = 0 + * mark_failed_objexts_alloc(slab); + * cmpxchg(&slab->obj_exts, old_exts, new_exts) != old_exts + * + * We should retry to ensure the validity of the slab_ext + * allocated in this operation. + */ + goto retry; } if (allow_spin) -- 2.25.1