[PATCH] slab: hexdump for check_poison

From: Manfred Spraul <manfred@colorfullife.com>

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
<<<

diff --git a/mm/slab.c b/mm/slab.c
index 435e5a15fc51fcaf8632f78320acb065a07acf21..d6a09da814b0704a4f8e83f58c0d9cd344074081 100644 (file)
--- a/mm/slab.c
+++ b/mm/slab.c
@@ -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 *cachep, void *addr, unsigned char val)
        *(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;
+       int i;
+       printk(KERN_ERR "%03x:", offset);
+       for (i=0;i<limit;i++) {
+               printk(" %02x", (unsigned char)data[offset+i]);
+       }
+       printk("\n");
+}
+#endif
+
+static void print_objinfo(kmem_cache_t *cachep, void *objp, int lines)
+{
+#if DEBUG
+       int i, size;
+       char *realobj;
 
-       for (; addr < end; addr++) {
-               if (*addr != POISON_BEFORE && *addr != POISON_AFTER)
-                       return addr;
+       if (cachep->flags & SLAB_RED_ZONE) {
+               printk(KERN_ERR "Redzone: 0x%lx/0x%lx.\n",
+                       *dbg_redzone1(cachep, objp),
+                       *dbg_redzone2(cachep, objp));
        }
-       if (*addr != POISON_END)
-               return addr;
-       return NULL;
+
+       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");
+       }
+       realobj = (char*)objp+obj_dbghead(cachep);
+       size = cachep->objsize;
+       for (i=0; i<size && lines;i+=16, lines--) {
+               int limit;
+               limit = 16;
+               if (i+limit > size)
+                       limit = size-i;
+               dump_line(realobj, i, limit);
+       }
+#endif
 }
 
+#if DEBUG
+
 static void check_poison_obj(kmem_cache_t *cachep, void *objp)
 {
-       void *end;
-       void *realobj;
-       int size = obj_reallen(cachep);
-
-       realobj = objp+obj_dbghead(cachep);
-
-       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");
+       char *realobj;
+       int size, i;
+       int lines = 0;
+
+       realobj = (char*)objp+obj_dbghead(cachep);
+       size = obj_reallen(cachep);
+
+       for (i=0;i<size;i++) {
+               char exp = POISON_FREE;
+               if (i == size-1)
+                       exp = POISON_END;
+               if (realobj[i] != exp) {
+                       int limit;
+                       /* Mismatch ! */
+                       /* Print header */
+                       if (lines == 0) {
+                               printk(KERN_ERR "Slab corruption: start=%p, len=%d\n",
+                                               realobj, size);
+                               print_objinfo(cachep, objp, 0);
+                       }
+                       /* Hexdump the affected line */
+                       i = (i/16)*16;
+                       limit = 16;
+                       if (i+limit > size)
+                               limit = size-i;
+                       dump_line(realobj, i, limit);
+                       i += 16;
+                       lines++;
+                       /* Limit to 5 lines */
+                       if (lines > 5)
+                               break;
                }
-               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 (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);
                }
-               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]);
+               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
@@ -1495,7 +1540,7 @@ static void cache_init_objs (kmem_cache_t * cachep,
 #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;
 
@@ -1714,13 +1759,13 @@ static inline void *cache_free_debugcheck (kmem_cache_t * cachep, void * objp, v
        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
@@ -1877,7 +1922,7 @@ cache_alloc_debugcheck_after(kmem_cache_t *cachep,
 #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;
@@ -2868,14 +2913,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);