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).

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"
 
 /*
@@ -428,6 +429,35 @@ cache_bin_full(cache_bin_t *bin) {
 }
 
 /*
+ * 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.
  */
 JEMALLOC_ALWAYS_INLINE bool
@@ -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
 }