Skip to content

Commit 4b1530f

Browse files
hygonitehcaster
hygoni
authored and
tehcaster
committed
mm/memcontrol,alloc_tag: handle slabobj_ext access under KASAN poison
In the near future, slabobj_ext may reside outside the allocated slab object range within a slab, which could be reported as an out-of-bounds access by KASAN. As suggested by Andrey Konovalov [1], explicitly disable KASAN and KMSAN checks when accessing slabobj_ext within slab allocator, memory profiling, and memory cgroup code. While an alternative approach could be to unpoison slabobj_ext, out-of-bounds accesses outside the slab allocator are generally more common. Move metadata_access_enable()/disable() helpers to mm/slab.h so that it can be used outside mm/slub.c. However, as suggested by Suren Baghdasaryan [2], instead of calling them directly from mm code (which is more prone to errors), change users to access slabobj_ext via get/put APIs: - Users should call get_slab_obj_exts() to access slabobj_metadata and call put_slab_obj_exts() when it's done. - From now on, accessing it outside the section covered by get_slab_obj_exts() ~ put_slab_obj_exts() is illegal. This ensures that accesses to slabobj_ext metadata won't be reported as access violations. Call kasan_reset_tag() in slab_obj_ext() before returning the address to prevent SW or HW tag-based KASAN from reporting false positives. Suggested-by: Andrey Konovalov <andreyknvl@gmail.com> Suggested-by: Suren Baghdasaryan <surenb@google.com> Link: https://lore.kernel.org/linux-mm/CA+fCnZezoWn40BaS3cgmCeLwjT+5AndzcQLc=wH3BjMCu6_YCw@mail.gmail.com [1] Link: https://lore.kernel.org/linux-mm/CAJuCfpG=Lb4WhYuPkSpdNO4Ehtjm1YcEEK0OM=3g9i=LxmpHSQ@mail.gmail.com [2] Signed-off-by: Harry Yoo <harry.yoo@oracle.com> Link: https://patch.msgid.link/20260113061845.159790-7-harry.yoo@oracle.com Signed-off-by: Vlastimil Babka <vbabka@suse.cz>
1 parent 7a8e71b commit 4b1530f

3 files changed

Lines changed: 95 additions & 40 deletions

File tree

‎mm/memcontrol.c‎

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2604,10 +2604,16 @@ struct mem_cgroup *mem_cgroup_from_obj_slab(struct slab *slab, void *p)
26042604
if (!obj_exts)
26052605
return NULL;
26062606

2607+
get_slab_obj_exts(obj_exts);
26072608
off = obj_to_index(slab->slab_cache, slab, p);
26082609
obj_ext = slab_obj_ext(slab, obj_exts, off);
2609-
if (obj_ext->objcg)
2610-
return obj_cgroup_memcg(obj_ext->objcg);
2610+
if (obj_ext->objcg) {
2611+
struct obj_cgroup *objcg = obj_ext->objcg;
2612+
2613+
put_slab_obj_exts(obj_exts);
2614+
return obj_cgroup_memcg(objcg);
2615+
}
2616+
put_slab_obj_exts(obj_exts);
26112617

26122618
return NULL;
26132619
}
@@ -3219,10 +3225,12 @@ bool __memcg_slab_post_alloc_hook(struct kmem_cache *s, struct list_lru *lru,
32193225
return false;
32203226

32213227
obj_exts = slab_obj_exts(slab);
3228+
get_slab_obj_exts(obj_exts);
32223229
off = obj_to_index(s, slab, p[i]);
32233230
obj_ext = slab_obj_ext(slab, obj_exts, off);
32243231
obj_cgroup_get(objcg);
32253232
obj_ext->objcg = objcg;
3233+
put_slab_obj_exts(obj_exts);
32263234
}
32273235

32283236
return true;

‎mm/slab.h‎

Lines changed: 51 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -508,6 +508,24 @@ bool slab_in_kunit_test(void);
508508
static inline bool slab_in_kunit_test(void) { return false; }
509509
#endif
510510

511+
/*
512+
* slub is about to manipulate internal object metadata. This memory lies
513+
* outside the range of the allocated object, so accessing it would normally
514+
* be reported by kasan as a bounds error. metadata_access_enable() is used
515+
* to tell kasan that these accesses are OK.
516+
*/
517+
static inline void metadata_access_enable(void)
518+
{
519+
kasan_disable_current();
520+
kmsan_disable_current();
521+
}
522+
523+
static inline void metadata_access_disable(void)
524+
{
525+
kmsan_enable_current();
526+
kasan_enable_current();
527+
}
528+
511529
#ifdef CONFIG_SLAB_OBJ_EXT
512530

513531
/*
@@ -517,8 +535,22 @@ static inline bool slab_in_kunit_test(void) { return false; }
517535
*
518536
* Returns the address of the object extension vector associated with the slab,
519537
* or zero if no such vector has been associated yet.
520-
* Do not dereference the return value directly; use slab_obj_ext() to access
521-
* its elements.
538+
* Do not dereference the return value directly; use get/put_slab_obj_exts()
539+
* pair and slab_obj_ext() to access individual elements.
540+
*
541+
* Example usage:
542+
*
543+
* obj_exts = slab_obj_exts(slab);
544+
* if (obj_exts) {
545+
* get_slab_obj_exts(obj_exts);
546+
* obj_ext = slab_obj_ext(slab, obj_exts, obj_to_index(s, slab, obj));
547+
* // do something with obj_ext
548+
* put_slab_obj_exts(obj_exts);
549+
* }
550+
*
551+
* Note that the get/put semantics does not involve reference counting.
552+
* Instead, it updates kasan/kmsan depth so that accesses to slabobj_ext
553+
* won't be reported as access violations.
522554
*/
523555
static inline unsigned long slab_obj_exts(struct slab *slab)
524556
{
@@ -537,6 +569,17 @@ static inline unsigned long slab_obj_exts(struct slab *slab)
537569
return obj_exts & ~OBJEXTS_FLAGS_MASK;
538570
}
539571

572+
static inline void get_slab_obj_exts(unsigned long obj_exts)
573+
{
574+
VM_WARN_ON_ONCE(!obj_exts);
575+
metadata_access_enable();
576+
}
577+
578+
static inline void put_slab_obj_exts(unsigned long obj_exts)
579+
{
580+
metadata_access_disable();
581+
}
582+
540583
#ifdef CONFIG_64BIT
541584
static inline void slab_set_stride(struct slab *slab, unsigned short stride)
542585
{
@@ -565,14 +608,19 @@ static inline unsigned short slab_get_stride(struct slab *slab)
565608
* @index: an index of the object
566609
*
567610
* Returns a pointer to the object extension associated with the object.
611+
* Must be called within a section covered by get/put_slab_obj_exts().
568612
*/
569613
static inline struct slabobj_ext *slab_obj_ext(struct slab *slab,
570614
unsigned long obj_exts,
571615
unsigned int index)
572616
{
617+
struct slabobj_ext *obj_ext;
618+
573619
VM_WARN_ON_ONCE(obj_exts != slab_obj_exts(slab));
574620

575-
return (struct slabobj_ext *)(obj_exts + slab_get_stride(slab) * index);
621+
obj_ext = (struct slabobj_ext *)(obj_exts +
622+
slab_get_stride(slab) * index);
623+
return kasan_reset_tag(obj_ext);
576624
}
577625

578626
int alloc_slab_obj_exts(struct slab *slab, struct kmem_cache *s,

‎mm/slub.c‎

Lines changed: 34 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -972,24 +972,6 @@ static slab_flags_t slub_debug;
972972
static const char *slub_debug_string __ro_after_init;
973973
static int disable_higher_order_debug;
974974

975-
/*
976-
* slub is about to manipulate internal object metadata. This memory lies
977-
* outside the range of the allocated object, so accessing it would normally
978-
* be reported by kasan as a bounds error. metadata_access_enable() is used
979-
* to tell kasan that these accesses are OK.
980-
*/
981-
static inline void metadata_access_enable(void)
982-
{
983-
kasan_disable_current();
984-
kmsan_disable_current();
985-
}
986-
987-
static inline void metadata_access_disable(void)
988-
{
989-
kmsan_enable_current();
990-
kasan_enable_current();
991-
}
992-
993975
/*
994976
* Object debugging
995977
*/
@@ -2055,23 +2037,27 @@ static bool freelist_corrupted(struct kmem_cache *s, struct slab *slab,
20552037

20562038
static inline void mark_objexts_empty(struct slabobj_ext *obj_exts)
20572039
{
2058-
unsigned long slab_exts;
20592040
struct slab *obj_exts_slab;
2041+
unsigned long slab_exts;
20602042

20612043
obj_exts_slab = virt_to_slab(obj_exts);
20622044
slab_exts = slab_obj_exts(obj_exts_slab);
20632045
if (slab_exts) {
2046+
get_slab_obj_exts(slab_exts);
20642047
unsigned int offs = obj_to_index(obj_exts_slab->slab_cache,
20652048
obj_exts_slab, obj_exts);
20662049
struct slabobj_ext *ext = slab_obj_ext(obj_exts_slab,
20672050
slab_exts, offs);
20682051

2069-
if (unlikely(is_codetag_empty(&ext->ref)))
2052+
if (unlikely(is_codetag_empty(&ext->ref))) {
2053+
put_slab_obj_exts(slab_exts);
20702054
return;
2055+
}
20712056

20722057
/* codetag should be NULL here */
20732058
WARN_ON(ext->ref.ct);
20742059
set_codetag_empty(&ext->ref);
2060+
put_slab_obj_exts(slab_exts);
20752061
}
20762062
}
20772063

@@ -2287,30 +2273,28 @@ static inline void free_slab_obj_exts(struct slab *slab)
22872273

22882274
#ifdef CONFIG_MEM_ALLOC_PROFILING
22892275

2290-
static inline struct slabobj_ext *
2291-
prepare_slab_obj_ext_hook(struct kmem_cache *s, gfp_t flags, void *p)
2276+
static inline unsigned long
2277+
prepare_slab_obj_exts_hook(struct kmem_cache *s, struct slab *slab,
2278+
gfp_t flags, void *p)
22922279
{
2293-
struct slab *slab;
2294-
unsigned long obj_exts;
2295-
2296-
slab = virt_to_slab(p);
2297-
obj_exts = slab_obj_exts(slab);
2298-
if (!obj_exts &&
2280+
if (!slab_obj_exts(slab) &&
22992281
alloc_slab_obj_exts(slab, s, flags, false)) {
23002282
pr_warn_once("%s, %s: Failed to create slab extension vector!\n",
23012283
__func__, s->name);
2302-
return NULL;
2284+
return 0;
23032285
}
23042286

2305-
obj_exts = slab_obj_exts(slab);
2306-
return slab_obj_ext(slab, obj_exts, obj_to_index(s, slab, p));
2287+
return slab_obj_exts(slab);
23072288
}
23082289

2290+
23092291
/* Should be called only if mem_alloc_profiling_enabled() */
23102292
static noinline void
23112293
__alloc_tagging_slab_alloc_hook(struct kmem_cache *s, void *object, gfp_t flags)
23122294
{
2295+
unsigned long obj_exts;
23132296
struct slabobj_ext *obj_ext;
2297+
struct slab *slab;
23142298

23152299
if (!object)
23162300
return;
@@ -2321,16 +2305,23 @@ __alloc_tagging_slab_alloc_hook(struct kmem_cache *s, void *object, gfp_t flags)
23212305
if (flags & __GFP_NO_OBJ_EXT)
23222306
return;
23232307

2324-
obj_ext = prepare_slab_obj_ext_hook(s, flags, object);
2308+
slab = virt_to_slab(object);
2309+
obj_exts = prepare_slab_obj_exts_hook(s, slab, flags, object);
23252310
/*
23262311
* Currently obj_exts is used only for allocation profiling.
23272312
* If other users appear then mem_alloc_profiling_enabled()
23282313
* check should be added before alloc_tag_add().
23292314
*/
2330-
if (likely(obj_ext))
2315+
if (obj_exts) {
2316+
unsigned int obj_idx = obj_to_index(s, slab, object);
2317+
2318+
get_slab_obj_exts(obj_exts);
2319+
obj_ext = slab_obj_ext(slab, obj_exts, obj_idx);
23312320
alloc_tag_add(&obj_ext->ref, current->alloc_tag, s->size);
2332-
else
2321+
put_slab_obj_exts(obj_exts);
2322+
} else {
23332323
alloc_tag_set_inaccurate(current->alloc_tag);
2324+
}
23342325
}
23352326

23362327
static inline void
@@ -2356,11 +2347,13 @@ __alloc_tagging_slab_free_hook(struct kmem_cache *s, struct slab *slab, void **p
23562347
if (!obj_exts)
23572348
return;
23582349

2350+
get_slab_obj_exts(obj_exts);
23592351
for (i = 0; i < objects; i++) {
23602352
unsigned int off = obj_to_index(s, slab, p[i]);
23612353

23622354
alloc_tag_sub(&slab_obj_ext(slab, obj_exts, off)->ref, s->size);
23632355
}
2356+
put_slab_obj_exts(obj_exts);
23642357
}
23652358

23662359
static inline void
@@ -2427,7 +2420,9 @@ void memcg_slab_free_hook(struct kmem_cache *s, struct slab *slab, void **p,
24272420
if (likely(!obj_exts))
24282421
return;
24292422

2423+
get_slab_obj_exts(obj_exts);
24302424
__memcg_slab_free_hook(s, slab, p, objects, obj_exts);
2425+
put_slab_obj_exts(obj_exts);
24312426
}
24322427

24332428
static __fastpath_inline
@@ -2477,10 +2472,14 @@ bool memcg_slab_post_charge(void *p, gfp_t flags)
24772472
/* Ignore already charged objects. */
24782473
obj_exts = slab_obj_exts(slab);
24792474
if (obj_exts) {
2475+
get_slab_obj_exts(obj_exts);
24802476
off = obj_to_index(s, slab, p);
24812477
obj_ext = slab_obj_ext(slab, obj_exts, off);
2482-
if (unlikely(obj_ext->objcg))
2478+
if (unlikely(obj_ext->objcg)) {
2479+
put_slab_obj_exts(obj_exts);
24832480
return true;
2481+
}
2482+
put_slab_obj_exts(obj_exts);
24842483
}
24852484

24862485
return __memcg_slab_post_alloc_hook(s, NULL, flags, 1, &p);

0 commit comments

Comments
 (0)