bind/backport-0033-Lock-the-trampoline-when-attaching.patch

From be7f672fcc20a03d08c1f50b5b7a9f353ccd5dac Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Ond=C5=99ej=20Sur=C3=BD?= <ondrej@isc.org>
Date: Wed, 4 May 2022 09:26:34 +0200
Subject: [PATCH] Lock the trampoline when attaching

When attaching to the trampoline, the isc__trampoline_max was access
unlocked.  This would not manifest under normal circumstances because we
initialize 65 trampolines by default and that's enough for most
commodity hardware, but there are ARM machines with 128+ cores where
this would be reported by ThreadSanitizer.

Add locking around the code in isc__trampoline_attach().  This also
requires the lock to leak on exit (along with memory that we already)
because a new thread might be attaching to the trampoline while we are
running the library destructor at the same time.

(cherry picked from commit 933162ae1400ed4d854c32613a7e0a6bbe0b31f7)
Conflict: NA
Reference: https://gitlab.isc.org/isc-projects/bind9/-/commit/be7f672fcc20a03d08c1f50b5b7a9f353ccd5dac
---
 lib/isc/trampoline.c | 87 ++++++++++++++++++++------------------------
 1 file changed, 39 insertions(+), 48 deletions(-)

diff --git a/lib/isc/trampoline.c b/lib/isc/trampoline.c
index e133c40084..965fd0552e 100644
--- a/lib/isc/trampoline.c
+++ b/lib/isc/trampoline.c
@@ -15,9 +15,9 @@
 
 #include <inttypes.h>
 #include <stdlib.h>
+#include <uv.h>
 
 #include <isc/mem.h>
-#include <isc/mutex.h>
 #include <isc/once.h>
 #include <isc/thread.h>
 #include <isc/util.h>
@@ -34,9 +34,26 @@ struct isc__trampoline {
 	void *jemalloc_enforce_init;
 };
 
-static isc_once_t isc__trampoline_initialize_once = ISC_ONCE_INIT;
-static isc_once_t isc__trampoline_shutdown_once = ISC_ONCE_INIT;
-static isc_mutex_t isc__trampoline_lock;
+/*
+ * We can't use isc_mem API here, because it's called too
+ * early and when the isc_mem_debugging flags are changed
+ * later and ISC_MEM_DEBUGSIZE or ISC_MEM_DEBUGCTX flags are
+ * added, neither isc_mem_put() nor isc_mem_free() can be used
+ * to free up the memory allocated here because the flags were
+ * not set when calling isc_mem_get() or isc_mem_allocate()
+ * here.
+ *
+ * Since this is a single allocation at library load and deallocation at library
+ * unload, using the standard allocator without the tracking is fine for this
+ * single purpose.
+ *
+ * We can't use isc_mutex API either, because we track whether the mutexes get
+ * properly destroyed, and we intentionally leak the static mutex here without
+ * destroying it to prevent data race between library destructor running while
+ * thread is being still created.
+ */
+
+static uv_mutex_t isc__trampoline_lock;
 static isc__trampoline_t **trampolines;
 #if defined(HAVE_THREAD_LOCAL)
 #include <threads.h>
@@ -49,19 +66,6 @@ __declspec(thread) size_t isc_tid_v = SIZE_MAX;
 static size_t isc__trampoline_min = 1;
 static size_t isc__trampoline_max = 65;
 
-/*
- * We can't use isc_mem API here, because it's called too
- * early and when the isc_mem_debugging flags are changed
- * later and ISC_MEM_DEBUGSIZE or ISC_MEM_DEBUGCTX flags are
- * added, neither isc_mem_put() nor isc_mem_free() can be used
- * to free up the memory allocated here because the flags were
- * not set when calling isc_mem_get() or isc_mem_allocate()
- * here.
- *
- * Actually, since this is a single allocation at library load
- * and deallocation at library unload, using the standard
- * allocator without the tracking is fine for this purpose.
- */
 static isc__trampoline_t *
 isc__trampoline_new(int tid, isc_threadfunc_t start, isc_threadarg_t arg) {
 	isc__trampoline_t *trampoline = calloc(1, sizeof(*trampoline));
@@ -77,17 +81,17 @@ isc__trampoline_new(int tid, isc_threadfunc_t start, isc_threadarg_t arg) {
 	return (trampoline);
 }
 
-static void
-trampoline_initialize(void) {
-	isc_mutex_init(&isc__trampoline_lock);
+void
+isc__trampoline_initialize(void) {
+	uv_mutex_init(&isc__trampoline_lock);
 
 	trampolines = calloc(isc__trampoline_max, sizeof(trampolines[0]));
 	RUNTIME_CHECK(trampolines != NULL);
 
 	/* Get the trampoline slot 0 for the main thread */
 	trampolines[0] = isc__trampoline_new(0, NULL, NULL);
-	trampolines[0]->self = isc_thread_self();
 	isc_tid_v = trampolines[0]->tid;
+	trampolines[0]->self = isc_thread_self();
 
 	/* Initialize the other trampolines */
 	for (size_t i = 1; i < isc__trampoline_max; i++) {
@@ -97,38 +101,22 @@ trampoline_initialize(void) {
 }
 
 void
-isc__trampoline_initialize(void) {
-	isc_result_t result = isc_once_do(&isc__trampoline_initialize_once,
-					  trampoline_initialize);
-	RUNTIME_CHECK(result == ISC_R_SUCCESS);
-}
-
-static void
-trampoline_shutdown(void) {
+isc__trampoline_shutdown(void) {
 	/*
 	 * When the program using the library exits abruptly and the library
 	 * gets unloaded, there might be some existing trampolines from unjoined
 	 * threads.  We intentionally ignore those and don't check whether all
-	 * trampolines have been cleared before exiting.
+	 * trampolines have been cleared before exiting, so we leak a little bit
+	 * of resources here, including the lock.
 	 */
 	free(trampolines[0]);
-	free(trampolines);
-	trampolines = NULL;
-	isc_mutex_destroy(&isc__trampoline_lock);
-}
-
-void
-isc__trampoline_shutdown(void) {
-	isc_result_t result = isc_once_do(&isc__trampoline_shutdown_once,
-					  trampoline_shutdown);
-	RUNTIME_CHECK(result == ISC_R_SUCCESS);
 }
 
 isc__trampoline_t *
 isc__trampoline_get(isc_threadfunc_t start, isc_threadarg_t arg) {
 	isc__trampoline_t **tmp = NULL;
 	isc__trampoline_t *trampoline = NULL;
-	LOCK(&isc__trampoline_lock);
+	uv_mutex_lock(&isc__trampoline_lock);
 again:
 	for (size_t i = isc__trampoline_min; i < isc__trampoline_max; i++) {
 		if (trampolines[i] == NULL) {
@@ -152,17 +140,17 @@ again:
 	goto again;
 done:
 	INSIST(trampoline != NULL);
-	UNLOCK(&isc__trampoline_lock);
+	uv_mutex_unlock(&isc__trampoline_lock);
 
 	return (trampoline);
 }
 
 void
 isc__trampoline_detach(isc__trampoline_t *trampoline) {
-	LOCK(&isc__trampoline_lock);
-	REQUIRE(trampoline->tid > 0 &&
-		(size_t)trampoline->tid < isc__trampoline_max);
+	uv_mutex_lock(&isc__trampoline_lock);
 	REQUIRE(trampoline->self == isc_thread_self());
+	REQUIRE(trampoline->tid > 0);
+	REQUIRE((size_t)trampoline->tid < isc__trampoline_max);
 	REQUIRE(trampolines[trampoline->tid] == trampoline);
 
 	trampolines[trampoline->tid] = NULL;
@@ -174,15 +162,17 @@ isc__trampoline_detach(isc__trampoline_t *trampoline) {
 	free(trampoline->jemalloc_enforce_init);
 	free(trampoline);
 
-	UNLOCK(&isc__trampoline_lock);
+	uv_mutex_unlock(&isc__trampoline_lock);
 	return;
 }
 
 void
 isc__trampoline_attach(isc__trampoline_t *trampoline) {
-	REQUIRE(trampoline->tid > 0 &&
-		(size_t)trampoline->tid < isc__trampoline_max);
+	uv_mutex_lock(&isc__trampoline_lock);
 	REQUIRE(trampoline->self == ISC__TRAMPOLINE_UNUSED);
+	REQUIRE(trampoline->tid > 0);
+	REQUIRE((size_t)trampoline->tid < isc__trampoline_max);
+	REQUIRE(trampolines[trampoline->tid] == trampoline);
 
 	/* Initialize the trampoline */
 	isc_tid_v = trampoline->tid;
@@ -196,6 +186,7 @@ isc__trampoline_attach(isc__trampoline_t *trampoline) {
 	 * malloc() + free() calls altogether, as it would foil the fix.
 	 */
 	trampoline->jemalloc_enforce_init = malloc(8);
+	uv_mutex_unlock(&isc__trampoline_lock);
 }
 
 isc_threadresult_t
-- 
2.23.0