From: Ondřej Surý <ondrej@isc.org>
Date: Fri, 12 Jun 2026 15:37:16 +0000 (+0200)
Subject: Cancel the offloaded verification job when canceling a validator
X-Git-Tag: v9.21.24~7^2
X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=6b3571dc79aa422b329e3bd7a999816560448d56;p=thirdparty%2Fbind9.git

Cancel the offloaded verification job when canceling a validator

The offloaded jobs were fire-and-forget: dns_validator_cancel() could
only raise a flag and wait for the queued crypto to get its turn, so
under a random-subdomain attack canceled validations piled up in the
worker queues, pinning memory.  Keep the isc_work handle and cancel it:
a still-queued job never runs its crypto and unwinds on dequeue; a
running one is unaffected.
---

diff --git a/lib/dns/include/dns/validator.h b/lib/dns/include/dns/validator.h
index b7d0d8e712f..269b4f6ae6b 100644
--- a/lib/dns/include/dns/validator.h
+++ b/lib/dns/include/dns/validator.h
@@ -50,6 +50,7 @@
 
 #include <isc/job.h>
 #include <isc/refcount.h>
+#include <isc/work.h>
 
 #include <dns/fixedname.h>
 #include <dns/rdata.h>
@@ -124,6 +125,8 @@ struct dns_validator {
 	/* Internal validator state */
 	atomic_bool	   canceling;
 	unsigned int	   attributes;
+	isc_work_t	  *offloaded_work;
+	isc_job_cb	   offloaded_cb;
 	dns_fetch_t	  *fetch;
 	dns_validator_t	  *subvalidator;
 	dns_validator_t	  *parent;
diff --git a/lib/dns/validator.c b/lib/dns/validator.c
index 20ee80887dd..f36b5c89b62 100644
--- a/lib/dns/validator.c
+++ b/lib/dns/validator.c
@@ -415,6 +415,12 @@ resume_answer_with_key(void *arg) {
 	dns_validator_t *val = arg;
 	dns_rdataset_t *rdataset = &val->frdataset;
 
+	if (CANCELED(val) || CANCELING(val)) {
+		val->result = ISC_R_CANCELED;
+		(void)validate_async_run(val, resume_answer_with_key_done);
+		return;
+	}
+
 	isc_result_t result = select_signing_key(val, rdataset);
 	if (result == ISC_R_SUCCESS) {
 		val->keyset = &val->frdataset;
@@ -1990,14 +1996,49 @@ validate_async_run(dns_validator_t *val, isc_job_cb cb) {
 }
 
 static void
-null_done(void *arg ISC_ATTR_UNUSED, isc_result_t result ISC_ATTR_UNUSED) {
-	/* no-op for now */
+helper_done(void *arg, isc_result_t result) {
+	dns_validator_t *val = arg;
+
+	if (result == ISC_R_CANCELED) {
+		/*
+		 * The job was tombstoned by dns_validator_cancel(), which has
+		 * already scheduled helper_cancel() to unwind the validation on
+		 * the loop.  That unwind may have freed the validator, so 'val'
+		 * is now a dangling pointer and must not be dereferenced.
+		 */
+		val = NULL;
+	}
+
+	/*
+	 * Nothing to do on either path: on success the offloaded callback has
+	 * already run on the worker and scheduled its continuation, which owns
+	 * the validator from here; on cancel helper_cancel() owns the unwind.
+	 */
+
+	UNUSED(val);
+
+	return;
+}
+
+static void
+helper_cancel(void *arg) {
+	dns_validator_t *val = arg;
+	/*
+	 * The job was canceled while it was still queued, so the offloaded
+	 * callback never ran.  Run it here on the loop instead: it sees the
+	 * canceling flag, skips the crypto, and unwinds the validation the
+	 * same way it would have after waiting its turn in the work queue.
+	 */
+	val->offloaded_work = NULL;
+	val->offloaded_cb(val);
 }
 
 static isc_result_t
 validate_work_enqueue(dns_validator_t *val, isc_job_cb cb) {
 	val->attributes |= VALATTR_OFFLOADED;
-	isc_work_enqueue(val->loop, ISC_WORKLANE_FAST, cb, null_done, val);
+	val->offloaded_cb = cb;
+	val->offloaded_work = isc_work_enqueue(val->loop, ISC_WORKLANE_FAST, cb,
+					       helper_done, val);
 	return DNS_R_WAIT;
 }
 
@@ -3896,6 +3937,23 @@ dns_validator_cancel(dns_validator_t *validator) {
 
 	if (!OFFLOADED(validator)) {
 		validator_cancel_finish(validator);
+	} else if (validator->offloaded_work != NULL) {
+		/*
+		 * Try to drop the offloaded job before its crypto runs.  If it
+		 * is still queued, isc_work_cancel() tombstones it so the
+		 * worker discards it without running the callback, and we
+		 * schedule helper_cancel() to unwind the validation on the loop
+		 * right away -- reclaiming the pinned response now instead of
+		 * waiting for the tombstone to reach the head of the work
+		 * queue.  If the job is already running, isc_work_cancel()
+		 * returns false and we leave the unwind to the worker, which
+		 * notices the canceling flag, skips the crypto, and finishes
+		 * through its continuation.
+		 */
+		if (isc_work_cancel(validator->offloaded_work)) {
+			isc_async_run(validator->loop, helper_cancel,
+				      validator);
+		}
 	}
 }