Realloc: Make behavior of realloc(ptr, 0) configurable.

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

doc/jemalloc.xml.in

@@ -1489,6 +1489,33 @@ malloc_conf = "xmalloc:true";]]></programlisting>
         by default.</para></listitem>
       </varlistentry>
 
+      <varlistentry id="opt.zero_realloc">
+        <term>
+          <mallctl>opt.zero_realloc</mallctl>
+          (<type>const char *</type>)
+          <literal>r-</literal>
+        </term>
+        <listitem><para> Determines the behavior of
+	<function>realloc()</function> when passed a value of zero for the new
+	size.  <quote>strict</quote> treats this as an allocation of size zero
+	(and returns a non-null result except in case of resource exhaustion).
+	<quote>free</quote> treats this as a deallocation of the pointer, and
+	returns <constant>NULL</constant> without setting
+	<varname>errno</varname>.  <quote>abort</quote> aborts the process if
+	zero is passed.  The default is <quote>strict</quote>.</para>
+
+	<para>There is considerable divergence of behaviors across
+	implementations in handling this case. Many have the behavior of
+	<quote>free</quote>. This can introduce security vulnerabilities, since
+	a <constant>NULL</constant> return value indicates failure, and the
+	continued validity of the passed-in pointer (per POSIX and C11).
+	<quote>strict</quote> 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 <quote>abort</quote> option enables these
+	testing this behavior.</para></listitem>
+      </varlistentry>
+
       <varlistentry id="thread.arena">
         <term>
           <mallctl>thread.arena</mallctl>

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;

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:
  *

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 *)
 
 /******************************************************************************/
 

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
+static void *
-void JEMALLOC_NOTHROW *
+do_realloc_nonnull_zero(void *ptr) {
-JEMALLOC_ALLOC_SIZE(2)
+	if (opt_zero_realloc_action == zero_realloc_action_strict) {
-je_realloc(void *ptr, size_t size) {
+		/*
-	LOG("core.realloc.entry", "ptr: %p, size: %zu\n", ptr, size);
+		 * The user might have gotten a strict setting while expecting a
-
+		 * free setting.  If that's the case, we at least try to
-	if (likely(ptr != NULL && size != 0)) {
+		 * reduce the harm, and turn off the tcache while allocating, so
-		void *ret = do_rallocx(ptr, size, 0, true);
+		 * that we'll get a true first fit.
-		LOG("core.realloc.exit", "result: %p", ret);
+		 */
-		return ret;
+		return do_rallocx(ptr, 1, MALLOCX_TCACHE_NONE, true);
-	} else if (ptr != NULL && size == 0) {
+	} else if (opt_zero_realloc_action == zero_realloc_action_free) {
-		/* realloc(ptr, 0) is equivalent to free(ptr). */
 		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;

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

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);
+	if (opt_zero_realloc_action == zero_realloc_action_free) {
-	reset();
+		ptr = malloc(1);
-	realloc(ptr, 0);
+		reset();
-	assert_d_eq(call_count, 1, "Hook not called");
+		realloc(ptr, 0);
-	assert_ptr_eq(arg_extra, (void *)123, "Wrong extra");
+		assert_d_eq(call_count, 1, "Hook not called");
-	assert_d_eq(arg_type, (int)hook_dalloc_realloc, "Wrong hook type");
+		assert_ptr_eq(arg_extra, (void *)123, "Wrong extra");
-	assert_ptr_eq(ptr, arg_address, "Wrong pointer freed");
+		assert_d_eq(arg_type, (int)hook_dalloc_realloc,
-	assert_u64_eq((uintptr_t)ptr, arg_args_raw[0], "Wrong raw arg");
+		    "Wrong hook type");
-	assert_u64_eq((uintptr_t)0, arg_args_raw[1], "Wrong raw arg");
+		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();

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);
 }

test/unit/zero_realloc_abort.c

@@ -0,0 +1,26 @@
+#include "test/jemalloc_test.h"
+
+#include <signal.h>
+
+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);
+}
+

test/unit/zero_realloc_abort.sh

@@ -0,0 +1,3 @@
+#!/bin/sh
+
+export MALLOC_CONF="zero_realloc:abort"

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);
+}

test/unit/zero_realloc_free.sh

@@ -0,0 +1,3 @@
+#!/bin/sh
+
+export MALLOC_CONF="zero_realloc:free"

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);
+}

test/unit/zero_realloc_strict.sh

@@ -0,0 +1,3 @@
+#!/bin/sh
+
+export MALLOC_CONF="zero_realloc:strict"