diff --git a/Makefile.in b/Makefile.in index 21a10532..e4d21805 100644 --- a/Makefile.in +++ b/Makefile.in @@ -232,7 +232,10 @@ TESTS_UNIT := \ $(srcroot)test/unit/nstime.c \ $(srcroot)test/unit/tsd.c \ $(srcroot)test/unit/witness.c \ - $(srcroot)test/unit/zero.c + $(srcroot)test/unit/zero.c \ + $(srcroot)test/unit/zero_realloc_abort.c \ + $(srcroot)test/unit/zero_realloc_free.c \ + $(srcroot)test/unit/zero_realloc_strict.c ifeq (@enable_prof@, 1) TESTS_UNIT += \ $(srcroot)test/unit/arena_reset_prof.c diff --git a/doc/jemalloc.xml.in b/doc/jemalloc.xml.in index e83bfbff..746c6bdb 100644 --- a/doc/jemalloc.xml.in +++ b/doc/jemalloc.xml.in @@ -1489,6 +1489,33 @@ malloc_conf = "xmalloc:true";]]> by default. + + + opt.zero_realloc + (const char *) + r- + + Determines the behavior of + realloc() when passed a value of zero for the new + size. strict treats this as an allocation of size zero + (and returns a non-null result except in case of resource exhaustion). + free treats this as a deallocation of the pointer, and + returns NULL without setting + errno. abort aborts the process if + zero is passed. The default is strict. + + There is considerable divergence of behaviors across + implementations in handling this case. Many have the behavior of + free. This can introduce security vulnerabilities, since + a NULL return value indicates failure, and the + continued validity of the passed-in pointer (per POSIX and C11). + strict is safe, but can cause leaks in programs that + expect the common behavior. Programs intended to be portable and + leak-free cannot assume either behavior, and must therefore never call + realloc with a size of 0. The abort option enables these + testing this behavior. + + thread.arena diff --git a/include/jemalloc/internal/jemalloc_internal_externs.h b/include/jemalloc/internal/jemalloc_internal_externs.h index d291170b..dae77b42 100644 --- a/include/jemalloc/internal/jemalloc_internal_externs.h +++ b/include/jemalloc/internal/jemalloc_internal_externs.h @@ -18,6 +18,8 @@ extern bool opt_utrace; extern bool opt_xmalloc; extern bool opt_zero; extern unsigned opt_narenas; +extern zero_realloc_action_t opt_zero_realloc_action; +extern const char *zero_realloc_mode_names[]; /* Number of CPUs. */ extern unsigned ncpus; diff --git a/include/jemalloc/internal/jemalloc_internal_types.h b/include/jemalloc/internal/jemalloc_internal_types.h index e296c5a7..324a4b13 100644 --- a/include/jemalloc/internal/jemalloc_internal_types.h +++ b/include/jemalloc/internal/jemalloc_internal_types.h @@ -12,6 +12,17 @@ typedef unsigned szind_t; /* Processor / core id type. */ typedef int malloc_cpuid_t; +/* When realloc(non-null-ptr, 0) is called, what happens? */ +enum zero_realloc_action_e { + /* Realloc(ptr, 0) is free(ptr); return malloc(0); */ + zero_realloc_action_strict = 0, + /* Realloc(ptr, 0) is free(ptr); */ + zero_realloc_action_free = 1, + /* Realloc(ptr, 0) aborts. */ + zero_realloc_action_abort = 2 +}; +typedef enum zero_realloc_action_e zero_realloc_action_t; + /* * Flags bits: * diff --git a/src/ctl.c b/src/ctl.c index 206af4cc..b51207f8 100644 --- a/src/ctl.c +++ b/src/ctl.c @@ -112,6 +112,7 @@ CTL_PROTO(opt_prof_gdump) CTL_PROTO(opt_prof_final) CTL_PROTO(opt_prof_leak) CTL_PROTO(opt_prof_accum) +CTL_PROTO(opt_zero_realloc) CTL_PROTO(tcache_create) CTL_PROTO(tcache_flush) CTL_PROTO(tcache_destroy) @@ -339,7 +340,8 @@ static const ctl_named_node_t opt_node[] = { {NAME("prof_gdump"), CTL(opt_prof_gdump)}, {NAME("prof_final"), CTL(opt_prof_final)}, {NAME("prof_leak"), CTL(opt_prof_leak)}, - {NAME("prof_accum"), CTL(opt_prof_accum)} + {NAME("prof_accum"), CTL(opt_prof_accum)}, + {NAME("zero_realloc"), CTL(opt_zero_realloc)} }; static const ctl_named_node_t tcache_node[] = { @@ -1793,6 +1795,8 @@ CTL_RO_NL_CGEN(config_prof, opt_lg_prof_interval, opt_lg_prof_interval, ssize_t) CTL_RO_NL_CGEN(config_prof, opt_prof_gdump, opt_prof_gdump, bool) CTL_RO_NL_CGEN(config_prof, opt_prof_final, opt_prof_final, bool) CTL_RO_NL_CGEN(config_prof, opt_prof_leak, opt_prof_leak, bool) +CTL_RO_NL_GEN(opt_zero_realloc, + zero_realloc_mode_names[opt_zero_realloc_action], const char *) /******************************************************************************/ diff --git a/src/jemalloc.c b/src/jemalloc.c index 8dd81bd8..35a9e7b5 100644 --- a/src/jemalloc.c +++ b/src/jemalloc.c @@ -67,6 +67,15 @@ bool opt_junk_free = #endif ; +zero_realloc_action_t opt_zero_realloc_action = + zero_realloc_action_strict; + +const char *zero_realloc_mode_names[] = { + "strict", + "free", + "abort", +}; + bool opt_utrace = false; bool opt_xmalloc = false; bool opt_zero = false; @@ -1411,6 +1420,22 @@ malloc_conf_init_helper(sc_data_t *sc_data, unsigned bin_shard_sizes[SC_NBINS], } CONF_CONTINUE; } + if (CONF_MATCH("zero_realloc")) { + if (CONF_MATCH_VALUE("strict")) { + opt_zero_realloc_action + = zero_realloc_action_strict; + } else if (CONF_MATCH_VALUE("free")) { + opt_zero_realloc_action + = zero_realloc_action_free; + } else if (CONF_MATCH_VALUE("abort")) { + opt_zero_realloc_action + = zero_realloc_action_abort; + } else { + CONF_ERROR("Invalid conf value", + k, klen, v, vlen); + } + CONF_CONTINUE; + } CONF_ERROR("Invalid conf pair", k, klen, v, vlen); #undef CONF_ERROR #undef CONF_CONTINUE @@ -3133,18 +3158,17 @@ je_rallocx(void *ptr, size_t size, int flags) { return ret; } -JEMALLOC_EXPORT JEMALLOC_ALLOCATOR JEMALLOC_RESTRICT_RETURN -void JEMALLOC_NOTHROW * -JEMALLOC_ALLOC_SIZE(2) -je_realloc(void *ptr, size_t size) { - LOG("core.realloc.entry", "ptr: %p, size: %zu\n", ptr, size); - - if (likely(ptr != NULL && size != 0)) { - void *ret = do_rallocx(ptr, size, 0, true); - LOG("core.realloc.exit", "result: %p", ret); - return ret; - } else if (ptr != NULL && size == 0) { - /* realloc(ptr, 0) is equivalent to free(ptr). */ +static void * +do_realloc_nonnull_zero(void *ptr) { + if (opt_zero_realloc_action == zero_realloc_action_strict) { + /* + * The user might have gotten a strict setting while expecting a + * free setting. If that's the case, we at least try to + * reduce the harm, and turn off the tcache while allocating, so + * that we'll get a true first fit. + */ + return do_rallocx(ptr, 1, MALLOCX_TCACHE_NONE, true); + } else if (opt_zero_realloc_action == zero_realloc_action_free) { UTRACE(ptr, 0, 0); tcache_t *tcache; tsd_t *tsd = tsd_fetch(); @@ -3156,14 +3180,39 @@ je_realloc(void *ptr, size_t size) { tcache = NULL; } - uintptr_t args[3] = {(uintptr_t)ptr, size}; + uintptr_t args[3] = {(uintptr_t)ptr, 0}; hook_invoke_dalloc(hook_dalloc_realloc, ptr, args); ifree(tsd, ptr, tcache, true); check_entry_exit_locking(tsd_tsdn(tsd)); - LOG("core.realloc.exit", "result: %p", NULL); return NULL; + } else { + safety_check_fail("Called realloc(non-null-ptr, 0) with " + "zero_realloc:abort set\n"); + /* In real code, this will never run; the safety check failure + * will call abort. In the unit test, we just want to bail out + * without corrupting internal state that the test needs to + * finish. + */ + return NULL; + } +} + +JEMALLOC_EXPORT JEMALLOC_ALLOCATOR JEMALLOC_RESTRICT_RETURN +void JEMALLOC_NOTHROW * +JEMALLOC_ALLOC_SIZE(2) +je_realloc(void *ptr, size_t size) { + LOG("core.realloc.entry", "ptr: %p, size: %zu\n", ptr, size); + + if (likely(ptr != NULL && size != 0)) { + void *ret = do_rallocx(ptr, size, 0, true); + LOG("core.realloc.exit", "result: %p", ret); + return ret; + } else if (ptr != NULL && size == 0) { + void *ret = do_realloc_nonnull_zero(ptr); + LOG("core.realloc.exit", "result: %p", ret); + return ret; } else { /* realloc(NULL, size) is equivalent to malloc(size). */ void *ret; diff --git a/src/stats.c b/src/stats.c index 2b744e19..c9bab4f7 100644 --- a/src/stats.c +++ b/src/stats.c @@ -1109,6 +1109,7 @@ stats_general_print(emitter_t *emitter) { OPT_WRITE_BOOL("prof_leak") OPT_WRITE_BOOL("stats_print") OPT_WRITE_CHAR_P("stats_print_opts") + OPT_WRITE_CHAR_P("zero_realloc") emitter_dict_end(emitter); diff --git a/test/unit/hook.c b/test/unit/hook.c index 72fcc433..36dcb89d 100644 --- a/test/unit/hook.c +++ b/test/unit/hook.c @@ -428,15 +428,21 @@ TEST_BEGIN(test_hooks_realloc_as_malloc_or_free) { free(ptr); /* realloc(ptr, 0) as free */ - ptr = malloc(1); - reset(); - realloc(ptr, 0); - assert_d_eq(call_count, 1, "Hook not called"); - assert_ptr_eq(arg_extra, (void *)123, "Wrong extra"); - assert_d_eq(arg_type, (int)hook_dalloc_realloc, "Wrong hook type"); - assert_ptr_eq(ptr, arg_address, "Wrong pointer freed"); - assert_u64_eq((uintptr_t)ptr, arg_args_raw[0], "Wrong raw arg"); - assert_u64_eq((uintptr_t)0, arg_args_raw[1], "Wrong raw arg"); + if (opt_zero_realloc_action == zero_realloc_action_free) { + ptr = malloc(1); + reset(); + realloc(ptr, 0); + assert_d_eq(call_count, 1, "Hook not called"); + assert_ptr_eq(arg_extra, (void *)123, "Wrong extra"); + assert_d_eq(arg_type, (int)hook_dalloc_realloc, + "Wrong hook type"); + assert_ptr_eq(ptr, arg_address, + "Wrong pointer freed"); + assert_u64_eq((uintptr_t)ptr, arg_args_raw[0], + "Wrong raw arg"); + assert_u64_eq((uintptr_t)0, arg_args_raw[1], + "Wrong raw arg"); + } /* realloc(NULL, 0) as malloc(0) */ reset(); diff --git a/test/unit/mallctl.c b/test/unit/mallctl.c index 0e88f314..4c0830f2 100644 --- a/test/unit/mallctl.c +++ b/test/unit/mallctl.c @@ -880,6 +880,16 @@ TEST_BEGIN(test_hooks_exhaustion) { } TEST_END +TEST_BEGIN(test_zero_realloc) { + const char *val; + size_t sz = sizeof(val); + int err = mallctl("opt.zero_realloc", &val, &sz, NULL, 0); + assert_d_eq(err, 0, "Unexpected mallctl result"); + assert_str_eq(val, "strict", + "Unexpected default zero_realloc_beahvior"); +} +TEST_END + int main(void) { return test( @@ -911,5 +921,6 @@ main(void) { test_prof_active, test_stats_arenas, test_hooks, - test_hooks_exhaustion); + test_hooks_exhaustion, + test_zero_realloc); } diff --git a/test/unit/zero_realloc_abort.c b/test/unit/zero_realloc_abort.c new file mode 100644 index 00000000..2f49392b --- /dev/null +++ b/test/unit/zero_realloc_abort.c @@ -0,0 +1,26 @@ +#include "test/jemalloc_test.h" + +#include + +static bool abort_called = false; + +void set_abort_called() { + abort_called = true; +}; + +TEST_BEGIN(test_realloc_abort) { + abort_called = false; + safety_check_set_abort(&set_abort_called); + void *ptr = mallocx(42, 0); + assert_ptr_not_null(ptr, "Unexpected mallocx error"); + ptr = realloc(ptr, 0); + assert_true(abort_called, "Realloc with zero size didn't abort"); +} +TEST_END + +int +main(void) { + return test( + test_realloc_abort); +} + diff --git a/test/unit/zero_realloc_abort.sh b/test/unit/zero_realloc_abort.sh new file mode 100644 index 00000000..37daeeaa --- /dev/null +++ b/test/unit/zero_realloc_abort.sh @@ -0,0 +1,3 @@ +#!/bin/sh + +export MALLOC_CONF="zero_realloc:abort" diff --git a/test/unit/zero_realloc_free.c b/test/unit/zero_realloc_free.c new file mode 100644 index 00000000..a0736881 --- /dev/null +++ b/test/unit/zero_realloc_free.c @@ -0,0 +1,33 @@ +#include "test/jemalloc_test.h" + +static uint64_t +deallocated() { + if (!config_stats) { + return 0; + } + uint64_t deallocated; + size_t sz = sizeof(deallocated); + assert_d_eq(mallctl("thread.deallocated", (void *)&deallocated, &sz, + NULL, 0), 0, "Unexpected mallctl failure"); + return deallocated; +} + +TEST_BEGIN(test_realloc_free) { + void *ptr = mallocx(42, 0); + assert_ptr_not_null(ptr, "Unexpected mallocx error"); + uint64_t deallocated_before = deallocated(); + ptr = realloc(ptr, 0); + uint64_t deallocated_after = deallocated(); + assert_ptr_null(ptr, "Realloc didn't free"); + if (config_stats) { + assert_u64_gt(deallocated_after, deallocated_before, + "Realloc didn't free"); + } +} +TEST_END + +int +main(void) { + return test( + test_realloc_free); +} diff --git a/test/unit/zero_realloc_free.sh b/test/unit/zero_realloc_free.sh new file mode 100644 index 00000000..51b01c91 --- /dev/null +++ b/test/unit/zero_realloc_free.sh @@ -0,0 +1,3 @@ +#!/bin/sh + +export MALLOC_CONF="zero_realloc:free" diff --git a/test/unit/zero_realloc_strict.c b/test/unit/zero_realloc_strict.c new file mode 100644 index 00000000..b7099517 --- /dev/null +++ b/test/unit/zero_realloc_strict.c @@ -0,0 +1,48 @@ +#include "test/jemalloc_test.h" + +static uint64_t +allocated() { + if (!config_stats) { + return 0; + } + uint64_t allocated; + size_t sz = sizeof(allocated); + assert_d_eq(mallctl("thread.allocated", (void *)&allocated, &sz, NULL, + 0), 0, "Unexpected mallctl failure"); + return allocated; +} + +static uint64_t +deallocated() { + if (!config_stats) { + return 0; + } + uint64_t deallocated; + size_t sz = sizeof(deallocated); + assert_d_eq(mallctl("thread.deallocated", (void *)&deallocated, &sz, + NULL, 0), 0, "Unexpected mallctl failure"); + return deallocated; +} + +TEST_BEGIN(test_realloc_strict) { + void *ptr = mallocx(1, 0); + assert_ptr_not_null(ptr, "Unexpected mallocx error"); + uint64_t allocated_before = allocated(); + uint64_t deallocated_before = deallocated(); + ptr = realloc(ptr, 0); + uint64_t allocated_after = allocated(); + uint64_t deallocated_after = deallocated(); + if (config_stats) { + assert_u64_lt(allocated_before, allocated_after, + "Unexpected stats change"); + assert_u64_lt(deallocated_before, deallocated_after, + "Unexpected stats change"); + } + dallocx(ptr, 0); +} +TEST_END +int +main(void) { + return test( + test_realloc_strict); +} diff --git a/test/unit/zero_realloc_strict.sh b/test/unit/zero_realloc_strict.sh new file mode 100644 index 00000000..314dcd0a --- /dev/null +++ b/test/unit/zero_realloc_strict.sh @@ -0,0 +1,3 @@ +#!/bin/sh + +export MALLOC_CONF="zero_realloc:strict"