I took another look at the way junk works in malloc, and there's a few
improvements I'd like to make.
1. Remove the Z option. In general, I think malloc options should make
programs crash more, not less. This option is a bandaid, but looks
like something that makes things better. (Anecdotally, I've seen
people turn it on because it somehow sounded even better than J.
Incorrect.)
2. Default to something halfway to J. When we free a chunk, always
overwrite it. When we free a larger allocation, just overwrite the
front of it. This has the desirable property of wiping small freed
secrets and data structures, but won't have the performance impact of
wiping large buffers (or faulting in pages that weren't even touched).
J turns it up to junking on malloc() as before, and j turns it off.
I have't been able to build perl reliably with this diff, but haven't
ruled out the 5.18 update either.
Index: malloc.3
===================================================================
RCS file: /cvs/src/lib/libc/stdlib/malloc.3,v
retrieving revision 1.73
diff -u -p -r1.73 malloc.3
--- malloc.3 18 Jul 2013 10:14:49 -0000 1.73
+++ malloc.3 13 Apr 2014 21:49:47 -0000
@@ -298,11 +298,6 @@ malloc_options = "X";
.Pp
Note that this will cause code that is supposed to handle
out-of-memory conditions gracefully to abort instead.
-.It Cm Z
-.Dq Zero .
-Fill some junk into the area allocated (see
-.Cm J ) ,
-except for the exact length the user asked for, which is zeroed.
.It Cm <
.Dq Half the cache size .
Decrease the size of the free page cache by a factor of two.
Index: malloc.c
===================================================================
RCS file: /cvs/src/lib/libc/stdlib/malloc.c,v
retrieving revision 1.152
diff -u -p -r1.152 malloc.c
--- malloc.c 3 Apr 2014 16:18:11 -0000 1.152
+++ malloc.c 13 Apr 2014 21:05:11 -0000
@@ -167,7 +167,6 @@ struct malloc_readonly {
int malloc_move; /* move allocations to end of page? */
int malloc_realloc; /* always realloc? */
int malloc_xmalloc; /* xmalloc behaviour? */
- int malloc_zero; /* zero fill? */
size_t malloc_guard; /* use guard pages after allocations? */
u_int malloc_cache; /* free pages we cache */
#ifdef MALLOC_STATS
@@ -412,7 +411,7 @@ map(struct dir_info *d, size_t sz, int z
d->free_regions_size -= psz;
if (zero_fill)
memset(p, 0, sz);
- else if (mopts.malloc_junk &&
+ else if (mopts.malloc_junk == 2 &&
mopts.malloc_freeunmap)
memset(p, SOME_FREEJUNK, sz);
return p;
@@ -431,7 +430,7 @@ map(struct dir_info *d, size_t sz, int z
d->free_regions_size -= psz;
if (zero_fill)
memset(p, 0, sz);
- else if (mopts.malloc_junk && mopts.malloc_freeunmap)
+ else if (mopts.malloc_junk == 2 && mopts.malloc_freeunmap)
memset(p, SOME_FREEJUNK, sz);
return p;
}
@@ -461,6 +460,7 @@ omalloc_init(struct dir_info **dp)
* Default options
*/
mopts.malloc_abort = 1;
+ mopts.malloc_junk = 1;
mopts.malloc_move = 1;
mopts.malloc_cache = MALLOC_DEFAULT_CACHE;
@@ -534,7 +534,7 @@ omalloc_init(struct dir_info **dp)
mopts.malloc_junk = 0;
break;
case 'J':
- mopts.malloc_junk = 1;
+ mopts.malloc_junk = 2;
break;
case 'n':
case 'N':
@@ -557,7 +557,8 @@ omalloc_init(struct dir_info **dp)
mopts.malloc_cache = MALLOC_DEFAULT_CACHE;
break;
case 'S':
- mopts.malloc_freeunmap = mopts.malloc_junk = 1;
+ mopts.malloc_freeunmap = 1;
+ mopts.malloc_junk = 2;
mopts.malloc_guard = MALLOC_PAGESIZE;
mopts.malloc_cache = 0;
break;
@@ -573,12 +574,6 @@ omalloc_init(struct dir_info **dp)
case 'X':
mopts.malloc_xmalloc = 1;
break;
- case 'z':
- mopts.malloc_zero = 0;
- break;
- case 'Z':
- mopts.malloc_zero = 1;
- break;
default: {
static const char q[] = "malloc() warning: "
"unknown char in MALLOC_OPTIONS\n";
@@ -589,13 +584,6 @@ omalloc_init(struct dir_info **dp)
}
}
- /*
- * We want junk in the entire allocation, and zero only in the part
- * the user asked for.
- */
- if (mopts.malloc_zero)
- mopts.malloc_junk = 1;
-
#ifdef MALLOC_STATS
if (mopts.malloc_stats && (atexit(malloc_exit) == -1)) {
static const char q[] = "malloc() warning: atexit(2) failed."
@@ -969,7 +957,7 @@ malloc_bytes(struct dir_info *d, size_t
k += (lp - bp->bits) * MALLOC_BITS;
k <<= bp->shift;
- if (mopts.malloc_junk && bp->size > 0)
+ if (mopts.malloc_junk == 2 && bp->size > 0)
memset((char *)bp->page + k, SOME_JUNK, bp->size);
return ((char *)bp->page + k);
}
@@ -1067,16 +1055,16 @@ omalloc(size_t sz, int zero_fill, void *
sz - mopts.malloc_guard < MALLOC_PAGESIZE -
MALLOC_LEEWAY) {
/* fill whole allocation */
- if (mopts.malloc_junk)
+ if (mopts.malloc_junk == 2)
memset(p, SOME_JUNK, psz - mopts.malloc_guard);
/* shift towards the end */
p = ((char *)p) + ((MALLOC_PAGESIZE - MALLOC_LEEWAY -
(sz - mopts.malloc_guard)) & ~(MALLOC_MINSIZE-1));
/* fill zeros if needed and overwritten above */
- if (zero_fill && mopts.malloc_junk)
+ if (zero_fill && mopts.malloc_junk == 2)
memset(p, 0, sz - mopts.malloc_guard);
} else {
- if (mopts.malloc_junk) {
+ if (mopts.malloc_junk == 2) {
if (zero_fill)
memset((char *)p + sz -
mopts.malloc_guard,
SOME_JUNK, psz - sz);
@@ -1144,7 +1132,7 @@ malloc(size_t size)
malloc_recurse();
return NULL;
}
- r = omalloc(size, mopts.malloc_zero, CALLER);
+ r = omalloc(size, 0, CALLER);
malloc_active--;
_MALLOC_UNLOCK();
if (r == NULL && mopts.malloc_xmalloc) {
@@ -1196,9 +1184,11 @@ ofree(void *p)
}
malloc_guarded -= mopts.malloc_guard;
}
- if (mopts.malloc_junk && !mopts.malloc_freeunmap)
- memset(p, SOME_FREEJUNK,
- PAGEROUND(sz) - mopts.malloc_guard);
+ if (mopts.malloc_junk && !mopts.malloc_freeunmap) {
+ size_t amt = mopts.malloc_junk == 1 ? MALLOC_MAXCHUNK :
+ PAGEROUND(sz) - mopts.malloc_guard;
+ memset(p, SOME_FREEJUNK, amt);
+ }
unmap(g_pool, p, PAGEROUND(sz));
delete(g_pool, r);
} else {
@@ -1302,7 +1292,7 @@ orealloc(void *p, size_t newsz, void *f)
q = MAP_FAILED;
if (q == hint) {
malloc_used += needed;
- if (mopts.malloc_junk)
+ if (mopts.malloc_junk == 2)
memset(q, SOME_JUNK, needed);
r->size = newsz;
STATS_SETF(r, f);
@@ -1329,7 +1319,7 @@ orealloc(void *p, size_t newsz, void *f)
STATS_SETF(r, f);
return p;
} else {
- if (newsz > oldsz && mopts.malloc_junk)
+ if (newsz > oldsz && mopts.malloc_junk == 2)
memset((char *)p + newsz, SOME_JUNK,
rnewsz - mopts.malloc_guard - newsz);
r->size = gnewsz;
@@ -1338,7 +1328,7 @@ orealloc(void *p, size_t newsz, void *f)
}
}
if (newsz <= oldsz && newsz > oldsz / 2 && !mopts.malloc_realloc) {
- if (mopts.malloc_junk && newsz > 0)
+ if (mopts.malloc_junk == 2 && newsz > 0)
memset((char *)p + newsz, SOME_JUNK, oldsz - newsz);
STATS_SETF(r, f);
return p;
@@ -1509,7 +1499,7 @@ omemalign(size_t alignment, size_t sz, i
malloc_guarded += mopts.malloc_guard;
}
- if (mopts.malloc_junk) {
+ if (mopts.malloc_junk == 2) {
if (zero_fill)
memset((char *)p + sz - mopts.malloc_guard,
SOME_JUNK, psz - sz);
@@ -1540,7 +1530,7 @@ posix_memalign(void **memptr, size_t ali
malloc_recurse();
goto err;
}
- r = omemalign(alignment, size, mopts.malloc_zero, CALLER);
+ r = omemalign(alignment, size, 0, CALLER);
malloc_active--;
_MALLOC_UNLOCK();
if (r == NULL) {
