Skip to content

Commit e9217ca

Browse files
hygoniVlastimil Babka (SUSE)
authored andcommitted
mm/slab: initialize slab->stride early to avoid memory ordering issues
When alloc_slab_obj_exts() is called later (instead of during slab allocation and initialization), slab->stride and slab->obj_exts are updated after the slab is already accessible by multiple CPUs. The current implementation does not enforce memory ordering between slab->stride and slab->obj_exts. For correctness, slab->stride must be visible before slab->obj_exts. Otherwise, concurrent readers may observe slab->obj_exts as non-zero while stride is still stale. With stale slab->stride, slab_obj_ext() could return the wrong obj_ext. This could cause two problems: - obj_cgroup_put() is called on the wrong objcg, leading to a use-after-free due to incorrect reference counting [1] by decrementing the reference count more than it was incremented. - refill_obj_stock() is called on the wrong objcg, leading to a page_counter overflow [2] by uncharging more memory than charged. Fix this by unconditionally initializing slab->stride in alloc_slab_obj_exts_early(), before the need_slab_obj_exts() check. In the case of SLAB_OBJ_EXT_IN_OBJ, it is overridden in the function. This ensures updates to slab->stride become visible before the slab can be accessed by other CPUs via the per-node partial slab list (protected by spinlock with acquire/release semantics). Thanks to Shakeel Butt for pointing out this issue [3]. [vbabka@kernel.org: the bug reports [1] and [2] are not yet fully fixed, with investigation ongoing, but it is nevertheless a step in the right direction to only set stride once after allocating the slab and not change it later ] Fixes: 7a8e71b ("mm/slab: use stride to access slabobj_ext") Reported-by: Venkat Rao Bagalkote <venkat88@linux.ibm.com> Link: https://lore.kernel.org/lkml/ca241daa-e7e7-4604-a48d-de91ec9184a5@linux.ibm.com [1] Link: https://lore.kernel.org/all/ddff7c7d-c0c3-4780-808f-9a83268bbf0c@linux.ibm.com [2] Link: https://lore.kernel.org/linux-mm/aZu9G9mVIVzSm6Ft@hyeyoo [3] Signed-off-by: Harry Yoo <harry.yoo@oracle.com> Signed-off-by: Vlastimil Babka (SUSE) <vbabka@kernel.org>
1 parent 2b351ea commit e9217ca

File tree

1 file changed

+3
-2
lines changed

1 file changed

+3
-2
lines changed

mm/slub.c

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2196,7 +2196,6 @@ int alloc_slab_obj_exts(struct slab *slab, struct kmem_cache *s,
21962196
retry:
21972197
old_exts = READ_ONCE(slab->obj_exts);
21982198
handle_failed_objexts_alloc(old_exts, vec, objects);
2199-
slab_set_stride(slab, sizeof(struct slabobj_ext));
22002199

22012200
if (new_slab) {
22022201
/*
@@ -2272,6 +2271,9 @@ static void alloc_slab_obj_exts_early(struct kmem_cache *s, struct slab *slab)
22722271
void *addr;
22732272
unsigned long obj_exts;
22742273

2274+
/* Initialize stride early to avoid memory ordering issues */
2275+
slab_set_stride(slab, sizeof(struct slabobj_ext));
2276+
22752277
if (!need_slab_obj_exts(s))
22762278
return;
22772279

@@ -2288,7 +2290,6 @@ static void alloc_slab_obj_exts_early(struct kmem_cache *s, struct slab *slab)
22882290
obj_exts |= MEMCG_DATA_OBJEXTS;
22892291
#endif
22902292
slab->obj_exts = obj_exts;
2291-
slab_set_stride(slab, sizeof(struct slabobj_ext));
22922293
} else if (s->flags & SLAB_OBJ_EXT_IN_OBJ) {
22932294
unsigned int offset = obj_exts_offset_in_object(s);
22942295

0 commit comments

Comments
 (0)