On Wed, May 14, 2014 at 03:28:02PM -0400, Ted Unangst wrote: > As I learned the hard way not long ago, free() doesn't detect all > errors because of the delay mechanism. We can make two improvements. > > 1. Perform the sanity checking from free_bytes before we insert > something into the delay array. This detects many kinds of badness > much sooner. > > 2. Check that the freed pointer isn't the same as the pointer in the > delay array. Checking the entire array would be more complete, but > slower. Randomly crashing immediately is a modest improvement.
I think you can return i in check_free_chunk() and use that in free_bytes(). Like this, -Otto Index: malloc.c =================================================================== RCS file: /cvs/src/lib/libc/stdlib/malloc.c,v retrieving revision 1.163 diff -u -p -r1.163 malloc.c --- malloc.c 12 May 2014 19:02:20 -0000 1.163 +++ malloc.c 16 May 2014 18:00:19 -0000 @@ -966,16 +966,11 @@ malloc_bytes(struct dir_info *d, size_t return ((char *)bp->page + k); } - -/* - * Free a chunk, and possibly the page it's on, if the page becomes empty. - */ -static void -free_bytes(struct dir_info *d, struct region_info *r, void *ptr) +static int +check_free_chunk(struct dir_info *d, struct region_info *r, void *ptr) { - struct chunk_head *mp; struct chunk_info *info; - int i, listnum; + int i; info = (struct chunk_info *)r->size; if (info->canary != d->canary1) @@ -986,12 +981,27 @@ free_bytes(struct dir_info *d, struct re if ((uintptr_t)ptr & ((1U << (info->shift)) - 1)) { wrterror("modified chunk-pointer", ptr); - return; + return i; } if (info->bits[i / MALLOC_BITS] & (1U << (i % MALLOC_BITS))) { wrterror("chunk is already free", ptr); - return; + return i; } + return i; +} + +/* + * Free a chunk, and possibly the page it's on, if the page becomes empty. + */ +static void +free_bytes(struct dir_info *d, struct region_info *r, void *ptr) +{ + struct chunk_head *mp; + struct chunk_info *info; + int i, listnum; + + info = (struct chunk_info *)r->size; + i = check_free_chunk(d, r, ptr); info->bits[i / MALLOC_BITS] |= 1U << (i % MALLOC_BITS); info->free++; @@ -1204,9 +1214,14 @@ ofree(void *p) if (mopts.malloc_junk && sz > 0) memset(p, SOME_FREEJUNK, sz); if (!mopts.malloc_freenow) { + check_free_chunk(g_pool, r, p); i = getrbyte() & MALLOC_DELAYED_CHUNK_MASK; tmp = p; p = g_pool->delayed_chunks[i]; + if (tmp == p) { + wrterror("double free", p); + return; + } g_pool->delayed_chunks[i] = tmp; } if (p != NULL) {