From: Manfred Spraul The patch is designed improve the diagnostics which are presented when the slab memory poison detector triggers. check_poison_obj checks for write accesses after kfree by comparing the object contents with the poison value. The current implementation contains several flaws: - it accepts both POISON_BEFORE and POISON_AFTER. check_poison_obj is only called with POISON_AFTER poison bytes. Fix: only accept POISON_AFTER. - the output is unreadable. Fix: use hexdump. - if a large objects is corrupted, then the relevant lines can scroll of the screen/dmesg buffer. Fix: line limit. - it can access addresses behind the end of the object, which can oops with CONFIG_DEBUG_PAGEALLOC. Fix: bounds checks. Additionally, the patch contains the following changes: - rename POISON_BEFORE and POISON_AFTER to POISON_FREE and POISON_INUSE. The old names are ambiguous. - use the new hexdump object function in ptrinfo. - store_stackinfo was called with wrong parameters: it should store caller, i.e. __builtin_return_address(0), not POISON_AFTER in the object. - dump both the object before and after the corrupted one, not just the one after. Example output: <<< Slab corruption: start=194e708c, len=2048 Redzone: 0x5a2cf071/0x5a2cf071. Last user: [<02399d7c>](dummy_init_module+0x1c/0xb0) 010: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 7b 030: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 63 Prev obj: start=194e6880, len=2048 Redzone: 0x5a2cf071/0x5a2cf071. Last user: [<00000000>](0x0) 000: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 010: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b <<< --- mm/slab.c | 158 ++++++++++++++++++++++++++++++++++++++------------------------ 1 files changed, 98 insertions(+), 60 deletions(-) diff -puN mm/slab.c~slab-poison-hex-dumping mm/slab.c --- 25/mm/slab.c~slab-poison-hex-dumping 2004-01-20 20:49:53.000000000 -0800 +++ 25-akpm/mm/slab.c 2004-01-20 20:49:53.000000000 -0800 @@ -357,8 +357,8 @@ struct kmem_cache_s { #define RED_ACTIVE 0x170FC2A5UL /* when obj is active */ /* ...and for poisoning */ -#define POISON_BEFORE 0x5a /* for use-uninitialised poisoning */ -#define POISON_AFTER 0x6b /* for use-after-free poisoning */ +#define POISON_INUSE 0x5a /* for use-uninitialised poisoning */ +#define POISON_FREE 0x6b /* for use-after-free poisoning */ #define POISON_END 0xa5 /* end-byte of poisoning */ /* memory layout of objects: @@ -887,60 +887,105 @@ static void poison_obj(kmem_cache_t *cac *(unsigned char *)(addr+size-1) = POISON_END; } -static void *scan_poisoned_obj(unsigned char* addr, unsigned int size) +static void dump_line(char *data, int offset, int limit) { - unsigned char *end; - - end = addr + size - 1; - - for (; addr < end; addr++) { - if (*addr != POISON_BEFORE && *addr != POISON_AFTER) - return addr; + int i; + printk(KERN_ERR "%03x:", offset); + for (i=0;iflags & SLAB_RED_ZONE) { + printk(KERN_ERR "Redzone: 0x%lx/0x%lx.\n", + *dbg_redzone1(cachep, objp), + *dbg_redzone2(cachep, objp)); + } - end = scan_poisoned_obj(realobj, size); - if (end) { - int s; - printk(KERN_ERR "Slab corruption: start=%p, expend=%p, " - "problemat=%p\n", realobj, realobj+size-1, end); - if (cachep->flags & SLAB_STORE_USER) { - printk(KERN_ERR "Last user: [<%p>]", *dbg_userword(cachep, objp)); - print_symbol("(%s)", (unsigned long)*dbg_userword(cachep, objp)); - printk("\n"); - } - printk(KERN_ERR "Data: "); - for (s = 0; s < size; s++) { - if (((char*)realobj)[s] == POISON_BEFORE) - printk("."); - else if (((char*)realobj)[s] == POISON_AFTER) - printk("*"); - else - printk("%02X ", ((unsigned char*)realobj)[s]); - } + if (cachep->flags & SLAB_STORE_USER) { + printk(KERN_ERR "Last user: [<%p>]", *dbg_userword(cachep, objp)); + print_symbol("(%s)", (unsigned long)*dbg_userword(cachep, objp)); printk("\n"); - printk(KERN_ERR "Next: "); - for (; s < size + 32; s++) { - if (((char*)realobj)[s] == POISON_BEFORE) - printk("."); - else if (((char*)realobj)[s] == POISON_AFTER) - printk("*"); - else - printk("%02X ", ((unsigned char*)realobj)[s]); + } + realobj = (char*)objp+obj_dbghead(cachep); + size = cachep->objsize; + for (i=0; i size) + limit = size-i; + dump_line(realobj, i, limit); + } +#endif +} + +#if DEBUG + +static void check_poison_obj(kmem_cache_t *cachep, void *objp) +{ + char *realobj; + int size, i; + int lines = 0; + + realobj = (char*)objp+obj_dbghead(cachep); + size = obj_reallen(cachep); + + for (i=0;i size) + limit = size-i; + dump_line(realobj, i, limit); + i += 16; + lines++; + /* Limit to 5 lines */ + if (lines > 5) + break; + } + } + if (lines != 0) { + /* Print some data about the neighboring objects, if they + * exist: + */ + struct slab *slabp = GET_PAGE_SLAB(virt_to_page(objp)); + int objnr; + + objnr = (objp-slabp->s_mem)/cachep->objsize; + if (objnr) { + objp = slabp->s_mem+(objnr-1)*cachep->objsize; + realobj = (char*)objp+obj_dbghead(cachep); + printk(KERN_ERR "Prev obj: start=%p, len=%d\n", + realobj, size); + print_objinfo(cachep, objp, 2); + } + if (objnr+1 < cachep->num) { + objp = slabp->s_mem+(objnr+1)*cachep->objsize; + realobj = (char*)objp+obj_dbghead(cachep); + printk(KERN_ERR "Next obj: start=%p, len=%d\n", + realobj, size); + print_objinfo(cachep, objp, 2); } - printk("\n"); - slab_error(cachep, "object was modified after freeing"); } } #endif @@ -1493,7 +1538,7 @@ static void cache_init_objs (kmem_cache_ #if DEBUG /* need to poison the objs? */ if (cachep->flags & SLAB_POISON) - poison_obj(cachep, objp, POISON_BEFORE); + poison_obj(cachep, objp, POISON_FREE); if (cachep->flags & SLAB_STORE_USER) *dbg_userword(cachep, objp) = NULL; @@ -1712,13 +1757,13 @@ static inline void *cache_free_debugchec if (cachep->flags & SLAB_POISON) { #ifdef CONFIG_DEBUG_PAGEALLOC if ((cachep->objsize % PAGE_SIZE) == 0 && OFF_SLAB(cachep)) { - store_stackinfo(cachep, objp, POISON_AFTER); + store_stackinfo(cachep, objp, (unsigned long)caller); kernel_map_pages(virt_to_page(objp), cachep->objsize/PAGE_SIZE, 0); } else { - poison_obj(cachep, objp, POISON_AFTER); + poison_obj(cachep, objp, POISON_FREE); } #else - poison_obj(cachep, objp, POISON_AFTER); + poison_obj(cachep, objp, POISON_FREE); #endif } #endif @@ -1875,7 +1920,7 @@ cache_alloc_debugcheck_after(kmem_cache_ #else check_poison_obj(cachep, objp); #endif - poison_obj(cachep, objp, POISON_BEFORE); + poison_obj(cachep, objp, POISON_INUSE); } if (cachep->flags & SLAB_STORE_USER) *dbg_userword(cachep, objp) = caller; @@ -2860,14 +2905,7 @@ void ptrinfo(unsigned long addr) kernel_map_pages(virt_to_page(objp), c->objsize/PAGE_SIZE, 1); - if (c->flags & SLAB_RED_ZONE) - printk("redzone: 0x%lx/0x%lx.\n", - *dbg_redzone1(c, objp), - *dbg_redzone2(c, objp)); - - if (c->flags & SLAB_STORE_USER) - printk("Last user: %p.\n", - *dbg_userword(c, objp)); + print_objinfo(c, objp, 2); } spin_unlock_irqrestore(&c->spinlock, flags); _