This is a kind of two steps forward, one step back diff.
I would like for the canary to be placed directly adjacent to the end of the
user specified size. No slack. To accomplish this, we record the original size
of the allocation at the end, then we can walk backwards to find the canary.
Mechanically, this requires passing the real size down a little deeper in some
places.
I also changed the canaries to be entirely random, with the correct value
stored in each chunk. This is a security tradeoff, since now an attacker can
overwrite the whole works predictably. But I consider this more of a debugging
feature than a security feature, so I'd prefer to greatly improve the ability
to catch one byte overwrites. A fair trade.
This required relaxing some of the junking code in places since we don't
always have the real size anymore, but I think this can be added back.
WIP, but I can confirm it detects errors like the following:
p = malloc(19);
p[19] = 'x';
free(p);
Index: stdlib/malloc.c
===
RCS file: /cvs/src/lib/libc/stdlib/malloc.c,v
retrieving revision 1.177
diff -u -p -r1.177 malloc.c
--- stdlib/malloc.c 9 Dec 2015 02:45:23 - 1.177
+++ stdlib/malloc.c 9 Dec 2015 12:35:41 -
@@ -84,6 +84,8 @@
#define SOME_JUNK 0xd0/* as in "Duh" :-) */
#define SOME_FREEJUNK 0xdf
+#define CANARYLEN 8
+
#define MMAP(sz) mmap(NULL, (size_t)(sz), PROT_READ | PROT_WRITE, \
MAP_ANON | MAP_PRIVATE, -1, (off_t) 0)
@@ -532,7 +534,8 @@ omalloc_init(struct dir_info **dp)
mopts.malloc_canaries = 0;
break;
case 'C':
- mopts.malloc_canaries = sizeof(void *);
+ mopts.malloc_canaries = sizeof(size_t) +
+ CANARYLEN * 2;
break;
#ifdef MALLOC_STATS
case 'd':
@@ -918,16 +921,17 @@ omalloc_make_chunks(struct dir_info *d,
* Allocate a chunk
*/
static void *
-malloc_bytes(struct dir_info *d, size_t size, void *f)
+malloc_bytes(struct dir_info *d, size_t size, size_t realsize, void *f)
{
int i, j, listnum;
- size_t k;
+ size_t k, junklen;
u_short u, *lp;
struct chunk_info *bp;
if (mopts.malloc_canary != (d->canary1 ^ (u_int32_t)(uintptr_t)d) ||
d->canary1 != ~d->canary2)
wrterror("internal struct corrupt", NULL);
+
/* Don't bother with anything less than this */
/* unless we have a malloc(0) requests */
if (size != 0 && size < MALLOC_MINSIZE)
@@ -995,15 +999,21 @@ malloc_bytes(struct dir_info *d, size_t
k += (lp - bp->bits) * MALLOC_BITS;
k <<= bp->shift;
+ junklen = bp->size;
if (mopts.malloc_canaries && bp->size > 0) {
- char *end = (char *)bp->page + k + bp->size;
- uintptr_t *canary = (uintptr_t *)(end - mopts.malloc_canaries);
- *canary = mopts.malloc_chunk_canary ^ hash(canary);
+ size_t *canarypos = (size_t *)((char *)bp->page + k + bp->size -
+ CANARYLEN - sizeof(size_t));
+ uint8_t *canaryval = (uint8_t *)((char *)bp->page + k +
bp->size -
+ CANARYLEN);
+ uint8_t *canary = (uint8_t *)((char *)bp->page + k + realsize);
+ *canarypos = realsize;
+ junklen = realsize;
+ arc4random_buf(canaryval, CANARYLEN);
+ memcpy(canary, canaryval, CANARYLEN);
}
if (mopts.malloc_junk == 2 && bp->size > 0)
- memset((char *)bp->page + k, SOME_JUNK,
- bp->size - mopts.malloc_canaries);
+ memset((char *)bp->page + k, SOME_JUNK, junklen);
return ((char *)bp->page + k);
}
@@ -1018,10 +1028,22 @@ find_chunknum(struct dir_info *d, struct
wrterror("chunk info corrupted", NULL);
if (mopts.malloc_canaries && info->size > 0) {
- char *end = (char *)ptr + info->size;
- uintptr_t *canary = (uintptr_t *)(end - mopts.malloc_canaries);
- if (*canary != (mopts.malloc_chunk_canary ^ hash(canary)))
+ size_t *canarypos = (size_t *)((char *)ptr + info->size -
+ CANARYLEN - sizeof(size_t));
+ uint8_t *canaryval = (uint8_t *)((char *)ptr + info->size -
+ CANARYLEN);
+ uint8_t *canary;
+
+ if (*canarypos > info->size - mopts.malloc_canaries) {
+ wrterror("corrupted canary len", ptr);
+ return -1;
+ }
+ canary = (uint8_t *)((char *)ptr + *canarypos);
+
+ if (memcmp(canaryval, canary, CANARYLEN) != 0) {
