On 23/10/15 07:22 AM, Janne Johansson wrote:
> With this patch on -current and
> ln -sf CJ /etc/malloc.conf
> a lot of things stopped working, like "man ls" and "tmux".
> With only C or only J, the system seems to work ok, but with both it
> doesn't.
> Will check the resulting core files.
> ntpd and tmux both bombed out on memset inside realloc if I read the
> dumps ok:
Thanks for testing it. I oversimplified handling the canary padding in
the orealloc malloc_junk == 2 branch while trying to clean up the code.
This should work properly:
diff --git a/stdlib/malloc.c b/stdlib/malloc.c
index 424dd77..567e56d 100644
--- a/stdlib/malloc.c
+++ b/stdlib/malloc.c
@@ -185,12 +185,14 @@ struct malloc_readonly {
int malloc_move; /* move allocations to end of page? */
int malloc_realloc; /* always realloc? */
int malloc_xmalloc; /* xmalloc behaviour? */
+ size_t malloc_canaries; /* use canaries after chunks? */
size_t malloc_guard; /* use guard pages after allocations? */
u_int malloc_cache; /* free pages we cache */
#ifdef MALLOC_STATS
int malloc_stats; /* dump statistics at end */
#endif
u_int32_t malloc_canary; /* Matched against ones in malloc_pool
*/
+ uintptr_t malloc_chunk_canary;
};
/* This object is mapped PROT_READ after initialisation to prevent tampering */
@@ -526,6 +528,12 @@ omalloc_init(struct dir_info **dp)
case 'A':
mopts.malloc_abort = 1;
break;
+ case 'c':
+ mopts.malloc_canaries = 0;
+ break;
+ case 'C':
+ mopts.malloc_canaries = sizeof(void *);
+ break;
#ifdef MALLOC_STATS
case 'd':
mopts.malloc_stats = 0;
@@ -619,6 +627,9 @@ omalloc_init(struct dir_info **dp)
while ((mopts.malloc_canary = arc4random()) == 0)
;
+ arc4random_buf(&mopts.malloc_chunk_canary,
+ sizeof(mopts.malloc_chunk_canary));
+
/*
* Allocate dir_info with a guard page on either side. Also
* randomise offset inside the page at which the dir_info
@@ -984,8 +995,15 @@ malloc_bytes(struct dir_info *d, size_t size, void *f)
k += (lp - bp->bits) * MALLOC_BITS;
k <<= bp->shift;
+ 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);
+ }
+
if (mopts.malloc_junk == 2 && bp->size > 0)
- memset((char *)bp->page + k, SOME_JUNK, bp->size);
+ memset((char *)bp->page + k, SOME_JUNK,
+ bp->size - mopts.malloc_canaries);
return ((char *)bp->page + k);
}
@@ -999,6 +1017,13 @@ find_chunknum(struct dir_info *d, struct region_info *r,
void *ptr)
if (info->canary != d->canary1)
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)))
+ wrterror("chunk canary corrupted", ptr);
+ }
+
/* Find the chunk number on the page */
chunknum = ((uintptr_t)ptr & MALLOC_PAGEMASK) >> info->shift;
@@ -1121,7 +1146,7 @@ omalloc(size_t sz, int zero_fill, void *f)
/* takes care of SOME_JUNK */
p = malloc_bytes(pool, sz, f);
if (zero_fill && p != NULL && sz > 0)
- memset(p, 0, sz);
+ memset(p, 0, sz - mopts.malloc_canaries);
}
return p;
@@ -1176,6 +1201,8 @@ malloc(size_t size)
malloc_recurse();
return NULL;
}
+ if (size > 0 && size <= MALLOC_MAXCHUNK)
+ size += mopts.malloc_canaries;
r = omalloc(size, 0, CALLER);
malloc_active--;
_MALLOC_UNLOCK();
@@ -1242,7 +1269,7 @@ ofree(void *p)
int i;
if (mopts.malloc_junk && sz > 0)
- memset(p, SOME_FREEJUNK, sz);
+ memset(p, SOME_FREEJUNK, sz - mopts.malloc_canaries);
if (!mopts.malloc_freenow) {
if (find_chunknum(pool, r, p) == -1)
return;
@@ -1386,16 +1413,25 @@ gotit:
}
}
if (newsz <= oldsz && newsz > oldsz / 2 && !mopts.malloc_realloc) {
- if (mopts.malloc_junk == 2 && newsz > 0)
- memset((char *)p + newsz, SOME_JUNK, oldsz - newsz);
+ if (mopts.malloc_junk == 2 && newsz > 0) {
+ size_t usable_oldsz = oldsz;
+ if (oldsz <= MALLOC_MAXCHUNK)
+ usable_oldsz -= mopts.malloc_canaries;
+ if (newsz < usable_oldsz)
+ memset((char *)p + newsz, SOME_JUNK,
usable_oldsz - newsz);
+ }
STATS_SETF(r, f);
return p;
} else if (newsz != oldsz || mopts.malloc_realloc) {
q = omalloc(newsz, 0, f);
if (q == NULL)
return NULL;
- if (newsz != 0 && oldsz != 0)
- memcpy(q, p, oldsz < newsz ? oldsz : newsz);
+ if (newsz != 0 && oldsz != 0) {
+ size_t copysz = oldsz < newsz ? oldsz : newsz;
+ if (copysz <= MALLOC_MAXCHUNK)
+ copysz -= mopts.malloc_canaries;
+ memcpy(q, p, copysz);
+ }
ofree(p);
return q;
} else {
@@ -1420,6 +1456,8 @@ realloc(void *ptr, size_t size)
malloc_recurse();
return NULL;
}
+ if (size > 0 && size <= MALLOC_MAXCHUNK)
+ size += mopts.malloc_canaries;
r = orealloc(ptr, size, CALLER);
malloc_active--;
@@ -1468,6 +1506,8 @@ calloc(size_t nmemb, size_t size)
}
size *= nmemb;
+ if (size > 0 && size <= MALLOC_MAXCHUNK)
+ size += mopts.malloc_canaries;
r = omalloc(size, 1, CALLER);
malloc_active--;
@@ -1595,6 +1635,8 @@ posix_memalign(void **memptr, size_t alignment, size_t
size)
malloc_recurse();
goto err;
}
+ if (size > 0 && size <= MALLOC_MAXCHUNK)
+ size += mopts.malloc_canaries;
r = omemalign(alignment, size, 0, CALLER);
malloc_active--;
_MALLOC_UNLOCK();
