From: Manfred Spraul <manfred@colorfullife.com>
The real memory allocation is usually larger than the actual object size:
either due to L1 cache line padding, or due to page padding with
CONFIG_DEBUG_PAGEALLOC. Right now objects are placed to the beginning of
the real allocation, but to trigger bugs it's better to move objects to the
end of the real allocation: that way accesses behind the end of the
allocation have a larger chance of hitting the (unmapped) next page. The
attached patch moves the objects to align them with the end of the real
allocation.
Actually it contains 4 seperate changes:
- Do not page-pad allocations that are <= SMP_CACHE_LINE_SIZE. This
crashes. Right now the limit is hardcoded to 128 bytes, but sooner or
later an arch will appear with 256 byte cache lines.
- cleanup: redzone bytes are not accessed with inline helper functions,
instead of magic offsets scattered throughout slab.c
- main change: move objects to the end of the allocation - trivial after
the cleanup.
- Print old redzone value if a redzone mismatch happens: This makes it
simpler to figure out what happened [single bit error, wrong redzone
code, overwritten]
DESC
slab debug fix
EDESC
From: Joshua Kwan <joshk@triplehelix.org>
Needs the following patch to compile:
25-akpm/mm/slab.c | 207 ++++++++++++++++++++++++++++--------------------------
1 files changed, 109 insertions(+), 98 deletions(-)
diff -puN mm/slab.c~slab-debug-additions mm/slab.c
--- 25/mm/slab.c~slab-debug-additions Thu Sep 11 10:29:25 2003
+++ 25-akpm/mm/slab.c Thu Sep 11 10:48:55 2003
@@ -294,6 +294,10 @@ struct kmem_cache_s {
atomic_t freehit;
atomic_t freemiss;
#endif
+#if DEBUG
+ int dbghead;
+ int reallen;
+#endif
};
#define CFLGS_OFF_SLAB (0x80000000UL)
@@ -357,33 +361,54 @@ struct kmem_cache_s {
#define POISON_AFTER 0x6b /* for use-after-free poisoning */
#define POISON_END 0xa5 /* end-byte of poisoning */
+/* memory layout of objects:
+ * 0 : objp
+ * 0 .. cachep->dbghead - BYTES_PER_WORD - 1: padding. This ensures that
+ * the end of an object is aligned with the end of the real
+ * allocation. Catches writes behind the end of the allocation.
+ * cachep->dbghead - BYTES_PER_WORD .. cachep->dbghead - 1:
+ * redzone word.
+ * cachep->dbghead: The real object.
+ * cachep->objsize - 2* BYTES_PER_WORD: redzone word [BYTES_PER_WORD long]
+ * cachep->objsize - 1* BYTES_PER_WORD: last caller address [BYTES_PER_WORD long]
+ */
static inline int obj_dbghead(kmem_cache_t *cachep)
{
- if (cachep->flags & SLAB_RED_ZONE)
- return BYTES_PER_WORD;
- return 0;
+ return cachep->dbghead;
}
-static inline int obj_dbglen(kmem_cache_t *cachep)
+static inline int obj_reallen(kmem_cache_t *cachep)
{
- int len = 0;
+ return cachep->reallen;
+}
- if (cachep->flags & SLAB_RED_ZONE) {
- len += 2*BYTES_PER_WORD;
- }
- if (cachep->flags & SLAB_STORE_USER) {
- len += BYTES_PER_WORD;
- }
- return len;
+static unsigned long *dbg_redzone1(kmem_cache_t *cachep, void *objp)
+{
+ BUG_ON(!(cachep->flags & SLAB_RED_ZONE));
+ return (unsigned long*) (objp+obj_dbghead(cachep)-BYTES_PER_WORD);
+}
+
+static unsigned long *dbg_redzone2(kmem_cache_t *cachep, void *objp)
+{
+ BUG_ON(!(cachep->flags & SLAB_RED_ZONE));
+ if (cachep->flags & SLAB_STORE_USER)
+ return (unsigned long*) (objp+cachep->objsize-2*BYTES_PER_WORD);
+ return (unsigned long*) (objp+cachep->objsize-BYTES_PER_WORD);
+}
+
+static void **dbg_userword(kmem_cache_t *cachep, void *objp)
+{
+ BUG_ON(!(cachep->flags & SLAB_STORE_USER));
+ return (void**)(objp+cachep->objsize-BYTES_PER_WORD);
}
#else
static inline int obj_dbghead(kmem_cache_t *cachep)
{
return 0;
}
-static inline int obj_dbglen(kmem_cache_t *cachep)
+static inline int obj_reallen(kmem_cache_t *cachep)
{
- return 0;
+ return cachep->objsize;
}
#endif
@@ -805,7 +830,7 @@ static inline void kmem_freepages (kmem_
#ifdef CONFIG_DEBUG_PAGEALLOC
static void store_stackinfo(kmem_cache_t *cachep, unsigned long *addr, unsigned long caller)
{
- int size = cachep->objsize-obj_dbglen(cachep);
+ int size = obj_reallen(cachep);
addr = (unsigned long *)&((char*)addr)[obj_dbghead(cachep)];
@@ -837,7 +862,7 @@ static void store_stackinfo(kmem_cache_t
static void poison_obj(kmem_cache_t *cachep, void *addr, unsigned char val)
{
- int size = cachep->objsize-obj_dbglen(cachep);
+ int size = obj_reallen(cachep);
addr = &((char*)addr)[obj_dbghead(cachep)];
memset(addr, val, size);
@@ -859,47 +884,42 @@ static void *scan_poisoned_obj(unsigned
return NULL;
}
-static void check_poison_obj(kmem_cache_t *cachep, void *addr)
+static void check_poison_obj(kmem_cache_t *cachep, void *objp)
{
void *end;
- int size = cachep->objsize-obj_dbglen(cachep);
+ void *realobj;
+ int size = obj_reallen(cachep);
- addr = &((char*)addr)[obj_dbghead(cachep)];
+ realobj = objp+obj_dbghead(cachep);
- end = scan_poisoned_obj(addr, size);
+ end = scan_poisoned_obj(realobj, size);
if (end) {
int s;
printk(KERN_ERR "Slab corruption: start=%p, expend=%p, "
- "problemat=%p\n", addr, addr+size-1, end);
+ "problemat=%p\n", realobj, realobj+size-1, end);
if (cachep->flags & SLAB_STORE_USER) {
- void *pc;
-
- if (cachep->flags & SLAB_RED_ZONE)
- pc = *(void**)(addr+size+BYTES_PER_WORD);
- else
- pc = *(void**)(addr+size);
- printk(KERN_ERR "Last user: [<%p>]", pc);
- print_symbol("(%s)", (unsigned long)pc);
+ 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*)addr)[s] == POISON_BEFORE)
+ if (((char*)realobj)[s] == POISON_BEFORE)
printk(".");
- else if (((char*)addr)[s] == POISON_AFTER)
+ else if (((char*)realobj)[s] == POISON_AFTER)
printk("*");
else
- printk("%02X ", ((unsigned char*)addr)[s]);
+ printk("%02X ", ((unsigned char*)realobj)[s]);
}
printk("\n");
printk(KERN_ERR "Next: ");
for (; s < size + 32; s++) {
- if (((char*)addr)[s] == POISON_BEFORE)
+ if (((char*)realobj)[s] == POISON_BEFORE)
printk(".");
- else if (((char*)addr)[s] == POISON_AFTER)
+ else if (((char*)realobj)[s] == POISON_AFTER)
printk("*");
else
- printk("%02X ", ((unsigned char*)addr)[s]);
+ printk("%02X ", ((unsigned char*)realobj)[s]);
}
printk("\n");
slab_error(cachep, "object was modified after freeing");
@@ -917,7 +937,6 @@ static void slab_destroy (kmem_cache_t *
int i;
for (i = 0; i < cachep->num; i++) {
void *objp = slabp->s_mem + cachep->objsize * i;
- int objlen = cachep->objsize;
if (cachep->flags & SLAB_POISON) {
#ifdef CONFIG_DEBUG_PAGEALLOC
@@ -929,21 +948,16 @@ static void slab_destroy (kmem_cache_t *
check_poison_obj(cachep, objp);
#endif
}
- if (cachep->flags & SLAB_STORE_USER)
- objlen -= BYTES_PER_WORD;
-
if (cachep->flags & SLAB_RED_ZONE) {
- if (*((unsigned long*)(objp)) != RED_INACTIVE)
+ if (*dbg_redzone1(cachep, objp) != RED_INACTIVE)
slab_error(cachep, "start of a freed object "
"was overwritten");
- if (*((unsigned long*)(objp + objlen - BYTES_PER_WORD))
- != RED_INACTIVE)
+ if (*dbg_redzone2(cachep, objp) != RED_INACTIVE)
slab_error(cachep, "end of a freed object "
"was overwritten");
- objp += BYTES_PER_WORD;
}
if (cachep->dtor && !(cachep->flags & SLAB_POISON))
- (cachep->dtor)(objp, cachep, 0);
+ (cachep->dtor)(objp+obj_dbghead(cachep), cachep, 0);
}
#else
if (cachep->dtor) {
@@ -1021,10 +1035,6 @@ kmem_cache_create (const char *name, siz
}
#if FORCED_DEBUG
-#ifdef CONFIG_DEBUG_PAGEALLOC
- if (size < PAGE_SIZE-3*BYTES_PER_WORD && size > 128)
- size = PAGE_SIZE-3*BYTES_PER_WORD;
-#endif
/*
* Enable redzoning and last user accounting, except
* - for caches with forced alignment: redzoning would violate the
@@ -1055,6 +1065,9 @@ kmem_cache_create (const char *name, siz
goto opps;
memset(cachep, 0, sizeof(kmem_cache_t));
+#if DEBUG
+ cachep->reallen = size;
+#endif
/* Check that size is in terms of words. This is needed to avoid
* unaligned accesses for some archs when redzoning is used, and makes
* sure any on-slab bufctl's are also correctly aligned.
@@ -1072,13 +1085,21 @@ kmem_cache_create (const char *name, siz
* when redzoning.
*/
flags &= ~SLAB_HWCACHE_ALIGN;
- size += 2*BYTES_PER_WORD; /* words for redzone */
+ /* add space for red zone words */
+ cachep->dbghead += BYTES_PER_WORD;
+ size += 2*BYTES_PER_WORD;
}
if (flags & SLAB_STORE_USER) {
flags &= ~SLAB_HWCACHE_ALIGN;
- size += BYTES_PER_WORD; /* word for kfree caller address */
+ size += BYTES_PER_WORD; /* add space */
+ }
+#if FORCED_DEBUG && defined(CONFIG_DEBUG_PAGEALLOC)
+ if (size > 128 && cachep->reallen > L1_CACHE_BYTES && size < PAGE_SIZE) {
+ cachep->dbghead += PAGE_SIZE - size;
+ size = PAGE_SIZE;
}
#endif
+#endif
align = BYTES_PER_WORD;
if (flags & SLAB_HWCACHE_ALIGN)
align = L1_CACHE_BYTES;
@@ -1445,20 +1466,15 @@ static void cache_init_objs (kmem_cache_
for (i = 0; i < cachep->num; i++) {
void* objp = slabp->s_mem+cachep->objsize*i;
#if DEBUG
- int objlen = cachep->objsize;
/* need to poison the objs? */
if (cachep->flags & SLAB_POISON)
poison_obj(cachep, objp, POISON_BEFORE);
- if (cachep->flags & SLAB_STORE_USER) {
- objlen -= BYTES_PER_WORD;
- ((unsigned long*)(objp+objlen))[0] = 0;
- }
+ if (cachep->flags & SLAB_STORE_USER)
+ *dbg_userword(cachep, objp) = NULL;
if (cachep->flags & SLAB_RED_ZONE) {
- *((unsigned long*)(objp)) = RED_INACTIVE;
- objp += BYTES_PER_WORD;
- objlen -= 2* BYTES_PER_WORD;
- *((unsigned long*)(objp + objlen)) = RED_INACTIVE;
+ *dbg_redzone1(cachep, objp) = RED_INACTIVE;
+ *dbg_redzone2(cachep, objp) = RED_INACTIVE;
}
/*
* Constructors are not allowed to allocate memory from
@@ -1466,14 +1482,13 @@ static void cache_init_objs (kmem_cache_
* Otherwise, deadlock. They must also be threaded.
*/
if (cachep->ctor && !(cachep->flags & SLAB_POISON))
- cachep->ctor(objp, cachep, ctor_flags);
+ cachep->ctor(objp+obj_dbghead(cachep), cachep, ctor_flags);
if (cachep->flags & SLAB_RED_ZONE) {
- if (*((unsigned long*)(objp + objlen)) != RED_INACTIVE)
+ if (*dbg_redzone2(cachep, objp) != RED_INACTIVE)
slab_error(cachep, "constructor overwrote the"
" end of an object");
- objp -= BYTES_PER_WORD;
- if (*((unsigned long*)(objp)) != RED_INACTIVE)
+ if (*dbg_redzone1(cachep, objp) != RED_INACTIVE)
slab_error(cachep, "constructor overwrote the"
" start of an object");
}
@@ -1624,9 +1639,9 @@ static inline void *cache_free_debugchec
#if DEBUG
struct page *page;
unsigned int objnr;
- int objlen = cachep->objsize;
struct slab *slabp;
+ objp -= obj_dbghead(cachep);
kfree_debugcheck(objp);
page = virt_to_page(objp);
@@ -1639,21 +1654,18 @@ static inline void *cache_free_debugchec
}
slabp = GET_PAGE_SLAB(page);
- if (cachep->flags & SLAB_STORE_USER) {
- objlen -= BYTES_PER_WORD;
- }
if (cachep->flags & SLAB_RED_ZONE) {
- objp -= BYTES_PER_WORD;
- if (xchg((unsigned long *)objp, RED_INACTIVE) != RED_ACTIVE)
- slab_error(cachep, "double free, or memory before"
+ if (*dbg_redzone1(cachep, objp) != RED_ACTIVE || *dbg_redzone2(cachep, objp) != RED_ACTIVE) {
+ slab_error(cachep, "double free, or memory outside"
" object was overwritten");
- if (xchg((unsigned long *)(objp+objlen-BYTES_PER_WORD), RED_INACTIVE) != RED_ACTIVE)
- slab_error(cachep, "double free, or memory after "
- " object was overwritten");
- }
- if (cachep->flags & SLAB_STORE_USER) {
- *((void**)(objp+objlen)) = caller;
+ printk(KERN_ERR "%p: redzone 1: 0x%lx, redzone 2: 0x%lx.\n",
+ objp, *dbg_redzone1(cachep, objp), *dbg_redzone2(cachep, objp));
+ }
+ *dbg_redzone1(cachep, objp) = RED_INACTIVE;
+ *dbg_redzone2(cachep, objp) = RED_INACTIVE;
}
+ if (cachep->flags & SLAB_STORE_USER)
+ *dbg_userword(cachep, objp) = caller;
objnr = (objp-slabp->s_mem)/cachep->objsize;
@@ -1829,8 +1841,6 @@ cache_alloc_debugcheck_after(kmem_cache_
unsigned long flags, void *objp, void *caller)
{
#if DEBUG
- int objlen = cachep->objsize;
-
if (!objp)
return objp;
if (cachep->flags & SLAB_POISON) {
@@ -1844,24 +1854,20 @@ cache_alloc_debugcheck_after(kmem_cache_
#endif
poison_obj(cachep, objp, POISON_BEFORE);
}
- if (cachep->flags & SLAB_STORE_USER) {
- objlen -= BYTES_PER_WORD;
- *((void **)(objp+objlen)) = caller;
- }
+ if (cachep->flags & SLAB_STORE_USER)
+ *dbg_userword(cachep, objp) = caller;
if (cachep->flags & SLAB_RED_ZONE) {
- /* Set alloc red-zone, and check old one. */
- if (xchg((unsigned long *)objp, RED_ACTIVE) != RED_INACTIVE) {
- slab_error(cachep, "memory before object was "
- "overwritten");
- }
- if (xchg((unsigned long *)(objp+objlen - BYTES_PER_WORD),
- RED_ACTIVE) != RED_INACTIVE) {
- slab_error(cachep, "memory after object was "
- "overwritten");
+ if (*dbg_redzone1(cachep, objp) != RED_INACTIVE || *dbg_redzone2(cachep, objp) != RED_INACTIVE) {
+ slab_error(cachep, "double free, or memory outside"
+ " object was overwritten");
+ printk(KERN_ERR "%p: redzone 1: 0x%lx, redzone 2: 0x%lx.\n",
+ objp, *dbg_redzone1(cachep, objp), *dbg_redzone2(cachep, objp));
}
- objp += BYTES_PER_WORD;
+ *dbg_redzone1(cachep, objp) = RED_ACTIVE;
+ *dbg_redzone2(cachep, objp) = RED_ACTIVE;
}
+ objp += obj_dbghead(cachep);
if (cachep->ctor && cachep->flags & SLAB_POISON) {
unsigned long ctor_flags = SLAB_CTOR_CONSTRUCTOR;
@@ -2186,7 +2192,7 @@ free_percpu(const void *objp)
unsigned int kmem_cache_size(kmem_cache_t *cachep)
{
- return cachep->objsize-obj_dbglen(cachep);
+ return obj_reallen(cachep);
}
kmem_cache_t * kmem_find_general_cachep (size_t size, int gfpflags)
@@ -2774,12 +2780,17 @@ void ptrinfo(unsigned long addr)
if (objnr >= c->num) {
printk("Bad obj number.\n");
} else {
- kernel_map_pages(virt_to_page(objp), c->objsize/PAGE_SIZE, 1);
+ kernel_map_pages(virt_to_page(objp),
+ c->objsize/PAGE_SIZE, 1);
- printk("redzone: %lxh/%lxh/%lxh.\n",
- ((unsigned long*)objp)[0],
- ((unsigned long*)(objp+c->objsize))[-2],
- ((unsigned long*)(objp+c->objsize))[-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));
}
spin_unlock_irqrestore(&c->spinlock, flags);
_