Skip to content

Commit d35426e

Browse files
JoonsooKimgregkh
authored andcommitted
slab: fix oops when reading /proc/slab_allocators
commit 0378730 upstream. Commit b1cb098 ("change the management method of free objects of the slab") introduced a bug on slab leak detector ('/proc/slab_allocators'). This detector works like as following decription. 1. traverse all objects on all the slabs. 2. determine whether it is active or not. 3. if active, print who allocate this object. but that commit changed the way how to manage free objects, so the logic determining whether it is active or not is also changed. In before, we regard object in cpu caches as inactive one, but, with this commit, we mistakenly regard object in cpu caches as active one. This intoduces kernel oops if DEBUG_PAGEALLOC is enabled. If DEBUG_PAGEALLOC is enabled, kernel_map_pages() is used to detect who corrupt free memory in the slab. It unmaps page table mapping if object is free and map it if object is active. When slab leak detector check object in cpu caches, it mistakenly think this object active so try to access object memory to retrieve caller of allocation. At this point, page table mapping to this object doesn't exist, so oops occurs. Following is oops message reported from Dave. It blew up when something tried to read /proc/slab_allocators (Just cat it, and you should see the oops below) Oops: 0000 [#1] PREEMPT SMP DEBUG_PAGEALLOC Modules linked in: [snip...] CPU: 1 PID: 9386 Comm: trinity-c33 Not tainted 3.14.0-rc5+ #131 task: ffff8801aa46e890 ti: ffff880076924000 task.ti: ffff880076924000 RIP: 0010:[<ffffffffaa1a8f4a>] [<ffffffffaa1a8f4a>] handle_slab+0x8a/0x180 RSP: 0018:ffff880076925de0 EFLAGS: 00010002 RAX: 0000000000001000 RBX: 0000000000000000 RCX: 000000005ce85ce7 RDX: ffffea00079be100 RSI: 0000000000001000 RDI: ffff880107458000 RBP: ffff880076925e18 R08: 0000000000000001 R09: 0000000000000000 R10: 0000000000000000 R11: 000000000000000f R12: ffff8801e6f84000 R13: ffffea00079be100 R14: ffff880107458000 R15: ffff88022bb8d2c0 FS: 00007fb769e45740(0000) GS:ffff88024d040000(0000) knlGS:0000000000000000 CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 CR2: ffff8801e6f84ff8 CR3: 00000000a22db000 CR4: 00000000001407e0 DR0: 0000000002695000 DR1: 0000000002695000 DR2: 0000000000000000 DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000070602 Call Trace: leaks_show+0xce/0x240 seq_read+0x28e/0x490 proc_reg_read+0x3d/0x80 vfs_read+0x9b/0x160 SyS_read+0x58/0xb0 tracesys+0xd4/0xd9 Code: f5 00 00 00 0f 1f 44 00 00 48 63 c8 44 3b 0c 8a 0f 84 e3 00 00 00 83 c0 01 44 39 c0 72 eb 41 f6 47 1a 01 0f 84 e9 00 00 00 89 f0 <4d> 8b 4c 04 f8 4d 85 c9 0f 84 88 00 00 00 49 8b 7e 08 4d 8d 46 RIP handle_slab+0x8a/0x180 To fix the problem, I introduce an object status buffer on each slab. With this, we can track object status precisely, so slab leak detector would not access active object and no kernel oops would occur. Memory overhead caused by this fix is only imposed to CONFIG_DEBUG_SLAB_LEAK which is mainly used for debugging, so memory overhead isn't big problem. Signed-off-by: Joonsoo Kim <[email protected]> Reported-by: Dave Jones <[email protected]> Reported-by: Tetsuo Handa <[email protected]> Reviewed-by: Vladimir Davydov <[email protected]> Cc: Christoph Lameter <[email protected]> Cc: Pekka Enberg <[email protected]> Cc: David Rientjes <[email protected]> Signed-off-by: Andrew Morton <[email protected]> Signed-off-by: Linus Torvalds <[email protected]> Signed-off-by: Greg Kroah-Hartman <[email protected]>
1 parent 347f31e commit d35426e

File tree

1 file changed

+68
-21
lines changed

1 file changed

+68
-21
lines changed

mm/slab.c

Lines changed: 68 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -375,6 +375,39 @@ static void **dbg_userword(struct kmem_cache *cachep, void *objp)
375375

376376
#endif
377377

378+
#define OBJECT_FREE (0)
379+
#define OBJECT_ACTIVE (1)
380+
381+
#ifdef CONFIG_DEBUG_SLAB_LEAK
382+
383+
static void set_obj_status(struct page *page, int idx, int val)
384+
{
385+
int freelist_size;
386+
char *status;
387+
struct kmem_cache *cachep = page->slab_cache;
388+
389+
freelist_size = cachep->num * sizeof(unsigned int);
390+
status = (char *)page->freelist + freelist_size;
391+
status[idx] = val;
392+
}
393+
394+
static inline unsigned int get_obj_status(struct page *page, int idx)
395+
{
396+
int freelist_size;
397+
char *status;
398+
struct kmem_cache *cachep = page->slab_cache;
399+
400+
freelist_size = cachep->num * sizeof(unsigned int);
401+
status = (char *)page->freelist + freelist_size;
402+
403+
return status[idx];
404+
}
405+
406+
#else
407+
static inline void set_obj_status(struct page *page, int idx, int val) {}
408+
409+
#endif
410+
378411
/*
379412
* Do not go above this order unless 0 objects fit into the slab or
380413
* overridden on the command line.
@@ -565,9 +598,18 @@ static inline struct array_cache *cpu_cache_get(struct kmem_cache *cachep)
565598
return cachep->array[smp_processor_id()];
566599
}
567600

568-
static size_t slab_mgmt_size(size_t nr_objs, size_t align)
601+
static size_t calculate_freelist_size(int nr_objs, size_t align)
569602
{
570-
return ALIGN(nr_objs * sizeof(unsigned int), align);
603+
size_t freelist_size;
604+
605+
freelist_size = nr_objs * sizeof(unsigned int);
606+
if (IS_ENABLED(CONFIG_DEBUG_SLAB_LEAK))
607+
freelist_size += nr_objs * sizeof(char);
608+
609+
if (align)
610+
freelist_size = ALIGN(freelist_size, align);
611+
612+
return freelist_size;
571613
}
572614

573615
/*
@@ -600,6 +642,10 @@ static void cache_estimate(unsigned long gfporder, size_t buffer_size,
600642
nr_objs = slab_size / buffer_size;
601643

602644
} else {
645+
int extra_space = 0;
646+
647+
if (IS_ENABLED(CONFIG_DEBUG_SLAB_LEAK))
648+
extra_space = sizeof(char);
603649
/*
604650
* Ignore padding for the initial guess. The padding
605651
* is at most @align-1 bytes, and @buffer_size is at
@@ -608,17 +654,18 @@ static void cache_estimate(unsigned long gfporder, size_t buffer_size,
608654
* into the memory allocation when taking the padding
609655
* into account.
610656
*/
611-
nr_objs = (slab_size) / (buffer_size + sizeof(unsigned int));
657+
nr_objs = (slab_size) /
658+
(buffer_size + sizeof(unsigned int) + extra_space);
612659

613660
/*
614661
* This calculated number will be either the right
615662
* amount, or one greater than what we want.
616663
*/
617-
if (slab_mgmt_size(nr_objs, align) + nr_objs*buffer_size
618-
> slab_size)
664+
if (calculate_freelist_size(nr_objs, align) >
665+
slab_size - nr_objs * buffer_size)
619666
nr_objs--;
620667

621-
mgmt_size = slab_mgmt_size(nr_objs, align);
668+
mgmt_size = calculate_freelist_size(nr_objs, align);
622669
}
623670
*num = nr_objs;
624671
*left_over = slab_size - nr_objs*buffer_size - mgmt_size;
@@ -2011,13 +2058,16 @@ static size_t calculate_slab_order(struct kmem_cache *cachep,
20112058
continue;
20122059

20132060
if (flags & CFLGS_OFF_SLAB) {
2061+
size_t freelist_size_per_obj = sizeof(unsigned int);
20142062
/*
20152063
* Max number of objs-per-slab for caches which
20162064
* use off-slab slabs. Needed to avoid a possible
20172065
* looping condition in cache_grow().
20182066
*/
2067+
if (IS_ENABLED(CONFIG_DEBUG_SLAB_LEAK))
2068+
freelist_size_per_obj += sizeof(char);
20192069
offslab_limit = size;
2020-
offslab_limit /= sizeof(unsigned int);
2070+
offslab_limit /= freelist_size_per_obj;
20212071

20222072
if (num > offslab_limit)
20232073
break;
@@ -2258,8 +2308,7 @@ __kmem_cache_create (struct kmem_cache *cachep, unsigned long flags)
22582308
if (!cachep->num)
22592309
return -E2BIG;
22602310

2261-
freelist_size =
2262-
ALIGN(cachep->num * sizeof(unsigned int), cachep->align);
2311+
freelist_size = calculate_freelist_size(cachep->num, cachep->align);
22632312

22642313
/*
22652314
* If the slab has been placed off-slab, and we have enough space then
@@ -2272,7 +2321,7 @@ __kmem_cache_create (struct kmem_cache *cachep, unsigned long flags)
22722321

22732322
if (flags & CFLGS_OFF_SLAB) {
22742323
/* really off slab. No need for manual alignment */
2275-
freelist_size = cachep->num * sizeof(unsigned int);
2324+
freelist_size = calculate_freelist_size(cachep->num, 0);
22762325

22772326
#ifdef CONFIG_PAGE_POISONING
22782327
/* If we're going to use the generic kernel_map_pages()
@@ -2589,6 +2638,7 @@ static void cache_init_objs(struct kmem_cache *cachep,
25892638
if (cachep->ctor)
25902639
cachep->ctor(objp);
25912640
#endif
2641+
set_obj_status(page, i, OBJECT_FREE);
25922642
slab_freelist(page)[i] = i;
25932643
}
25942644
}
@@ -2797,6 +2847,7 @@ static void *cache_free_debugcheck(struct kmem_cache *cachep, void *objp,
27972847
BUG_ON(objnr >= cachep->num);
27982848
BUG_ON(objp != index_to_obj(cachep, page, objnr));
27992849

2850+
set_obj_status(page, objnr, OBJECT_FREE);
28002851
if (cachep->flags & SLAB_POISON) {
28012852
#ifdef CONFIG_DEBUG_PAGEALLOC
28022853
if ((cachep->size % PAGE_SIZE)==0 && OFF_SLAB(cachep)) {
@@ -2930,6 +2981,8 @@ static inline void cache_alloc_debugcheck_before(struct kmem_cache *cachep,
29302981
static void *cache_alloc_debugcheck_after(struct kmem_cache *cachep,
29312982
gfp_t flags, void *objp, unsigned long caller)
29322983
{
2984+
struct page *page;
2985+
29332986
if (!objp)
29342987
return objp;
29352988
if (cachep->flags & SLAB_POISON) {
@@ -2960,6 +3013,9 @@ static void *cache_alloc_debugcheck_after(struct kmem_cache *cachep,
29603013
*dbg_redzone1(cachep, objp) = RED_ACTIVE;
29613014
*dbg_redzone2(cachep, objp) = RED_ACTIVE;
29623015
}
3016+
3017+
page = virt_to_head_page(objp);
3018+
set_obj_status(page, obj_to_index(cachep, page, objp), OBJECT_ACTIVE);
29633019
objp += obj_offset(cachep);
29643020
if (cachep->ctor && cachep->flags & SLAB_POISON)
29653021
cachep->ctor(objp);
@@ -4201,21 +4257,12 @@ static void handle_slab(unsigned long *n, struct kmem_cache *c,
42014257
struct page *page)
42024258
{
42034259
void *p;
4204-
int i, j;
4260+
int i;
42054261

42064262
if (n[0] == n[1])
42074263
return;
42084264
for (i = 0, p = page->s_mem; i < c->num; i++, p += c->size) {
4209-
bool active = true;
4210-
4211-
for (j = page->active; j < c->num; j++) {
4212-
/* Skip freed item */
4213-
if (slab_freelist(page)[j] == i) {
4214-
active = false;
4215-
break;
4216-
}
4217-
}
4218-
if (!active)
4265+
if (get_obj_status(page, i) != OBJECT_ACTIVE)
42194266
continue;
42204267

42214268
if (!add_caller(n, (unsigned long)*dbg_userword(c, p)))

0 commit comments

Comments
 (0)