Fix potential TLS-related memory corruption.

Avoid writing to uninitialized TLS as a side effect of deallocation.
Initializing TLS during deallocation is unsafe because it is possible
that a thread never did any allocation, and that TLS has already been
deallocated by the threads library, resulting in write-after-free
corruption.  These fixes affect prof_tdata and quarantine; all other
uses of TLS are already safe, whether intentionally (as for tcache) or
unintentionally (as for arenas).
This commit is contained in:
Jason Evans 2013-01-30 15:03:11 -08:00
parent 83789f4530
commit bbe29d374d
7 changed files with 105 additions and 65 deletions

View File

@ -6,6 +6,13 @@ found in the git revision history:
http://www.canonware.com/cgi-bin/gitweb.cgi?p=jemalloc.git http://www.canonware.com/cgi-bin/gitweb.cgi?p=jemalloc.git
git://canonware.com/jemalloc.git git://canonware.com/jemalloc.git
* 3.x.x (XXX Not yet released)
Bug fixes:
- Fix TLS-related memory corruption that could occur during thread exit if the
thread never allocated memory. Only the quarantine and prof facilities were
susceptible.
* 3.3.0 (January 23, 2013) * 3.3.0 (January 23, 2013)
This version includes a few minor performance improvements in addition to the This version includes a few minor performance improvements in addition to the

View File

@ -306,7 +306,13 @@
#define prof_tdata_tsd_get_wrapper JEMALLOC_N(prof_tdata_tsd_get_wrapper) #define prof_tdata_tsd_get_wrapper JEMALLOC_N(prof_tdata_tsd_get_wrapper)
#define prof_tdata_tsd_set JEMALLOC_N(prof_tdata_tsd_set) #define prof_tdata_tsd_set JEMALLOC_N(prof_tdata_tsd_set)
#define quarantine JEMALLOC_N(quarantine) #define quarantine JEMALLOC_N(quarantine)
#define quarantine_alloc_hook JEMALLOC_N(quarantine_alloc_hook)
#define quarantine_boot JEMALLOC_N(quarantine_boot) #define quarantine_boot JEMALLOC_N(quarantine_boot)
#define quarantine_booted JEMALLOC_N(quarantine_booted)
#define quarantine_cleanup JEMALLOC_N(quarantine_cleanup)
#define quarantine_init JEMALLOC_N(quarantine_init)
#define quarantine_tls JEMALLOC_N(quarantine_tls)
#define quarantine_tsd JEMALLOC_N(quarantine_tsd)
#define quarantine_tsd_boot JEMALLOC_N(quarantine_tsd_boot) #define quarantine_tsd_boot JEMALLOC_N(quarantine_tsd_boot)
#define quarantine_tsd_cleanup_wrapper JEMALLOC_N(quarantine_tsd_cleanup_wrapper) #define quarantine_tsd_cleanup_wrapper JEMALLOC_N(quarantine_tsd_cleanup_wrapper)
#define quarantine_tsd_get JEMALLOC_N(quarantine_tsd_get) #define quarantine_tsd_get JEMALLOC_N(quarantine_tsd_get)

View File

@ -237,7 +237,7 @@ void prof_postfork_child(void);
\ \
assert(size == s2u(size)); \ assert(size == s2u(size)); \
\ \
prof_tdata = prof_tdata_get(); \ prof_tdata = prof_tdata_get(true); \
if ((uintptr_t)prof_tdata <= (uintptr_t)PROF_TDATA_STATE_MAX) { \ if ((uintptr_t)prof_tdata <= (uintptr_t)PROF_TDATA_STATE_MAX) { \
if (prof_tdata != NULL) \ if (prof_tdata != NULL) \
ret = (prof_thr_cnt_t *)(uintptr_t)1U; \ ret = (prof_thr_cnt_t *)(uintptr_t)1U; \
@ -286,7 +286,7 @@ void prof_postfork_child(void);
#ifndef JEMALLOC_ENABLE_INLINE #ifndef JEMALLOC_ENABLE_INLINE
malloc_tsd_protos(JEMALLOC_ATTR(unused), prof_tdata, prof_tdata_t *) malloc_tsd_protos(JEMALLOC_ATTR(unused), prof_tdata, prof_tdata_t *)
prof_tdata_t *prof_tdata_get(void); prof_tdata_t *prof_tdata_get(bool create);
void prof_sample_threshold_update(prof_tdata_t *prof_tdata); void prof_sample_threshold_update(prof_tdata_t *prof_tdata);
prof_ctx_t *prof_ctx_get(const void *ptr); prof_ctx_t *prof_ctx_get(const void *ptr);
void prof_ctx_set(const void *ptr, prof_ctx_t *ctx); void prof_ctx_set(const void *ptr, prof_ctx_t *ctx);
@ -304,17 +304,15 @@ malloc_tsd_funcs(JEMALLOC_INLINE, prof_tdata, prof_tdata_t *, NULL,
prof_tdata_cleanup) prof_tdata_cleanup)
JEMALLOC_INLINE prof_tdata_t * JEMALLOC_INLINE prof_tdata_t *
prof_tdata_get(void) prof_tdata_get(bool create)
{ {
prof_tdata_t *prof_tdata; prof_tdata_t *prof_tdata;
cassert(config_prof); cassert(config_prof);
prof_tdata = *prof_tdata_tsd_get(); prof_tdata = *prof_tdata_tsd_get();
if ((uintptr_t)prof_tdata <= (uintptr_t)PROF_TDATA_STATE_MAX) { if (create && prof_tdata == NULL)
if (prof_tdata == NULL)
prof_tdata = prof_tdata_init(); prof_tdata = prof_tdata_init();
}
return (prof_tdata); return (prof_tdata);
} }
@ -397,7 +395,7 @@ prof_sample_accum_update(size_t size)
/* Sampling logic is unnecessary if the interval is 1. */ /* Sampling logic is unnecessary if the interval is 1. */
assert(opt_lg_prof_sample != 0); assert(opt_lg_prof_sample != 0);
prof_tdata = *prof_tdata_tsd_get(); prof_tdata = prof_tdata_get(false);
if ((uintptr_t)prof_tdata <= (uintptr_t)PROF_TDATA_STATE_MAX) if ((uintptr_t)prof_tdata <= (uintptr_t)PROF_TDATA_STATE_MAX)
return (true); return (true);

View File

@ -1,6 +1,9 @@
/******************************************************************************/ /******************************************************************************/
#ifdef JEMALLOC_H_TYPES #ifdef JEMALLOC_H_TYPES
typedef struct quarantine_obj_s quarantine_obj_t;
typedef struct quarantine_s quarantine_t;
/* Default per thread quarantine size if valgrind is enabled. */ /* Default per thread quarantine size if valgrind is enabled. */
#define JEMALLOC_VALGRIND_QUARANTINE_DEFAULT (ZU(1) << 24) #define JEMALLOC_VALGRIND_QUARANTINE_DEFAULT (ZU(1) << 24)
@ -8,17 +11,57 @@
/******************************************************************************/ /******************************************************************************/
#ifdef JEMALLOC_H_STRUCTS #ifdef JEMALLOC_H_STRUCTS
struct quarantine_obj_s {
void *ptr;
size_t usize;
};
struct quarantine_s {
size_t curbytes;
size_t curobjs;
size_t first;
#define LG_MAXOBJS_INIT 10
size_t lg_maxobjs;
quarantine_obj_t objs[1]; /* Dynamically sized ring buffer. */
};
#endif /* JEMALLOC_H_STRUCTS */ #endif /* JEMALLOC_H_STRUCTS */
/******************************************************************************/ /******************************************************************************/
#ifdef JEMALLOC_H_EXTERNS #ifdef JEMALLOC_H_EXTERNS
quarantine_t *quarantine_init(size_t lg_maxobjs);
void quarantine(void *ptr); void quarantine(void *ptr);
void quarantine_cleanup(void *arg);
bool quarantine_boot(void); bool quarantine_boot(void);
#endif /* JEMALLOC_H_EXTERNS */ #endif /* JEMALLOC_H_EXTERNS */
/******************************************************************************/ /******************************************************************************/
#ifdef JEMALLOC_H_INLINES #ifdef JEMALLOC_H_INLINES
#ifndef JEMALLOC_ENABLE_INLINE
malloc_tsd_protos(JEMALLOC_ATTR(unused), quarantine, quarantine_t *)
void quarantine_alloc_hook(void);
#endif
#if (defined(JEMALLOC_ENABLE_INLINE) || defined(JEMALLOC_QUARANTINE_C_))
malloc_tsd_externs(quarantine, quarantine_t *)
malloc_tsd_funcs(JEMALLOC_ALWAYS_INLINE, quarantine, quarantine_t *, NULL,
quarantine_cleanup)
JEMALLOC_ALWAYS_INLINE void
quarantine_alloc_hook(void)
{
quarantine_t *quarantine;
assert(config_fill && opt_quarantine);
quarantine = *quarantine_tsd_get();
if (quarantine == NULL)
quarantine_init(LG_MAXOBJS_INIT);
}
#endif
#endif /* JEMALLOC_H_INLINES */ #endif /* JEMALLOC_H_INLINES */
/******************************************************************************/ /******************************************************************************/

View File

@ -282,12 +282,30 @@ arenas_cleanup(void *arg)
malloc_mutex_unlock(&arenas_lock); malloc_mutex_unlock(&arenas_lock);
} }
static JEMALLOC_ATTR(always_inline) void
malloc_thread_init(void)
{
/*
* TSD initialization can't be safely done as a side effect of
* deallocation, because it is possible for a thread to do nothing but
* deallocate its TLS data via free(), in which case writing to TLS
* would cause write-after-free memory corruption. The quarantine
* facility *only* gets used as a side effect of deallocation, so make
* a best effort attempt at initializing its TSD by hooking all
* allocation events.
*/
if (config_fill && opt_quarantine)
quarantine_alloc_hook();
}
static JEMALLOC_ATTR(always_inline) bool static JEMALLOC_ATTR(always_inline) bool
malloc_init(void) malloc_init(void)
{ {
if (malloc_initialized == false) if (malloc_initialized == false && malloc_init_hard())
return (malloc_init_hard()); return (true);
malloc_thread_init();
return (false); return (false);
} }
@ -1095,6 +1113,7 @@ je_realloc(void *ptr, size_t size)
if (size == 0) { if (size == 0) {
if (ptr != NULL) { if (ptr != NULL) {
/* realloc(ptr, 0) is equivalent to free(p). */ /* realloc(ptr, 0) is equivalent to free(p). */
assert(malloc_initialized || IS_INITIALIZER);
if (config_prof) { if (config_prof) {
old_size = isalloc(ptr, true); old_size = isalloc(ptr, true);
if (config_valgrind && opt_valgrind) if (config_valgrind && opt_valgrind)
@ -1120,6 +1139,7 @@ je_realloc(void *ptr, size_t size)
if (ptr != NULL) { if (ptr != NULL) {
assert(malloc_initialized || IS_INITIALIZER); assert(malloc_initialized || IS_INITIALIZER);
malloc_thread_init();
if (config_prof) { if (config_prof) {
old_size = isalloc(ptr, true); old_size = isalloc(ptr, true);
@ -1323,6 +1343,7 @@ je_malloc_usable_size(JEMALLOC_USABLE_SIZE_CONST void *ptr)
size_t ret; size_t ret;
assert(malloc_initialized || IS_INITIALIZER); assert(malloc_initialized || IS_INITIALIZER);
malloc_thread_init();
if (config_ivsalloc) if (config_ivsalloc)
ret = ivsalloc(ptr, config_prof); ret = ivsalloc(ptr, config_prof);
@ -1497,6 +1518,7 @@ je_rallocm(void **ptr, size_t *rsize, size_t size, size_t extra, int flags)
assert(size != 0); assert(size != 0);
assert(SIZE_T_MAX - size >= extra); assert(SIZE_T_MAX - size >= extra);
assert(malloc_initialized || IS_INITIALIZER); assert(malloc_initialized || IS_INITIALIZER);
malloc_thread_init();
if (arena_ind != UINT_MAX) { if (arena_ind != UINT_MAX) {
arena_chunk_t *chunk; arena_chunk_t *chunk;
@ -1611,6 +1633,7 @@ je_sallocm(const void *ptr, size_t *rsize, int flags)
size_t sz; size_t sz;
assert(malloc_initialized || IS_INITIALIZER); assert(malloc_initialized || IS_INITIALIZER);
malloc_thread_init();
if (config_ivsalloc) if (config_ivsalloc)
sz = ivsalloc(ptr, config_prof); sz = ivsalloc(ptr, config_prof);

View File

@ -438,7 +438,7 @@ prof_lookup(prof_bt_t *bt)
cassert(config_prof); cassert(config_prof);
prof_tdata = prof_tdata_get(); prof_tdata = prof_tdata_get(false);
if ((uintptr_t)prof_tdata <= (uintptr_t)PROF_TDATA_STATE_MAX) if ((uintptr_t)prof_tdata <= (uintptr_t)PROF_TDATA_STATE_MAX)
return (NULL); return (NULL);
@ -684,7 +684,7 @@ prof_ctx_destroy(prof_ctx_t *ctx)
* avoid a race between the main body of prof_ctx_merge() and entry * avoid a race between the main body of prof_ctx_merge() and entry
* into this function. * into this function.
*/ */
prof_tdata = *prof_tdata_tsd_get(); prof_tdata = prof_tdata_get(false);
assert((uintptr_t)prof_tdata > (uintptr_t)PROF_TDATA_STATE_MAX); assert((uintptr_t)prof_tdata > (uintptr_t)PROF_TDATA_STATE_MAX);
prof_enter(prof_tdata); prof_enter(prof_tdata);
malloc_mutex_lock(ctx->lock); malloc_mutex_lock(ctx->lock);
@ -844,7 +844,7 @@ prof_dump(bool propagate_err, const char *filename, bool leakcheck)
cassert(config_prof); cassert(config_prof);
prof_tdata = prof_tdata_get(); prof_tdata = prof_tdata_get(false);
if ((uintptr_t)prof_tdata <= (uintptr_t)PROF_TDATA_STATE_MAX) if ((uintptr_t)prof_tdata <= (uintptr_t)PROF_TDATA_STATE_MAX)
return (true); return (true);
prof_enter(prof_tdata); prof_enter(prof_tdata);
@ -966,11 +966,7 @@ prof_idump(void)
if (prof_booted == false) if (prof_booted == false)
return; return;
/* prof_tdata = prof_tdata_get(false);
* Don't call prof_tdata_get() here, because it could cause recursive
* allocation.
*/
prof_tdata = *prof_tdata_tsd_get();
if ((uintptr_t)prof_tdata <= (uintptr_t)PROF_TDATA_STATE_MAX) if ((uintptr_t)prof_tdata <= (uintptr_t)PROF_TDATA_STATE_MAX)
return; return;
if (prof_tdata->enq) { if (prof_tdata->enq) {
@ -1020,11 +1016,7 @@ prof_gdump(void)
if (prof_booted == false) if (prof_booted == false)
return; return;
/* prof_tdata = prof_tdata_get(false);
* Don't call prof_tdata_get() here, because it could cause recursive
* allocation.
*/
prof_tdata = *prof_tdata_tsd_get();
if ((uintptr_t)prof_tdata <= (uintptr_t)PROF_TDATA_STATE_MAX) if ((uintptr_t)prof_tdata <= (uintptr_t)PROF_TDATA_STATE_MAX)
return; return;
if (prof_tdata->enq) { if (prof_tdata->enq) {

View File

@ -1,3 +1,4 @@
#define JEMALLOC_QUARANTINE_C_
#include "jemalloc/internal/jemalloc_internal.h" #include "jemalloc/internal/jemalloc_internal.h"
/* /*
@ -11,39 +12,17 @@
/******************************************************************************/ /******************************************************************************/
/* Data. */ /* Data. */
typedef struct quarantine_obj_s quarantine_obj_t; malloc_tsd_data(, quarantine, quarantine_t *, NULL)
typedef struct quarantine_s quarantine_t;
struct quarantine_obj_s {
void *ptr;
size_t usize;
};
struct quarantine_s {
size_t curbytes;
size_t curobjs;
size_t first;
#define LG_MAXOBJS_INIT 10
size_t lg_maxobjs;
quarantine_obj_t objs[1]; /* Dynamically sized ring buffer. */
};
static void quarantine_cleanup(void *arg);
malloc_tsd_data(static, quarantine, quarantine_t *, NULL)
malloc_tsd_funcs(JEMALLOC_INLINE, quarantine, quarantine_t *, NULL,
quarantine_cleanup)
/******************************************************************************/ /******************************************************************************/
/* Function prototypes for non-inline static functions. */ /* Function prototypes for non-inline static functions. */
static quarantine_t *quarantine_init(size_t lg_maxobjs);
static quarantine_t *quarantine_grow(quarantine_t *quarantine); static quarantine_t *quarantine_grow(quarantine_t *quarantine);
static void quarantine_drain(quarantine_t *quarantine, size_t upper_bound); static void quarantine_drain(quarantine_t *quarantine, size_t upper_bound);
/******************************************************************************/ /******************************************************************************/
static quarantine_t * quarantine_t *
quarantine_init(size_t lg_maxobjs) quarantine_init(size_t lg_maxobjs)
{ {
quarantine_t *quarantine; quarantine_t *quarantine;
@ -119,17 +98,10 @@ quarantine(void *ptr)
quarantine = *quarantine_tsd_get(); quarantine = *quarantine_tsd_get();
if ((uintptr_t)quarantine <= (uintptr_t)QUARANTINE_STATE_MAX) { if ((uintptr_t)quarantine <= (uintptr_t)QUARANTINE_STATE_MAX) {
if (quarantine == NULL) {
if ((quarantine = quarantine_init(LG_MAXOBJS_INIT)) ==
NULL) {
idalloc(ptr);
return;
}
} else {
if (quarantine == QUARANTINE_STATE_PURGATORY) { if (quarantine == QUARANTINE_STATE_PURGATORY) {
/* /*
* Make a note that quarantine() was called * Make a note that quarantine() was called after
* after quarantine_cleanup() was called. * quarantine_cleanup() was called.
*/ */
quarantine = QUARANTINE_STATE_REINCARNATED; quarantine = QUARANTINE_STATE_REINCARNATED;
quarantine_tsd_set(&quarantine); quarantine_tsd_set(&quarantine);
@ -137,7 +109,6 @@ quarantine(void *ptr)
idalloc(ptr); idalloc(ptr);
return; return;
} }
}
/* /*
* Drain one or more objects if the quarantine size limit would be * Drain one or more objects if the quarantine size limit would be
* exceeded by appending ptr. * exceeded by appending ptr.
@ -169,7 +140,7 @@ quarantine(void *ptr)
} }
} }
static void void
quarantine_cleanup(void *arg) quarantine_cleanup(void *arg)
{ {
quarantine_t *quarantine = *(quarantine_t **)arg; quarantine_t *quarantine = *(quarantine_t **)arg;