From: Evan Hunt <each@isc.org>
Date: Wed, 20 Apr 2022 02:14:49 +0000 (-0700)
Subject: lock find when unlinking adbname->finds in dns_adb_cancelfind()
X-Git-Tag: v9.19.1~49^2
X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=a1e9a59e2b4cef2ed2169b5fb01687ab70d70d25;p=thirdparty%2Fbind9.git

lock find when unlinking adbname->finds in dns_adb_cancelfind()

In dns_adb_cancelfind(), we need to release the find lock and
then acquire the bucket and find locks in that order, for
consistency with locking hierarchy elsehwere. Previously we
were only acquiring the bucket lock.

Also rewrote the function for better readability.
---

diff --git a/lib/dns/adb.c b/lib/dns/adb.c
index dd311ee495b..4a769f36351 100644
--- a/lib/dns/adb.c
+++ b/lib/dns/adb.c
@@ -2662,54 +2662,66 @@ dns_adb_destroyfind(dns_adbfind_t **findp) {
 	free_adbfind(&find);
 }
 
+/*
+ * Caller must hold find lock.
+ */
+static void
+find_sendevent(dns_adbfind_t *find) {
+	if (!FIND_EVENTSENT(find)) {
+		isc_event_t *ev = &find->event;
+		isc_task_t *task = ev->ev_sender;
+
+		ev->ev_sender = find;
+		ev->ev_type = DNS_EVENT_ADBCANCELED;
+		ev->ev_destroy = event_freefind;
+		ev->ev_destroy_arg = find;
+		find->result_v4 = ISC_R_CANCELED;
+		find->result_v6 = ISC_R_CANCELED;
+
+		DP(DEF_LEVEL, "sending event %p to task %p for find %p", ev,
+		   task, find);
+
+		isc_task_sendanddetach(&task, (isc_event_t **)&ev);
+	}
+}
+
 void
 dns_adb_cancelfind(dns_adbfind_t *find) {
-	isc_event_t *ev = NULL;
-	isc_task_t *task = NULL;
-	dns_adb_t *adb = NULL;
 	dns_adbname_t *adbname = NULL;
-	dns_adbnamebucket_t *nbucket = NULL;
-
-	LOCK(&find->lock);
 
 	DP(DEF_LEVEL, "dns_adb_cancelfind on find %p", find);
 
-	adb = find->adb;
-	REQUIRE(DNS_ADB_VALID(adb));
+	REQUIRE(DNS_ADBFIND_VALID(find));
+	REQUIRE(DNS_ADB_VALID(find->adb));
 
+	LOCK(&find->lock);
 	REQUIRE(!FIND_EVENTFREED(find));
 	REQUIRE(FIND_WANTEVENT(find));
 
-	if (find->adbname == NULL) {
-		goto cleanup;
-	}
 	adbname = find->adbname;
-	find->adbname = NULL;
-	nbucket = adbname->bucket;
 
-	UNLOCK(&find->lock);
-	LOCK(&nbucket->lock);
-	ISC_LIST_UNLINK(adbname->finds, find, plink);
-	UNLOCK(&nbucket->lock);
-	LOCK(&find->lock);
-
-cleanup:
-	if (!FIND_EVENTSENT(find)) {
-		ev = &find->event;
-		task = ev->ev_sender;
-		ev->ev_sender = find;
-		ev->ev_type = DNS_EVENT_ADBCANCELED;
-		ev->ev_destroy = event_freefind;
-		ev->ev_destroy_arg = find;
-		find->result_v4 = ISC_R_CANCELED;
-		find->result_v6 = ISC_R_CANCELED;
+	if (adbname == NULL) {
+		find_sendevent(find);
+		UNLOCK(&find->lock);
+	} else {
+		/*
+		 * Release the find lock, then acquire the bucket and find
+		 * locks in that order, to match locking hierarchy
+		 * elsewhere.
+		 */
+		UNLOCK(&find->lock);
+		LOCK(&adbname->bucket->lock);
+		LOCK(&find->lock);
 
-		DP(DEF_LEVEL, "sending event %p to task %p for find %p", ev,
-		   task, find);
+		if (find->adbname != NULL) {
+			ISC_LIST_UNLINK(adbname->finds, find, plink);
+			find->adbname = NULL;
+		}
+		find_sendevent(find);
 
-		isc_task_sendanddetach(&task, (isc_event_t **)&ev);
+		UNLOCK(&find->lock);
+		UNLOCK(&adbname->bucket->lock);
 	}
-	UNLOCK(&find->lock);
 }
 
 void