From: Evan Hunt <each@isc.org>
Date: Mon, 4 May 2026 07:05:27 +0000 (-0700)
Subject: Hold a reference to the NTA table for the lifetime of each NTA
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=3db5d47ed4fd73ca515b224ef42d2a810ceab5ad;p=thirdparty%2Fbind9.git

Hold a reference to the NTA table for the lifetime of each NTA

Each dns__nta_t now references its parent ntatable in nta_create() and
releases it in dns__nta_destroy().  This avoids a use-after-free in
fetch_done() and other callbacks that dereference nta->ntatable: the
ntatable could otherwise be released by view destruction while an
in-flight resolver fetch still holds a reference to the NTA.

(cherry picked from commit 26c895cc9281d5cc36447d4a2c5464ad31137f76)
---

diff --git a/lib/dns/nta.c b/lib/dns/nta.c
index 83e9c38e010..fcfe8aca711 100644
--- a/lib/dns/nta.c
+++ b/lib/dns/nta.c
@@ -153,34 +153,11 @@ ISC_REFCOUNT_IMPL(dns_ntatable, dns__ntatable_destroy);
 #endif
 
 static void
-fetch_done(void *arg) {
-	dns_fetchresponse_t *resp = (dns_fetchresponse_t *)arg;
-	dns__nta_t *nta = resp->arg;
-	isc_result_t eresult = resp->result;
+dns__nta_update_expiry(dns__nta_t *nta, isc_result_t eresult) {
 	dns_ntatable_t *ntatable = nta->ntatable;
 	dns_view_t *view = ntatable->view;
 	isc_stdtime_t now = isc_stdtime_now();
 
-	if (dns_rdataset_isassociated(&nta->rdataset)) {
-		dns_rdataset_disassociate(&nta->rdataset);
-	}
-	if (dns_rdataset_isassociated(&nta->sigrdataset)) {
-		dns_rdataset_disassociate(&nta->sigrdataset);
-	}
-	if (nta->fetch == resp->fetch) {
-		nta->fetch = NULL;
-	}
-	dns_resolver_destroyfetch(&resp->fetch);
-
-	if (resp->node != NULL) {
-		dns_db_detachnode(resp->db, &resp->node);
-	}
-	if (resp->db != NULL) {
-		dns_db_detach(&resp->db);
-	}
-
-	dns_resolver_freefresp(&resp);
-
 	switch (eresult) {
 	case ISC_R_SUCCESS:
 	case DNS_R_NCACHENXDOMAIN:
@@ -206,6 +183,37 @@ fetch_done(void *arg) {
 		isc_timer_stop(nta->timer);
 	}
 	RWUNLOCK(&ntatable->rwlock, isc_rwlocktype_read);
+}
+
+static void
+fetch_done(void *arg) {
+	dns_fetchresponse_t *resp = (dns_fetchresponse_t *)arg;
+	dns__nta_t *nta = resp->arg;
+	isc_result_t eresult = resp->result;
+
+	if (dns_rdataset_isassociated(&nta->rdataset)) {
+		dns_rdataset_disassociate(&nta->rdataset);
+	}
+	if (dns_rdataset_isassociated(&nta->sigrdataset)) {
+		dns_rdataset_disassociate(&nta->sigrdataset);
+	}
+	if (nta->fetch == resp->fetch) {
+		nta->fetch = NULL;
+	}
+	dns_resolver_destroyfetch(&resp->fetch);
+
+	if (resp->node != NULL) {
+		dns_db_detachnode(resp->db, &resp->node);
+	}
+	if (resp->db != NULL) {
+		dns_db_detach(&resp->db);
+	}
+
+	dns_resolver_freefresp(&resp);
+
+	if (!nta->shuttingdown) {
+		dns__nta_update_expiry(nta, eresult);
+	}
 
 	dns__nta_detach(&nta); /* for dns_resolver_createfetch() */
 }
@@ -228,7 +236,7 @@ checkbogus(void *arg) {
 		dns_rdataset_disassociate(&nta->sigrdataset);
 	}
 
-	if (atomic_load(&ntatable->shuttingdown)) {
+	if (nta->shuttingdown || atomic_load_acquire(&ntatable->shuttingdown)) {
 		isc_timer_stop(nta->timer);
 		return;
 	}
@@ -278,7 +286,7 @@ nta_create(dns_ntatable_t *ntatable, const dns_name_t *name,
 
 	nta = isc_mem_get(ntatable->mctx, sizeof(dns__nta_t));
 	*nta = (dns__nta_t){
-		.ntatable = ntatable,
+		.ntatable = dns_ntatable_ref(ntatable),
 		.name = DNS_NAME_INITEMPTY,
 		.magic = NTA_MAGIC,
 	};
@@ -305,7 +313,7 @@ dns_ntatable_add(dns_ntatable_t *ntatable, const dns_name_t *name, bool force,
 
 	REQUIRE(VALID_NTATABLE(ntatable));
 
-	if (atomic_load(&ntatable->shuttingdown)) {
+	if (atomic_load_acquire(&ntatable->shuttingdown)) {
 		return ISC_R_SUCCESS;
 	}
 
@@ -366,14 +374,23 @@ dns_ntatable_delete(dns_ntatable_t *ntatable, const dns_name_t *name) {
 	return result;
 }
 
+typedef struct dns__nta_async_data {
+	dns__nta_t *nta;
+	dns_ntatable_t *ntatable;
+} dns__nta_async_data_t;
+
 static void
 delete_expired(void *arg) {
-	dns__nta_t *nta = arg;
-	dns_ntatable_t *ntatable = nta->ntatable;
+	dns__nta_async_data_t *data = arg;
+	dns__nta_t *nta = data->nta;
+	dns_ntatable_t *ntatable = data->ntatable;
 	isc_result_t result;
 	dns_qp_t *qp = NULL;
 	void *pval = NULL;
 
+	isc_mem_put(nta->mctx, data, sizeof(*data));
+
+	REQUIRE(VALID_NTA(nta));
 	REQUIRE(VALID_NTATABLE(ntatable));
 
 	RWLOCK(&ntatable->rwlock, isc_rwlocktype_write);
@@ -435,9 +452,13 @@ dns_ntatable_covered(dns_ntatable_t *ntatable, isc_stdtime_t now,
 
 	if (nta->expiry <= now) {
 		/* NTA is expired */
-		dns__nta_ref(nta);
-		dns_ntatable_ref(nta->ntatable);
-		isc_async_current(delete_expired, nta);
+		dns__nta_async_data_t *data = isc_mem_get(nta->mctx,
+							  sizeof(*data));
+		*data = (dns__nta_async_data_t){
+			.nta = dns__nta_ref(nta),
+			.ntatable = dns_ntatable_ref(nta->ntatable),
+		};
+		isc_async_current(delete_expired, data);
 		goto done;
 	}
 
@@ -594,6 +615,7 @@ dns__nta_shutdown_cb(void *arg) {
 		isc_timer_destroy(&nta->timer);
 	}
 
+	dns_ntatable_detach(&nta->ntatable);
 	dns__nta_detach(&nta);
 }
 
@@ -616,7 +638,7 @@ dns_ntatable_shutdown(dns_ntatable_t *ntatable) {
 
 	RWLOCK(&ntatable->rwlock, isc_rwlocktype_write);
 	dns_qpmulti_query(ntatable->table, &qpr);
-	ntatable->shuttingdown = true;
+	atomic_store_release(&ntatable->shuttingdown, true);
 
 	dns_qpiter_init(&qpr, &iter);
 	while (dns_qpiter_next(&iter, NULL, &pval, NULL) == ISC_R_SUCCESS) {