diff --git a/Makefile.in b/Makefile.in index ba0c80b6..008cffd8 100644 --- a/Makefile.in +++ b/Makefile.in @@ -204,6 +204,7 @@ TESTS_UNIT := \ $(srcroot)test/unit/counter.c \ $(srcroot)test/unit/decay.c \ $(srcroot)test/unit/div.c \ + $(srcroot)test/unit/double_free.c \ $(srcroot)test/unit/edata_cache.c \ $(srcroot)test/unit/emitter.c \ $(srcroot)test/unit/extent_quantize.c \ @@ -308,7 +309,6 @@ TESTS_STRESS := $(srcroot)test/stress/batch_alloc.c \ $(srcroot)test/stress/large_microbench.c \ $(srcroot)test/stress/mallctl.c \ $(srcroot)test/stress/microbench.c - TESTS := $(TESTS_UNIT) $(TESTS_INTEGRATION) $(TESTS_INTEGRATION_CPP) \ diff --git a/include/jemalloc/internal/arena_inlines_b.h b/include/jemalloc/internal/arena_inlines_b.h index 335c0797..7971b4c7 100644 --- a/include/jemalloc/internal/arena_inlines_b.h +++ b/include/jemalloc/internal/arena_inlines_b.h @@ -5,6 +5,7 @@ #include "jemalloc/internal/jemalloc_internal_types.h" #include "jemalloc/internal/mutex.h" #include "jemalloc/internal/rtree.h" +#include "jemalloc/internal/safety_check.h" #include "jemalloc/internal/sc.h" #include "jemalloc/internal/sz.h" #include "jemalloc/internal/ticker.h" @@ -203,6 +204,32 @@ arena_vsalloc(tsdn_t *tsdn, const void *ptr) { return sz_index2size(full_alloc_ctx.szind); } +JEMALLOC_ALWAYS_INLINE bool +large_dalloc_safety_checks(edata_t *edata, szind_t szind) { + if (!config_opt_safety_checks) { + return false; + } + + /* + * Eagerly detect double free and sized dealloc bugs for large sizes. + * The cost is low enough (as edata will be accessed anyway) to be + * enabled all the time. + */ + if (unlikely(edata_state_get(edata) != extent_state_active)) { + safety_check_fail("Invalid deallocation detected: " + "pages being freed (%p) not currently active, " + "possibly caused by double free bugs.", + (uintptr_t)edata_addr_get(edata)); + return true; + } + if (unlikely(sz_index2size(szind) != edata_usize_get(edata))) { + safety_check_fail_sized_dealloc(/* current_dealloc */ true); + return true; + } + + return false; +} + static inline void arena_dalloc_large_no_tcache(tsdn_t *tsdn, void *ptr, szind_t szind) { if (config_prof && unlikely(szind < SC_NBINS)) { @@ -210,6 +237,10 @@ arena_dalloc_large_no_tcache(tsdn_t *tsdn, void *ptr, szind_t szind) { } else { edata_t *edata = emap_edata_lookup(tsdn, &arena_emap_global, ptr); + if (large_dalloc_safety_checks(edata, szind)) { + /* See the comment in isfree. */ + return; + } large_dalloc(tsdn, edata); } } @@ -250,6 +281,10 @@ arena_dalloc_large(tsdn_t *tsdn, void *ptr, tcache_t *tcache, szind_t szind, } else { edata_t *edata = emap_edata_lookup(tsdn, &arena_emap_global, ptr); + if (large_dalloc_safety_checks(edata, szind)) { + /* See the comment in isfree. */ + return; + } large_dalloc(tsdn, edata); } } diff --git a/src/jemalloc.c b/src/jemalloc.c index bbf62555..1d6191ae 100644 --- a/src/jemalloc.c +++ b/src/jemalloc.c @@ -2812,7 +2812,7 @@ maybe_check_alloc_ctx(tsd_t *tsd, void *ptr, emap_alloc_ctx_t *alloc_ctx) { &dbg_ctx); if (alloc_ctx->szind != dbg_ctx.szind) { safety_check_fail_sized_dealloc( - /* curent_dealloc */ true); + /* current_dealloc */ true); return true; } if (alloc_ctx->slab != dbg_ctx.slab) { diff --git a/src/tcache.c b/src/tcache.c index 90ca372e..06efe66a 100644 --- a/src/tcache.c +++ b/src/tcache.c @@ -428,6 +428,10 @@ tcache_bin_flush_impl(tsd_t *tsd, tcache_t *tcache, cache_bin_t *cache_bin, dalloc_count++; } } else { + if (large_dalloc_safety_checks(edata, binind)) { + /* See the comment in isfree. */ + continue; + } large_dalloc_finish(tsdn, edata); } } diff --git a/test/unit/double_free.c b/test/unit/double_free.c new file mode 100644 index 00000000..73155b9c --- /dev/null +++ b/test/unit/double_free.c @@ -0,0 +1,56 @@ +#include "test/jemalloc_test.h" + +#include "jemalloc/internal/safety_check.h" + +bool fake_abort_called; +void fake_abort(const char *message) { + (void)message; + fake_abort_called = true; +} + +void +test_large_double_free_pre(void) { + safety_check_set_abort(&fake_abort); + fake_abort_called = false; +} + +void +test_large_double_free_post() { + expect_b_eq(fake_abort_called, true, "Double-free check didn't fire."); + safety_check_set_abort(NULL); +} + +TEST_BEGIN(test_large_double_free_tcache) { + test_skip_if(!config_opt_safety_checks); + /* + * Skip debug builds, since too many assertions will be triggered with + * double-free before hitting the one we are interested in. + */ + test_skip_if(config_debug); + + test_large_double_free_pre(); + char *ptr = malloc(SC_LARGE_MINCLASS); + free(ptr); + free(ptr); + mallctl("thread.tcache.flush", NULL, NULL, NULL, 0); + test_large_double_free_post(); +} +TEST_END + +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(); + char *ptr = mallocx(SC_LARGE_MINCLASS, MALLOCX_TCACHE_NONE); + dallocx(ptr, MALLOCX_TCACHE_NONE); + dallocx(ptr, MALLOCX_TCACHE_NONE); + test_large_double_free_post(); +} +TEST_END + +int +main(void) { + return test(test_large_double_free_no_tcache, + test_large_double_free_tcache); +} diff --git a/test/unit/double_free.h b/test/unit/double_free.h new file mode 100644 index 00000000..8b137891 --- /dev/null +++ b/test/unit/double_free.h @@ -0,0 +1 @@ + diff --git a/test/unit/size_check.c b/test/unit/size_check.c index 3d2912df..accdc405 100644 --- a/test/unit/size_check.c +++ b/test/unit/size_check.c @@ -8,48 +8,65 @@ void fake_abort(const char *message) { fake_abort_called = true; } -#define SIZE1 SC_SMALL_MAXCLASS -#define SIZE2 (SC_SMALL_MAXCLASS / 2) +#define SMALL_SIZE1 SC_SMALL_MAXCLASS +#define SMALL_SIZE2 (SC_SMALL_MAXCLASS / 2) -TEST_BEGIN(test_invalid_size_sdallocx) { - test_skip_if(!config_opt_size_checks); +#define LARGE_SIZE1 SC_LARGE_MINCLASS +#define LARGE_SIZE2 (LARGE_SIZE1 * 2) + +void * +test_invalid_size_pre(size_t sz) { safety_check_set_abort(&fake_abort); fake_abort_called = false; - void *ptr = malloc(SIZE1); + void *ptr = malloc(sz); assert_ptr_not_null(ptr, "Unexpected failure"); - sdallocx(ptr, SIZE2, 0); - expect_true(fake_abort_called, "Safety check didn't fire"); + return ptr; +} + +void +test_invalid_size_post(void) { + expect_true(fake_abort_called, "Safety check didn't fire"); safety_check_set_abort(NULL); } + +TEST_BEGIN(test_invalid_size_sdallocx) { + test_skip_if(!config_opt_size_checks); + + void *ptr = test_invalid_size_pre(SMALL_SIZE1); + sdallocx(ptr, SMALL_SIZE2, 0); + test_invalid_size_post(); + + ptr = test_invalid_size_pre(LARGE_SIZE1); + sdallocx(ptr, LARGE_SIZE2, 0); + test_invalid_size_post(); +} TEST_END TEST_BEGIN(test_invalid_size_sdallocx_nonzero_flag) { test_skip_if(!config_opt_size_checks); - safety_check_set_abort(&fake_abort); - fake_abort_called = false; - void *ptr = malloc(SIZE1); - assert_ptr_not_null(ptr, "Unexpected failure"); - sdallocx(ptr, SIZE2, MALLOCX_TCACHE_NONE); - expect_true(fake_abort_called, "Safety check didn't fire"); + void *ptr = test_invalid_size_pre(SMALL_SIZE1); + sdallocx(ptr, SMALL_SIZE2, MALLOCX_TCACHE_NONE); + test_invalid_size_post(); - safety_check_set_abort(NULL); + ptr = test_invalid_size_pre(LARGE_SIZE1); + sdallocx(ptr, LARGE_SIZE2, MALLOCX_TCACHE_NONE); + test_invalid_size_post(); } TEST_END TEST_BEGIN(test_invalid_size_sdallocx_noflags) { test_skip_if(!config_opt_size_checks); - safety_check_set_abort(&fake_abort); - fake_abort_called = false; - void *ptr = malloc(SIZE1); - assert_ptr_not_null(ptr, "Unexpected failure"); - je_sdallocx_noflags(ptr, SIZE2); - expect_true(fake_abort_called, "Safety check didn't fire"); + void *ptr = test_invalid_size_pre(SMALL_SIZE1); + je_sdallocx_noflags(ptr, SMALL_SIZE2); + test_invalid_size_post(); - safety_check_set_abort(NULL); + ptr = test_invalid_size_pre(LARGE_SIZE1); + je_sdallocx_noflags(ptr, LARGE_SIZE2); + test_invalid_size_post(); } TEST_END