From 92d2cf9d5b2139bbf8b2a57bb0666ace4396645b Mon Sep 17 00:00:00 2001 From: otto <> Date: Mon, 10 Apr 2017 05:45:02 +0000 Subject: [PATCH] Introducing freezero(3) a version of free that guarantees the process no longer has access to the content of a memmory object. It does this by either clearing (if the object memory remains cached) or by calling munmap(2). ok millert@, deraadt@, guenther@ --- src/include/stdlib.h | 3 +- src/lib/libc/stdlib/malloc.3 | 82 ++++++++++++++++++++++++----------- src/lib/libc/stdlib/malloc.c | 84 ++++++++++++++++++++++++++++++------ 3 files changed, 132 insertions(+), 37 deletions(-) diff --git a/src/include/stdlib.h b/src/include/stdlib.h index ccbccfc5..ab054216 100644 --- a/src/include/stdlib.h +++ b/src/include/stdlib.h @@ -1,4 +1,4 @@ -/* $OpenBSD: stdlib.h,v 1.68 2017/03/06 18:50:28 otto Exp $ */ +/* $OpenBSD: stdlib.h,v 1.69 2017/04/10 05:45:02 otto Exp $ */ /* $NetBSD: stdlib.h,v 1.25 1995/12/27 21:19:08 jtc Exp $ */ /*- @@ -113,6 +113,7 @@ long labs(long); ldiv_t ldiv(long, long); void *malloc(size_t); #if __BSD_VISIBLE +void freezero(void *, size_t); void *reallocarray(void *, size_t, size_t); void *recallocarray(void *, size_t, size_t, size_t); #endif /* __BSD_VISIBLE */ diff --git a/src/lib/libc/stdlib/malloc.3 b/src/lib/libc/stdlib/malloc.3 index c65c08ef..c7a79b5e 100644 --- a/src/lib/libc/stdlib/malloc.3 +++ b/src/lib/libc/stdlib/malloc.3 @@ -30,18 +30,19 @@ .\" OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF .\" SUCH DAMAGE. .\" -.\" $OpenBSD: malloc.3,v 1.109 2017/04/06 17:00:52 otto Exp $ +.\" $OpenBSD: malloc.3,v 1.110 2017/04/10 05:45:02 otto Exp $ .\" -.Dd $Mdocdate: April 6 2017 $ +.Dd $Mdocdate: April 10 2017 $ .Dt MALLOC 3 .Os .Sh NAME .Nm malloc , .Nm calloc , -.Nm reallocarray , -.Nm recallocarray , .Nm realloc , .Nm free +.Nm reallocarray , +.Nm recallocarray , +.Nm freezero , .Nd memory allocation and deallocation .Sh SYNOPSIS .In stdlib.h @@ -50,15 +51,23 @@ .Ft void * .Fn calloc "size_t nmemb" "size_t size" .Ft void * +.Fn realloc "void *ptr" "size_t size" +.Ft void +.Fn free "void *ptr" +.Ft void * .Fn reallocarray "void *ptr" "size_t nmemb" "size_t size" .Ft void * .Fn recallocarray "void *ptr" "size_t oldnmemb" "size_t nmemb" "size_t size" -.Ft void * -.Fn realloc "void *ptr" "size_t size" .Ft void -.Fn free "void *ptr" +.Fn freezero "void *ptr" "size_t size" .Vt char *malloc_options ; .Sh DESCRIPTION +The standard functions +.Fn malloc , +.Fn calloc , +and +.Fn realloc +allocate memory space. The .Fn malloc function allocates uninitialized space for an object of @@ -103,6 +112,26 @@ behaves like and allocates a new object. .Pp The +.Fn free +function causes the space pointed to by +.Fa ptr +to be either placed on a list of free blocks to make it available for future +allocation or, when appropiate, to be returned to the kernel using +.Xr munmap 2 . +If +.Fa ptr +is a +.Dv NULL +pointer, no action occurs. +If +.Fa ptr +was previously freed by +.Fn free +or a reallocation function, +the behavior is undefined and the double free is a security concern. +.Pp +Designed for safe allocation of arrays, +the .Fn reallocarray function is similar to .Fn realloc @@ -115,7 +144,8 @@ and checks for integer overflow in the calculation * .Fa size . .Pp -The +Used for the allocation of memory holding sensitive data, +the .Fn recallocarray function is similar to .Fn reallocarray @@ -150,23 +180,25 @@ is the size of the earlier allocation that returned otherwise the behaviour is undefined. .Pp The +.Fn freezero +function is similar to the .Fn free -function causes the space pointed to by -.Fa ptr -to be either placed on a list of free pages to make it available for future -allocation or, if required, to be returned to the kernel using -.Xr munmap 2 . +function except it ensures the memory being deallocated is explicitly +discarded. If .Fa ptr -is a -.Dv NULL -pointer, no action occurs. +is +.Dv NULL , +no action occurs. If .Fa ptr -was previously freed by -.Fn free -or a reallocation function, -the behavior is undefined and the double free is a security concern. +is not +.Dv NULL , +the +.Fa size +argument must be the size of the earlier allocation that returned +.Fa ptr , +otherwise the behaviour is undefined. .Sh RETURN VALUES Upon successful completion, the allocation functions return a pointer to the allocated space; otherwise, a @@ -319,10 +351,8 @@ function should be used for resizing objects containing sensitive data like keys. To avoid leaking information, it guarantees memory is cleared before placing it on the internal free list. -A -.Fn free -call for such an object should still be preceded by a call to -.Xr explicit_bzero 3 . +Deallocation of such an object should be done by calling +.Fn freezero . .Sh ENVIRONMENT .Bl -tag -width "/etc/malloc.conf" .It Ev MALLOC_OPTIONS @@ -539,6 +569,10 @@ The .Fn recallocarray function appeared in .Ox 6.1 . +The +.Fn freezero +function appeared in +.Ox 6.2 . .Sh CAVEATS When using .Fn malloc , diff --git a/src/lib/libc/stdlib/malloc.c b/src/lib/libc/stdlib/malloc.c index f2b8b154..07c73ca7 100644 --- a/src/lib/libc/stdlib/malloc.c +++ b/src/lib/libc/stdlib/malloc.c @@ -1,4 +1,4 @@ -/* $OpenBSD: malloc.c,v 1.219 2017/04/06 08:39:47 otto Exp $ */ +/* $OpenBSD: malloc.c,v 1.220 2017/04/10 05:45:02 otto Exp $ */ /* * Copyright (c) 2008, 2010, 2011, 2016 Otto Moerbeek * Copyright (c) 2012 Matthew Dempsky @@ -185,7 +185,7 @@ struct malloc_readonly { int malloc_realloc; /* always realloc? */ int malloc_xmalloc; /* xmalloc behaviour? */ int chunk_canaries; /* use canaries after chunks? */ - int internal_recallocarray; /* use better recallocarray? */ + int internal_funcs; /* use better recallocarray/freezero? */ u_int malloc_cache; /* free pages we cache */ size_t malloc_guard; /* use guard pages after allocations? */ #ifdef MALLOC_STATS @@ -343,7 +343,14 @@ unmap(struct dir_info *d, void *p, size_t sz, int clear) if (sz != PAGEROUND(sz)) wrterror(d, "munmap round"); - if (psz > mopts.malloc_cache) { + rsz = mopts.malloc_cache - d->free_regions_size; + + /* + * normally the cache holds recently freed regions, but if the region + * to unmap is larger than the cache size or we're clearing and the + * cache is full, just munmap + */ + if (psz > mopts.malloc_cache || (clear && rsz == 0)) { i = munmap(p, sz); if (i) wrterror(d, "munmap %p", p); @@ -351,7 +358,6 @@ unmap(struct dir_info *d, void *p, size_t sz, int clear) return; } tounmap = 0; - rsz = mopts.malloc_cache - d->free_regions_size; if (psz > rsz) tounmap = psz - rsz; offset = getrbyte(d); @@ -1234,7 +1240,7 @@ _malloc_init(int from_rthreads) if (from_rthreads) mopts.malloc_mt = 1; else - mopts.internal_recallocarray = 1; + mopts.internal_funcs = 1; /* * Options have been set and will never be reset. @@ -1297,7 +1303,7 @@ validate_junk(struct dir_info *pool, void *p) } static void -ofree(struct dir_info *argpool, void *p, int clear) +ofree(struct dir_info *argpool, void *p, int clear, int check, size_t argsz) { struct dir_info *pool; struct region_info *r; @@ -1326,6 +1332,25 @@ ofree(struct dir_info *argpool, void *p, int clear) } REALSIZE(sz, r); + if (check) { + if (sz <= MALLOC_MAXCHUNK) { + if (mopts.chunk_canaries) { + struct chunk_info *info = + (struct chunk_info *)r->size; + uint32_t chunknum = + find_chunknum(pool, r, p, 0); + + if (info->bits[info->offset + chunknum] != + argsz) + wrterror(pool, "recorded old size %hu" + " != %zu", + info->bits[info->offset + chunknum], + argsz); + } + } else if (argsz != sz - mopts.malloc_guard) + wrterror(pool, "recorded old size %zu != %zu", + sz - mopts.malloc_guard, argsz); + } if (sz > MALLOC_MAXCHUNK) { if (!MALLOC_MOVE_COND(sz)) { if (r->p != p) @@ -1411,13 +1436,48 @@ free(void *ptr) malloc_recurse(d); return; } - ofree(d, ptr, 0); + ofree(d, ptr, 0, 0, 0); d->active--; _MALLOC_UNLOCK(d->mutex); errno = saved_errno; } /*DEF_STRONG(free);*/ +static void +freezero_p(void *ptr, size_t sz) +{ + explicit_bzero(ptr, sz); + free(ptr); +} + +void +freezero(void *ptr, size_t sz) +{ + struct dir_info *d; + int saved_errno = errno; + + /* This is legal. */ + if (ptr == NULL) + return; + + if (!mopts.internal_funcs) + return freezero_p(ptr, sz); + + d = getpool(); + if (d == NULL) + wrterror(d, "freezero() called before allocation"); + _MALLOC_LOCK(d->mutex); + d->func = "freezero"; + if (d->active++) { + malloc_recurse(d); + return; + } + ofree(d, ptr, 1, 1, sz); + d->active--; + _MALLOC_UNLOCK(d->mutex); + errno = saved_errno; +} +DEF_WEAK(freezero); static void * orealloc(struct dir_info *argpool, void *p, size_t newsz, void *f) @@ -1591,7 +1651,7 @@ gotit: } if (newsz != 0 && oldsz != 0) memcpy(q, p, oldsz < newsz ? oldsz : newsz); - ofree(pool, p, 0); + ofree(pool, p, 0, 0, 0); ret = q; } else { /* oldsz == newsz */ @@ -1751,7 +1811,7 @@ orecallocarray(struct dir_info *argpool, void *p, size_t oldsize, } else memcpy(newptr, p, newsize); - ofree(pool, p, 1); + ofree(pool, p, 1, 0, 0); done: if (argpool != pool) { @@ -1824,7 +1884,7 @@ recallocarray(void *ptr, size_t oldnmemb, size_t newnmemb, size_t size) void *r; int saved_errno = errno; - if (!mopts.internal_recallocarray) + if (!mopts.internal_funcs) return recallocarray_p(ptr, oldnmemb, newnmemb, size); d = getpool(); @@ -2275,8 +2335,8 @@ malloc_exit(void) __progname); write(fd, buf, strlen(buf)); snprintf(buf, sizeof(buf), - "MT=%d IRC=%d F=%d U=%d J=%d R=%d X=%d C=%d cache=%u G=%zu\n", - mopts.malloc_mt, mopts.internal_recallocarray, + "MT=%d I=%d F=%d U=%d J=%d R=%d X=%d C=%d cache=%u G=%zu\n", + mopts.malloc_mt, mopts.internal_funcs, mopts.malloc_freenow, mopts.malloc_freeunmap, mopts.malloc_junk, mopts.malloc_realloc, mopts.malloc_xmalloc,