From 36366f3c4c741723369853c923e56999716398fc Mon Sep 17 00:00:00 2001 From: Ivan Zaitsev Date: Wed, 20 Jul 2022 15:25:56 -0700 Subject: [PATCH] Add double free detection in thread cache for debug build Add new runtime option `debug_double_free_max_scan` that specifies the max number of stack entries to scan in the cache bit when trying to detect the double free bug (currently debug build only). --- include/jemalloc/internal/cache_bin.h | 34 +++++++++++++ .../internal/jemalloc_internal_externs.h | 1 + include/jemalloc/internal/safety_check.h | 2 + src/ctl.c | 7 ++- src/jemalloc.c | 11 +++++ src/stats.c | 1 + test/unit/double_free.c | 49 ++++++++++++++++--- test/unit/mallctl.c | 1 + 8 files changed, 97 insertions(+), 9 deletions(-) diff --git a/include/jemalloc/internal/cache_bin.h b/include/jemalloc/internal/cache_bin.h index caf5be33..87c7ea5e 100644 --- a/include/jemalloc/internal/cache_bin.h +++ b/include/jemalloc/internal/cache_bin.h @@ -2,6 +2,7 @@ #define JEMALLOC_INTERNAL_CACHE_BIN_H #include "jemalloc/internal/ql.h" +#include "jemalloc/internal/safety_check.h" #include "jemalloc/internal/sz.h" /* @@ -427,6 +428,35 @@ cache_bin_full(cache_bin_t *bin) { return ((uint16_t)(uintptr_t)bin->stack_head == bin->low_bits_full); } +/* + * Scans the allocated area of the cache_bin for the given pointer up to limit. + * Fires safety_check_fail if the ptr is found and returns true. + */ +JEMALLOC_ALWAYS_INLINE bool +cache_bin_dalloc_safety_checks(cache_bin_t *bin, void *ptr) { + if (!config_debug || opt_debug_double_free_max_scan == 0) { + return false; + } + + cache_bin_sz_t ncached = cache_bin_ncached_get_internal(bin, false); + unsigned max_scan = opt_debug_double_free_max_scan < ncached + ? opt_debug_double_free_max_scan + : ncached; + + void **cur = bin->stack_head; + void **limit = cur + max_scan; + for (; cur < limit; cur++) { + if (*cur == ptr) { + safety_check_fail( + "Invalid deallocation detected: double free of " + "pointer %p\n", + ptr); + return true; + } + } + return false; +} + /* * Free an object into the given bin. Fails only if the bin is full. */ @@ -436,6 +466,10 @@ cache_bin_dalloc_easy(cache_bin_t *bin, void *ptr) { return false; } + if (unlikely(cache_bin_dalloc_safety_checks(bin, ptr))) { + return true; + } + bin->stack_head--; *bin->stack_head = ptr; cache_bin_assert_earlier(bin, bin->low_bits_full, diff --git a/include/jemalloc/internal/jemalloc_internal_externs.h b/include/jemalloc/internal/jemalloc_internal_externs.h index fc834c67..63b9bd2c 100644 --- a/include/jemalloc/internal/jemalloc_internal_externs.h +++ b/include/jemalloc/internal/jemalloc_internal_externs.h @@ -34,6 +34,7 @@ extern malloc_init_t malloc_init_state; extern const char *zero_realloc_mode_names[]; extern atomic_zu_t zero_realloc_count; extern bool opt_cache_oblivious; +extern unsigned opt_debug_double_free_max_scan; /* Escape free-fastpath when ptr & mask == 0 (for sanitization purpose). */ extern uintptr_t san_cache_bin_nonfast_mask; diff --git a/include/jemalloc/internal/safety_check.h b/include/jemalloc/internal/safety_check.h index f1a74f17..900cfa55 100644 --- a/include/jemalloc/internal/safety_check.h +++ b/include/jemalloc/internal/safety_check.h @@ -1,6 +1,8 @@ #ifndef JEMALLOC_INTERNAL_SAFETY_CHECK_H #define JEMALLOC_INTERNAL_SAFETY_CHECK_H +#define SAFETY_CHECK_DOUBLE_FREE_MAX_SCAN_DEFAULT 32 + void safety_check_fail_sized_dealloc(bool current_dealloc, const void *ptr, size_t true_size, size_t input_size); void safety_check_fail(const char *format, ...); diff --git a/src/ctl.c b/src/ctl.c index 135271ba..e942cb1a 100644 --- a/src/ctl.c +++ b/src/ctl.c @@ -92,6 +92,7 @@ CTL_PROTO(config_xmalloc) CTL_PROTO(opt_abort) CTL_PROTO(opt_abort_conf) CTL_PROTO(opt_cache_oblivious) +CTL_PROTO(opt_debug_double_free_max_scan) CTL_PROTO(opt_trust_madvise) CTL_PROTO(opt_confirm_conf) CTL_PROTO(opt_hpa) @@ -479,7 +480,9 @@ static const ctl_named_node_t opt_node[] = { {NAME("prof_sys_thread_name"), CTL(opt_prof_sys_thread_name)}, {NAME("prof_time_resolution"), CTL(opt_prof_time_res)}, {NAME("lg_san_uaf_align"), CTL(opt_lg_san_uaf_align)}, - {NAME("zero_realloc"), CTL(opt_zero_realloc)} + {NAME("zero_realloc"), CTL(opt_zero_realloc)}, + {NAME("debug_double_free_max_scan"), + CTL(opt_debug_double_free_max_scan)} }; static const ctl_named_node_t tcache_node[] = { @@ -2128,6 +2131,8 @@ CTL_RO_CONFIG_GEN(config_xmalloc, bool) CTL_RO_NL_GEN(opt_abort, opt_abort, bool) CTL_RO_NL_GEN(opt_abort_conf, opt_abort_conf, bool) CTL_RO_NL_GEN(opt_cache_oblivious, opt_cache_oblivious, bool) +CTL_RO_NL_GEN(opt_debug_double_free_max_scan, + opt_debug_double_free_max_scan, unsigned) CTL_RO_NL_GEN(opt_trust_madvise, opt_trust_madvise, bool) CTL_RO_NL_GEN(opt_confirm_conf, opt_confirm_conf, bool) diff --git a/src/jemalloc.c b/src/jemalloc.c index 7ccbf8ac..83d69dd0 100644 --- a/src/jemalloc.c +++ b/src/jemalloc.c @@ -154,6 +154,9 @@ fxp_t opt_narenas_ratio = FXP_INIT_INT(4); unsigned ncpus; +unsigned opt_debug_double_free_max_scan = + SAFETY_CHECK_DOUBLE_FREE_MAX_SCAN_DEFAULT; + /* Protects arenas initialization. */ malloc_mutex_t arenas_lock; @@ -1420,6 +1423,10 @@ malloc_conf_init_helper(sc_data_t *sc_data, unsigned bin_shard_sizes[SC_NBINS], CONF_HANDLE_UNSIGNED(opt_lg_tcache_flush_large_div, "lg_tcache_flush_large_div", 1, 16, CONF_CHECK_MIN, CONF_CHECK_MAX, /* clip */ true) + CONF_HANDLE_UNSIGNED(opt_debug_double_free_max_scan, + "debug_double_free_max_scan", 0, UINT_MAX, + CONF_DONT_CHECK_MIN, CONF_DONT_CHECK_MAX, + /* clip */ false) /* * The runtime option of oversize_threshold remains @@ -1737,6 +1744,10 @@ malloc_conf_init_check_deps(void) { "prof_final.\n"); return true; } + /* To emphasize in the stats output that opt is disabled when !debug. */ + if (!config_debug) { + opt_debug_double_free_max_scan = 0; + } return false; } diff --git a/src/stats.c b/src/stats.c index efc70fd3..d150baef 100644 --- a/src/stats.c +++ b/src/stats.c @@ -1518,6 +1518,7 @@ stats_general_print(emitter_t *emitter) { OPT_WRITE_SIZE_T("tcache_gc_delay_bytes") OPT_WRITE_UNSIGNED("lg_tcache_flush_small_div") OPT_WRITE_UNSIGNED("lg_tcache_flush_large_div") + OPT_WRITE_UNSIGNED("debug_double_free_max_scan") OPT_WRITE_CHAR_P("thp") OPT_WRITE_BOOL("prof") OPT_WRITE_CHAR_P("prof_prefix") diff --git a/test/unit/double_free.c b/test/unit/double_free.c index 12122c1b..b52fcf90 100644 --- a/test/unit/double_free.c +++ b/test/unit/double_free.c @@ -10,13 +10,13 @@ void fake_abort(const char *message) { } void -test_large_double_free_pre(void) { +test_double_free_pre(void) { safety_check_set_abort(&fake_abort); fake_abort_called = false; } void -test_large_double_free_post() { +test_double_free_post() { expect_b_eq(fake_abort_called, true, "Double-free check didn't fire."); safety_check_set_abort(NULL); } @@ -29,7 +29,7 @@ TEST_BEGIN(test_large_double_free_tcache) { */ test_skip_if(config_debug); - test_large_double_free_pre(); + test_double_free_pre(); char *ptr = malloc(SC_LARGE_MINCLASS); bool guarded = extent_is_guarded(tsdn_fetch(), ptr); free(ptr); @@ -44,7 +44,7 @@ TEST_BEGIN(test_large_double_free_tcache) { fake_abort_called = true; } mallctl("thread.tcache.flush", NULL, NULL, NULL, 0); - test_large_double_free_post(); + test_double_free_post(); } TEST_END @@ -52,7 +52,7 @@ TEST_BEGIN(test_large_double_free_no_tcache) { test_skip_if(!config_opt_safety_checks); test_skip_if(config_debug); - test_large_double_free_pre(); + test_double_free_pre(); char *ptr = mallocx(SC_LARGE_MINCLASS, MALLOCX_TCACHE_NONE); bool guarded = extent_is_guarded(tsdn_fetch(), ptr); dallocx(ptr, MALLOCX_TCACHE_NONE); @@ -66,12 +66,45 @@ TEST_BEGIN(test_large_double_free_no_tcache) { */ fake_abort_called = true; } - test_large_double_free_post(); + test_double_free_post(); +} +TEST_END + +TEST_BEGIN(test_small_double_free_tcache) { + test_skip_if(!config_debug); + + test_skip_if(opt_debug_double_free_max_scan == 0); + + bool tcache_enabled; + size_t sz = sizeof(tcache_enabled); + assert_d_eq( + mallctl("thread.tcache.enabled", &tcache_enabled, &sz, NULL, 0), 0, + "Unexpected mallctl failure"); + test_skip_if(!tcache_enabled); + + test_double_free_pre(); + char *ptr = malloc(1); + bool guarded = extent_is_guarded(tsdn_fetch(), ptr); + free(ptr); + if (!guarded) { + free(ptr); + } else { + /* + * Skip because guarded extents may unguard immediately on + * deallocation, in which case the second free will crash before + * reaching the intended safety check. + */ + fake_abort_called = true; + } + mallctl("thread.tcache.flush", NULL, NULL, NULL, 0); + test_double_free_post(); } TEST_END int main(void) { - return test(test_large_double_free_no_tcache, - test_large_double_free_tcache); + return test( + test_large_double_free_no_tcache, + test_large_double_free_tcache, + test_small_double_free_tcache); } diff --git a/test/unit/mallctl.c b/test/unit/mallctl.c index 6efc8f1b..62bd1a2d 100644 --- a/test/unit/mallctl.c +++ b/test/unit/mallctl.c @@ -325,6 +325,7 @@ TEST_BEGIN(test_mallctl_opt) { TEST_MALLCTL_OPT(bool, prof_stats, prof); TEST_MALLCTL_OPT(bool, prof_sys_thread_name, prof); TEST_MALLCTL_OPT(ssize_t, lg_san_uaf_align, uaf_detection); + TEST_MALLCTL_OPT(unsigned, debug_double_free_max_scan, always); #undef TEST_MALLCTL_OPT }