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 -0000 1.177
+++ stdlib/malloc.c 9 Dec 2015 12:35:41 -0000
@@ -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) {
wrterror("chunk canary corrupted", ptr);
+ return -1;
+ }
}
/* Find the chunk number on the page */
@@ -1089,7 +1111,7 @@ free_bytes(struct dir_info *d, struct re
static void *
-omalloc(size_t sz, int zero_fill, void *f)
+omalloc(size_t sz, size_t realsz, int zero_fill, void *f)
{
struct dir_info *pool = getpool();
void *p;
@@ -1144,9 +1166,9 @@ omalloc(size_t sz, int zero_fill, void *
} else {
/* takes care of SOME_JUNK */
- p = malloc_bytes(pool, sz, f);
+ p = malloc_bytes(pool, sz, realsz, f);
if (zero_fill && p != NULL && sz > 0)
- memset(p, 0, sz - mopts.malloc_canaries);
+ memset(p, 0, realsz);
}
return p;
@@ -1189,6 +1211,7 @@ malloc(size_t size)
{
void *r;
int saved_errno = errno;
+ size_t realsize = size;
_MALLOC_LOCK();
malloc_func = "malloc():";
@@ -1203,7 +1226,7 @@ malloc(size_t size)
}
if (size > 0 && size <= MALLOC_MAXCHUNK)
size += mopts.malloc_canaries;
- r = omalloc(size, 0, CALLER);
+ r = omalloc(size, realsize, 0, CALLER);
malloc_active--;
_MALLOC_UNLOCK();
if (r == NULL && mopts.malloc_xmalloc) {
@@ -1224,12 +1247,12 @@ validate_junk(void *p) {
if (p == NULL)
return;
+ if (mopts.malloc_canaries)
+ return;
r = find(pool, p);
if (r == NULL)
wrterror("bogus pointer in validate_junk", p);
REALSIZE(sz, r);
- if (sz > 0 && sz <= MALLOC_MAXCHUNK)
- sz -= mopts.malloc_canaries;
if (sz > 32)
sz = 32;
for (byte = 0; byte < sz; byte++) {
@@ -1292,8 +1315,11 @@ ofree(void *p)
void *tmp;
int i;
- if (mopts.malloc_junk && sz > 0)
- memset(p, SOME_FREEJUNK, sz - mopts.malloc_canaries);
+ if (mopts.malloc_junk && sz > 0 && !mopts.malloc_canaries) {
+ size_t junklen = sz;
+ /* handle canary case */
+ memset(p, SOME_FREEJUNK, junklen);
+ }
if (!mopts.malloc_freenow) {
if (find_chunknum(pool, r, p) == -1)
return;
@@ -1352,11 +1378,11 @@ orealloc(void *p, size_t newsz, void *f)
{
struct dir_info *pool = getpool();
struct region_info *r;
- size_t oldsz, goldsz, gnewsz;
+ size_t oldsz, goldsz, gnewsz, realsz = newsz;
void *q;
if (p == NULL)
- return omalloc(newsz, 0, f);
+ return omalloc(newsz, realsz, 0, f);
r = find(pool, p);
if (r == NULL) {
@@ -1449,7 +1475,7 @@ gotit:
STATS_SETF(r, f);
return p;
} else if (newsz != oldsz || mopts.malloc_realloc) {
- q = omalloc(newsz, 0, f);
+ q = omalloc(newsz, realsz, 0, f);
if (q == NULL)
return NULL;
if (newsz != 0 && oldsz != 0) {
@@ -1510,6 +1536,7 @@ calloc(size_t nmemb, size_t size)
{
void *r;
int saved_errno = errno;
+ size_t realsize;
_MALLOC_LOCK();
malloc_func = "calloc():";
@@ -1532,9 +1559,10 @@ calloc(size_t nmemb, size_t size)
}
size *= nmemb;
+ realsize = size;
if (size > 0 && size <= MALLOC_MAXCHUNK)
size += mopts.malloc_canaries;
- r = omalloc(size, 1, CALLER);
+ r = omalloc(size, realsize, 1, CALLER);
malloc_active--;
_MALLOC_UNLOCK();
@@ -1587,7 +1615,7 @@ mapalign(struct dir_info *d, size_t alig
}
static void *
-omemalign(size_t alignment, size_t sz, int zero_fill, void *f)
+omemalign(size_t alignment, size_t sz, size_t realsz, int zero_fill, void *f)
{
struct dir_info *pool = getpool();
size_t psz;
@@ -1600,7 +1628,7 @@ omemalign(size_t alignment, size_t sz, i
*/
if (sz < alignment)
sz = alignment;
- return omalloc(sz, zero_fill, f);
+ return omalloc(sz, realsz, zero_fill, f);
}
if (sz >= SIZE_MAX - mopts.malloc_guard - MALLOC_PAGESIZE) {
@@ -1645,6 +1673,7 @@ int
posix_memalign(void **memptr, size_t alignment, size_t size)
{
int res, saved_errno = errno;
+ size_t realsize = size;
void *r;
/* Make sure that alignment is a large enough power of 2. */
@@ -1663,7 +1692,7 @@ posix_memalign(void **memptr, size_t ali
}
if (size > 0 && size <= MALLOC_MAXCHUNK)
size += mopts.malloc_canaries;
- r = omemalign(alignment, size, 0, CALLER);
+ r = omemalign(alignment, size, realsize, 0, CALLER);
malloc_active--;
_MALLOC_UNLOCK();
if (r == NULL) {
