From c2e43582678ec5d0c40e19c671d60012b36ac312 Mon Sep 17 00:00:00 2001
From: Evan Hunt <each@isc.org>
Date: Tue, 21 Jan 2025 17:57:00 -0800
Subject: [PATCH] prevent a reference leak from the ns_query_done hooks

if the NS_QUERY_DONE_BEGIN or NS_QUERY_DONE_SEND hook is
used in a plugin and returns NS_HOOK_RETURN, some of the
cleanup in ns_query_done() can be skipped over, leading
to reference leaks that can cause named to hang on shut
down.

this has been addressed by adding more housekeeping
code after the cleanup: tag in ns_query_done().
---
 lib/ns/include/ns/query.h |  1 +
 lib/ns/query.c            | 21 +++++++++++++++++----
 2 files changed, 18 insertions(+), 4 deletions(-)

diff --git a/lib/ns/include/ns/query.h b/lib/ns/include/ns/query.h
index b340c59e3a..c38561fa21 100644
--- a/lib/ns/include/ns/query.h
+++ b/lib/ns/include/ns/query.h
@@ -212,6 +212,7 @@ struct query_ctx {
 
 	ns_client_t *client;	    /* client object */
 	bool	     detach_client; /* client needs detaching */
+	bool	     async;	    /* asynchronous hook running */
 
 	dns_fetchresponse_t *fresp; /* recursion response */
 
diff --git a/lib/ns/query.c b/lib/ns/query.c
index 8382b8ede3..35193b9c17 100644
--- a/lib/ns/query.c
+++ b/lib/ns/query.c
@@ -5070,6 +5070,9 @@ qctx_clean(query_ctx_t *qctx) {
 	if (qctx->db != NULL && qctx->node != NULL) {
 		dns_db_detachnode(qctx->db, &qctx->node);
 	}
+	if (qctx->client != NULL && qctx->client->query.gluedb != NULL) {
+		dns_db_detach(&qctx->client->query.gluedb);
+	}
 }
 
 /*%
@@ -6728,6 +6731,9 @@ ns_query_hookasync(query_ctx_t *qctx, ns_query_starthookasync_t runasync,
 		goto cleanup_and_detach_from_quota;
 	}
 
+	/* Record that an asynchronous copy of the qctx has been started */
+	qctx->async = true;
+
 	/*
 	 * Typically the runasync() function will trigger recursion, but
 	 * there is no need to set NS_QUERYATTR_RECURSING. The calling hook
@@ -11372,10 +11378,6 @@ ns_query_done(query_ctx_t *qctx) {
 	qctx_clean(qctx);
 	qctx_freedata(qctx);
 
-	if (qctx->client->query.gluedb != NULL) {
-		dns_db_detach(&qctx->client->query.gluedb);
-	}
-
 	/*
 	 * Clear the AA bit if we're not authoritative.
 	 */
@@ -11513,6 +11515,17 @@ ns_query_done(query_ctx_t *qctx) {
 	return qctx->result;
 
 cleanup:
+	/*
+	 * We'd only get here if one of the hooks above
+	 * (NS_QUERY_DONE_BEGIN or NS_QUERY_DONE_SEND) returned
+	 * NS_HOOK_RETURN. Some housekeeping may be needed.
+	 */
+	qctx_clean(qctx);
+	qctx_freedata(qctx);
+	if (!qctx->async) {
+		qctx->detach_client = true;
+		query_error(qctx->client, DNS_R_SERVFAIL, __LINE__);
+	}
 	return result;
 }