From: Ondřej Surý <ondrej@isc.org>
Date: Wed, 29 Sep 2021 08:58:58 +0000 (+0200)
Subject: Use assertions to check for failed allocations
X-Git-Tag: v9.17.19~19^2
X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=c3250a9b819403e6297518d11dafd8d56af449f1;p=thirdparty%2Fbind9.git

Use assertions to check for failed allocations

It was discovered that named could crash due to a segmentation fault
when jemalloc was in use and memory allocation failed.  This was not
intended to happen as jemalloc's "xmalloc" option was set to "true" in
the "malloc_conf" configuration variable.  However, that variable was
only set after jemalloc was already done with parsing it, which
effectively caused setting that variable to have no effect.

While investigating this issue, it was also discovered that enabling the
"xmalloc" option makes jemalloc use a slow processing path, decreasing
its performance by about 25%. [1]

Additionally, further testing (carried out after fixing the way
"malloc_conf" was set) revealed that the non-default configuration
options do not have any measurable effect on either authoritative or
recursive DNS server performance.

Replace code setting various jemalloc options to non-default values with
assertion checks of mallocx()/rallocx() return values.

[1] https://github.com/jemalloc/jemalloc/pull/523
---

diff --git a/lib/isc/jemalloc_shim.h b/lib/isc/jemalloc_shim.h
index 95dbf0dc153..b207eabc890 100644
--- a/lib/isc/jemalloc_shim.h
+++ b/lib/isc/jemalloc_shim.h
@@ -26,9 +26,8 @@ const char *malloc_conf = NULL;
 static inline void *
 mallocx(size_t size, int flags) {
 	UNUSED(flags);
-	void *__ptr = malloc(size);
-	REQUIRE(__ptr != NULL);
-	return (__ptr);
+
+	return (malloc(size));
 }
 
 static inline void
@@ -44,10 +43,7 @@ rallocx(void *ptr, size_t size, int flags) {
 	UNUSED(flags);
 	REQUIRE(size != 0);
 
-	void *__ptr = realloc(ptr, size);
-	REQUIRE(__ptr != NULL);
-
-	return (__ptr);
+	return (realloc(ptr, size));
 }
 
 #ifdef HAVE_MALLOC_SIZE
@@ -90,7 +86,7 @@ mallocx(size_t size, int flags) {
 	UNUSED(flags);
 
 	size_info *si = malloc(size + sizeof(*si));
-	REQUIRE(si != NULL);
+	INSIST(si != NULL);
 
 	si->size = size;
 	ptr = &si[1];
@@ -124,7 +120,7 @@ rallocx(void *ptr, size_t size, int flags) {
 	UNUSED(flags);
 
 	si = realloc(si, size + sizeof(*si));
-	REQUIRE(si != NULL);
+	INSIST(si != NULL);
 
 	si->size = size;
 	ptr = &si[1];
diff --git a/lib/isc/mem.c b/lib/isc/mem.c
index 67235bf41fe..a75524ece39 100644
--- a/lib/isc/mem.c
+++ b/lib/isc/mem.c
@@ -339,6 +339,7 @@ mem_get(isc_mem_t *ctx, size_t size) {
 	ADJUST_ZERO_ALLOCATION_SIZE(size);
 
 	ret = mallocx(size, 0);
+	INSIST(ret != NULL);
 
 	if (ISC_UNLIKELY((ctx->flags & ISC_MEMFLAG_FILL) != 0)) {
 		memset(ret, 0xbe, size); /* Mnemonic for "beef". */
@@ -368,6 +369,7 @@ mem_realloc(isc_mem_t *ctx, void *old_ptr, size_t old_size, size_t new_size) {
 	ADJUST_ZERO_ALLOCATION_SIZE(new_size);
 
 	new_ptr = rallocx(old_ptr, new_size, 0);
+	INSIST(new_ptr != NULL);
 
 	if (ISC_UNLIKELY((ctx->flags & ISC_MEMFLAG_FILL) != 0)) {
 		ssize_t diff_size = new_size - old_size;
@@ -424,9 +426,6 @@ mem_putstats(isc_mem_t *ctx, void *ptr, size_t size) {
 
 static void
 mem_initialize(void) {
-	malloc_conf = "xmalloc:true,background_thread:true,metadata_thp:auto,"
-		      "dirty_decay_ms:30000,muzzy_decay_ms:30000";
-
 	isc_mutex_init(&contextslock);
 	ISC_LIST_INIT(contexts);
 	totallost = 0;
@@ -456,6 +455,7 @@ mem_create(isc_mem_t **ctxp, unsigned int flags) {
 	REQUIRE(ctxp != NULL && *ctxp == NULL);
 
 	ctx = mallocx(sizeof(*ctx), 0);
+	INSIST(ctx != NULL);
 
 	*ctx = (isc_mem_t){
 		.magic = MEM_MAGIC,
@@ -488,6 +488,8 @@ mem_create(isc_mem_t **ctxp, unsigned int flags) {
 
 		ctx->debuglist =
 			mallocx((DEBUG_TABLE_COUNT * sizeof(debuglist_t)), 0);
+		INSIST(ctx->debuglist != NULL);
+
 		for (i = 0; i < DEBUG_TABLE_COUNT; i++) {
 			ISC_LIST_INIT(ctx->debuglist[i]);
 		}