From 33e1dad6803ea3e20971b46baa299045f736d22a Mon Sep 17 00:00:00 2001 From: David Goldblatt Date: Fri, 22 Mar 2019 12:53:11 -0700 Subject: [PATCH] Safety checks: Add a redzoning feature. --- Makefile.in | 1 + include/jemalloc/internal/arena_externs.h | 2 +- include/jemalloc/internal/arena_inlines_b.h | 2 +- .../jemalloc/internal/jemalloc_preamble.h.in | 2 +- include/jemalloc/internal/prof_inlines_b.h | 3 +- include/jemalloc/internal/safety_check.h | 20 +++ src/arena.c | 22 ++- src/jemalloc.c | 1 + src/prof.c | 16 +- src/safety_check.c | 25 ++- test/unit/safety_check.c | 156 ++++++++++++++++++ test/unit/safety_check.sh | 5 + 12 files changed, 233 insertions(+), 22 deletions(-) create mode 100644 test/unit/safety_check.c create mode 100644 test/unit/safety_check.sh diff --git a/Makefile.in b/Makefile.in index 8b4a98fd..38722ff9 100644 --- a/Makefile.in +++ b/Makefile.in @@ -210,6 +210,7 @@ TESTS_UNIT := \ $(srcroot)test/unit/rb.c \ $(srcroot)test/unit/retained.c \ $(srcroot)test/unit/rtree.c \ + $(srcroot)test/unit/safety_check.c \ $(srcroot)test/unit/seq.c \ $(srcroot)test/unit/SFMT.c \ $(srcroot)test/unit/sc.c \ diff --git a/include/jemalloc/internal/arena_externs.h b/include/jemalloc/internal/arena_externs.h index 2bdddb77..a4523ae0 100644 --- a/include/jemalloc/internal/arena_externs.h +++ b/include/jemalloc/internal/arena_externs.h @@ -60,7 +60,7 @@ void *arena_malloc_hard(tsdn_t *tsdn, arena_t *arena, size_t size, szind_t ind, bool zero); void *arena_palloc(tsdn_t *tsdn, arena_t *arena, size_t usize, size_t alignment, bool zero, tcache_t *tcache); -void arena_prof_promote(tsdn_t *tsdn, const void *ptr, size_t usize); +void arena_prof_promote(tsdn_t *tsdn, void *ptr, size_t usize); void arena_dalloc_promoted(tsdn_t *tsdn, void *ptr, tcache_t *tcache, bool slow_path); void arena_dalloc_bin_junked_locked(tsdn_t *tsdn, arena_t *arena, bin_t *bin, diff --git a/include/jemalloc/internal/arena_inlines_b.h b/include/jemalloc/internal/arena_inlines_b.h index 614deddd..7e61a44c 100644 --- a/include/jemalloc/internal/arena_inlines_b.h +++ b/include/jemalloc/internal/arena_inlines_b.h @@ -90,7 +90,7 @@ arena_prof_alloc_time_get(tsdn_t *tsdn, const void *ptr, assert(ptr != NULL); extent_t *extent = iealloc(tsdn, ptr); - /* + /* * Unlike arena_prof_prof_tctx_{get, set}, we only call this once we're * sure we have a sampled allocation. */ diff --git a/include/jemalloc/internal/jemalloc_preamble.h.in b/include/jemalloc/internal/jemalloc_preamble.h.in index 9fd2a7f6..3418cbfa 100644 --- a/include/jemalloc/internal/jemalloc_preamble.h.in +++ b/include/jemalloc/internal/jemalloc_preamble.h.in @@ -166,7 +166,7 @@ static const bool config_log = * deallocations, double-frees, etc. */ static const bool config_opt_safety_checks = -#if defined(JEMALLOC_EXTRA_SAFETY_CHECKS) +#ifdef JEMALLOC_OPT_SAFETY_CHECKS true #elif defined(JEMALLOC_DEBUG) /* diff --git a/include/jemalloc/internal/prof_inlines_b.h b/include/jemalloc/internal/prof_inlines_b.h index 8358bffb..8ba8a1e1 100644 --- a/include/jemalloc/internal/prof_inlines_b.h +++ b/include/jemalloc/internal/prof_inlines_b.h @@ -1,6 +1,7 @@ #ifndef JEMALLOC_INTERNAL_PROF_INLINES_B_H #define JEMALLOC_INTERNAL_PROF_INLINES_B_H +#include "jemalloc/internal/safety_check.h" #include "jemalloc/internal/sz.h" JEMALLOC_ALWAYS_INLINE bool @@ -71,7 +72,7 @@ prof_alloc_time_get(tsdn_t *tsdn, const void *ptr, alloc_ctx_t *alloc_ctx) { JEMALLOC_ALWAYS_INLINE void prof_alloc_time_set(tsdn_t *tsdn, const void *ptr, alloc_ctx_t *alloc_ctx, - nstime_t t) { + nstime_t t) { cassert(config_prof); assert(ptr != NULL); diff --git a/include/jemalloc/internal/safety_check.h b/include/jemalloc/internal/safety_check.h index 52157d16..1b53fc4c 100644 --- a/include/jemalloc/internal/safety_check.h +++ b/include/jemalloc/internal/safety_check.h @@ -2,5 +2,25 @@ #define JEMALLOC_INTERNAL_SAFETY_CHECK_H void safety_check_fail(const char *format, ...); +/* Can set to NULL for a default. */ +void safety_check_set_abort(void (*abort_fn)()); + +JEMALLOC_ALWAYS_INLINE void +safety_check_set_redzone(void *ptr, size_t usize, size_t bumped_usize) { + assert(usize < bumped_usize); + for (size_t i = usize; i < bumped_usize && i < usize + 32; ++i) { + *((unsigned char *)ptr + usize) = 0xBC; + } +} + +JEMALLOC_ALWAYS_INLINE void +safety_check_verify_redzone(const void *ptr, size_t usize, size_t bumped_usize) +{ + for (size_t i = usize; i < bumped_usize && i < usize + 32; ++i) { + if (unlikely(*((unsigned char *)ptr + usize) != 0xBC)) { + safety_check_fail("Use after free error\n"); + } + } +} #endif /*JEMALLOC_INTERNAL_SAFETY_CHECK_H */ diff --git a/src/arena.c b/src/arena.c index 60eac232..084df855 100644 --- a/src/arena.c +++ b/src/arena.c @@ -8,6 +8,7 @@ #include "jemalloc/internal/extent_mmap.h" #include "jemalloc/internal/mutex.h" #include "jemalloc/internal/rtree.h" +#include "jemalloc/internal/safety_check.h" #include "jemalloc/internal/util.h" JEMALLOC_DIAGNOSTIC_DISABLE_SPURIOUS @@ -1531,12 +1532,16 @@ arena_palloc(tsdn_t *tsdn, arena_t *arena, size_t usize, size_t alignment, } void -arena_prof_promote(tsdn_t *tsdn, const void *ptr, size_t usize) { +arena_prof_promote(tsdn_t *tsdn, void *ptr, size_t usize) { cassert(config_prof); assert(ptr != NULL); assert(isalloc(tsdn, ptr) == SC_LARGE_MINCLASS); assert(usize <= SC_SMALL_MAXCLASS); + if (config_opt_safety_checks) { + safety_check_set_redzone(ptr, usize, SC_LARGE_MINCLASS); + } + rtree_ctx_t rtree_ctx_fallback; rtree_ctx_t *rtree_ctx = tsdn_rtree_ctx(tsdn, &rtree_ctx_fallback); @@ -1577,10 +1582,19 @@ arena_dalloc_promoted(tsdn_t *tsdn, void *ptr, tcache_t *tcache, assert(opt_prof); extent_t *extent = iealloc(tsdn, ptr); - size_t usize = arena_prof_demote(tsdn, extent, ptr); - if (usize <= tcache_maxclass) { + size_t usize = extent_usize_get(extent); + size_t bumped_usize = arena_prof_demote(tsdn, extent, ptr); + if (config_opt_safety_checks && usize < SC_LARGE_MINCLASS) { + /* + * Currently, we only do redzoning for small sampled + * allocations. + */ + assert(bumped_usize == SC_LARGE_MINCLASS); + safety_check_verify_redzone(ptr, usize, bumped_usize); + } + if (bumped_usize <= tcache_maxclass) { tcache_dalloc_large(tsdn_tsd(tsdn), tcache, ptr, - sz_size2index(usize), slow_path); + sz_size2index(bumped_usize), slow_path); } else { large_dalloc(tsdn, extent); } diff --git a/src/jemalloc.c b/src/jemalloc.c index 7bc7b957..818ce3aa 100644 --- a/src/jemalloc.c +++ b/src/jemalloc.c @@ -13,6 +13,7 @@ #include "jemalloc/internal/malloc_io.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/spin.h" #include "jemalloc/internal/sz.h" diff --git a/src/prof.c b/src/prof.c index 4d7d65db..a4e30f42 100644 --- a/src/prof.c +++ b/src/prof.c @@ -125,7 +125,7 @@ struct prof_thr_node_s { uint64_t thr_uid; /* Variable size based on thr_name_sz. */ char name[1]; -}; +}; typedef struct prof_alloc_node_s prof_alloc_node_t; @@ -388,7 +388,7 @@ prof_log_bt_index(tsd_t *tsd, prof_bt_t *bt) { new_node->next = NULL; new_node->index = log_bt_index; - /* + /* * Copy the backtrace: bt is inside a tdata or gctx, which * might die before prof_log_stop is called. */ @@ -402,7 +402,7 @@ prof_log_bt_index(tsd_t *tsd, prof_bt_t *bt) { } else { return node->index; } -} +} static size_t prof_log_thr_index(tsd_t *tsd, uint64_t thr_uid, const char *name) { assert(prof_logging_state == prof_logging_state_started); @@ -452,7 +452,7 @@ prof_try_log(tsd_t *tsd, const void *ptr, size_t usize, prof_tctx_t *tctx) { * it's being destroyed). */ return; - } + } malloc_mutex_lock(tsd_tsdn(tsd), &log_mtx); @@ -514,11 +514,11 @@ prof_try_log(tsd_t *tsd, const void *ptr, size_t usize, prof_tctx_t *tctx) { } label_done: - malloc_mutex_unlock(tsd_tsdn(tsd), &log_mtx); + malloc_mutex_unlock(tsd_tsdn(tsd), &log_mtx); } void -prof_free_sampled_object(tsd_t *tsd, const void *ptr, size_t usize, +prof_free_sampled_object(tsd_t *tsd, const void *ptr, size_t usize, prof_tctx_t *tctx) { malloc_mutex_lock(tsd_tsdn(tsd), tctx->tdata->lock); @@ -2604,8 +2604,8 @@ static void prof_log_emit_traces(tsd_t *tsd, emitter_t *emitter) { emitter_json_array_kv_begin(emitter, "stack_traces"); prof_bt_node_t *bt_node = log_bt_first; - prof_bt_node_t *bt_old_node; - /* + prof_bt_node_t *bt_old_node; + /* * Calculate how many hex digits we need: twice number of bytes, two for * "0x", and then one more for terminating '\0'. */ diff --git a/src/safety_check.c b/src/safety_check.c index cbec1907..804155dc 100644 --- a/src/safety_check.c +++ b/src/safety_check.c @@ -1,11 +1,24 @@ #include "jemalloc/internal/jemalloc_preamble.h" #include "jemalloc/internal/jemalloc_internal_includes.h" -void safety_check_fail(const char *format, ...) { - va_list ap; +static void (*safety_check_abort)(const char *message); - va_start(ap, format); - malloc_vcprintf(NULL, NULL, format, ap); - va_end(ap); - abort(); +void safety_check_set_abort(void (*abort_fn)(const char *)) { + safety_check_abort = abort_fn; +} + +void safety_check_fail(const char *format, ...) { + char buf[MALLOC_PRINTF_BUFSIZE]; + + va_list ap; + va_start(ap, format); + malloc_vsnprintf(buf, MALLOC_PRINTF_BUFSIZE, format, ap); + va_end(ap); + + if (safety_check_abort == NULL) { + malloc_write(buf); + abort(); + } else { + safety_check_abort(buf); + } } diff --git a/test/unit/safety_check.c b/test/unit/safety_check.c new file mode 100644 index 00000000..bf4bd86d --- /dev/null +++ b/test/unit/safety_check.c @@ -0,0 +1,156 @@ +#include "test/jemalloc_test.h" + +#include "jemalloc/internal/safety_check.h" + +/* + * Note that we get called through safety_check.sh, which turns on sampling for + * everything. + */ + +bool fake_abort_called; +void fake_abort(const char *message) { + (void)message; + fake_abort_called = true; +} + +TEST_BEGIN(test_malloc_free_overflow) { + test_skip_if(!config_prof); + test_skip_if(!config_opt_safety_checks); + + safety_check_set_abort(&fake_abort); + /* Buffer overflow! */ + char* ptr = malloc(128); + ptr[128] = 0; + free(ptr); + safety_check_set_abort(NULL); + + assert_b_eq(fake_abort_called, true, "Redzone check didn't fire."); + fake_abort_called = false; +} +TEST_END + +TEST_BEGIN(test_mallocx_dallocx_overflow) { + test_skip_if(!config_prof); + test_skip_if(!config_opt_safety_checks); + + safety_check_set_abort(&fake_abort); + /* Buffer overflow! */ + char* ptr = mallocx(128, 0); + ptr[128] = 0; + dallocx(ptr, 0); + safety_check_set_abort(NULL); + + assert_b_eq(fake_abort_called, true, "Redzone check didn't fire."); + fake_abort_called = false; +} +TEST_END + +TEST_BEGIN(test_malloc_sdallocx_overflow) { + test_skip_if(!config_prof); + test_skip_if(!config_opt_safety_checks); + + safety_check_set_abort(&fake_abort); + /* Buffer overflow! */ + char* ptr = malloc(128); + ptr[128] = 0; + sdallocx(ptr, 128, 0); + safety_check_set_abort(NULL); + + assert_b_eq(fake_abort_called, true, "Redzone check didn't fire."); + fake_abort_called = false; +} +TEST_END + +TEST_BEGIN(test_realloc_overflow) { + test_skip_if(!config_prof); + test_skip_if(!config_opt_safety_checks); + + safety_check_set_abort(&fake_abort); + /* Buffer overflow! */ + char* ptr = malloc(128); + ptr[128] = 0; + ptr = realloc(ptr, 129); + safety_check_set_abort(NULL); + free(ptr); + + assert_b_eq(fake_abort_called, true, "Redzone check didn't fire."); + fake_abort_called = false; +} +TEST_END + +TEST_BEGIN(test_rallocx_overflow) { + test_skip_if(!config_prof); + test_skip_if(!config_opt_safety_checks); + + safety_check_set_abort(&fake_abort); + /* Buffer overflow! */ + char* ptr = malloc(128); + ptr[128] = 0; + ptr = rallocx(ptr, 129, 0); + safety_check_set_abort(NULL); + free(ptr); + + assert_b_eq(fake_abort_called, true, "Redzone check didn't fire."); + fake_abort_called = false; +} +TEST_END + +TEST_BEGIN(test_xallocx_overflow) { + test_skip_if(!config_prof); + test_skip_if(!config_opt_safety_checks); + + safety_check_set_abort(&fake_abort); + /* Buffer overflow! */ + char* ptr = malloc(128); + ptr[128] = 0; + size_t result = xallocx(ptr, 129, 0, 0); + assert_zu_eq(result, 128, ""); + free(ptr); + assert_b_eq(fake_abort_called, true, "Redzone check didn't fire."); + fake_abort_called = false; + safety_check_set_abort(NULL); +} +TEST_END + +TEST_BEGIN(test_realloc_no_overflow) { + char* ptr = malloc(128); + ptr = realloc(ptr, 256); + ptr[128] = 0; + ptr[255] = 0; + free(ptr); + + ptr = malloc(128); + ptr = realloc(ptr, 64); + ptr[63] = 0; + ptr[0] = 0; + free(ptr); +} +TEST_END + +TEST_BEGIN(test_rallocx_no_overflow) { + char* ptr = malloc(128); + ptr = rallocx(ptr, 256, 0); + ptr[128] = 0; + ptr[255] = 0; + free(ptr); + + ptr = malloc(128); + ptr = rallocx(ptr, 64, 0); + ptr[63] = 0; + ptr[0] = 0; + free(ptr); +} +TEST_END + +int +main(void) { + return test( + test_malloc_free_overflow, + test_mallocx_dallocx_overflow, + test_malloc_sdallocx_overflow, + test_realloc_overflow, + test_rallocx_overflow, + test_xallocx_overflow, + test_realloc_no_overflow, + test_rallocx_no_overflow); +} diff --git a/test/unit/safety_check.sh b/test/unit/safety_check.sh new file mode 100644 index 00000000..8fcc7d8a --- /dev/null +++ b/test/unit/safety_check.sh @@ -0,0 +1,5 @@ +#!/bin/sh + +if [ "x${enable_prof}" = "x1" ] ; then + export MALLOC_CONF="prof:true,lg_prof_sample:0" +fi