From: Aram Sargsyan <aram@isc.org>
Date: Mon, 25 Mar 2024 14:35:20 +0000 (+0000)
Subject: Rename and fix dns_validator_destroy() to dns_validator_shutdown()
X-Git-Tag: v9.19.23~5^2~1
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=a5ea7bcd257f6f314d5ac34f49245526c4538371;p=thirdparty%2Fbind9.git

Rename and fix dns_validator_destroy() to dns_validator_shutdown()

Since the dns_validator_destroy() function doesn't guarantee that
it destroys the validator, rename it to dns_validator_shutdown()
and require explicit dns_validator_detach() to follow.

Enforce the documented function requirement that the validator must
be completed when the function is called.

Make sure to set val->name to NULL when the function is called,
so that the owner of the validator may destroy the name, even if
the validator is not destroyed immediately. This should be safe,
because the name can be used further only for logging by the
offloaded work callbacks when they detect that the validator is
already canceled/complete, and the logging function has a condition
to use the name only when it is non-NULL.
---

diff --git a/lib/dns/include/dns/validator.h b/lib/dns/include/dns/validator.h
index 0b0222c5c6d..c68c5555b99 100644
--- a/lib/dns/include/dns/validator.h
+++ b/lib/dns/include/dns/validator.h
@@ -235,17 +235,17 @@ dns_validator_cancel(dns_validator_t *validator);
  */
 
 void
-dns_validator_destroy(dns_validator_t **validatorp);
+dns_validator_shutdown(dns_validator_t *val);
 /*%<
- * Destroy a DNSSEC validator.
+ * Release the name associated with the DNSSEC validator.
  *
  * Requires:
- *\li	'*validatorp' points to a valid DNSSEC validator.
+ * \li	'val' points to a valid DNSSEC validator.
  * \li	The validator must have completed and sent its completion
  *	event.
  *
  * Ensures:
- *\li	All resources used by the validator are freed.
+ *\li	The name associated with the DNSSEC validator is released.
  */
 
 #if DNS_VALIDATOR_TRACE
diff --git a/lib/dns/resolver.c b/lib/dns/resolver.c
index c30cea17244..6fe8ba448bd 100644
--- a/lib/dns/resolver.c
+++ b/lib/dns/resolver.c
@@ -5557,10 +5557,11 @@ cleanup_fetchctx:
 
 	/*
 	 * val->name points to name on a message on one of the
-	 * queries on the fetch context so the validator has to
-	 * be destroyed first.
+	 * queries on the fetch context so the name has to be
+	 * released first with a dns_validator_shutdown() call.
 	 */
-	dns_validator_destroy(&val);
+	dns_validator_shutdown(val);
+	dns_validator_detach(&val);
 	fetchctx_detach(&fctx);
 	INSIST(node == NULL);
 }
diff --git a/lib/dns/validator.c b/lib/dns/validator.c
index d80c22b04cf..2ba8ce77d08 100644
--- a/lib/dns/validator.c
+++ b/lib/dns/validator.c
@@ -614,7 +614,8 @@ validator_callback_dnskey(void *arg) {
 
 cleanup:
 	dns_validator_detach(&subvalidator->parent);
-	dns_validator_destroy(&subvalidator);
+	dns_validator_shutdown(subvalidator);
+	dns_validator_detach(&subvalidator);
 	validate_async_done(val, result);
 }
 
@@ -672,7 +673,8 @@ validator_callback_ds(void *arg) {
 
 cleanup:
 	dns_validator_detach(&subvalidator->parent);
-	dns_validator_destroy(&subvalidator);
+	dns_validator_shutdown(subvalidator);
+	dns_validator_detach(&subvalidator);
 	validate_async_done(val, result);
 }
 
@@ -714,7 +716,8 @@ validator_callback_cname(void *arg) {
 
 cleanup:
 	dns_validator_detach(&subvalidator->parent);
-	dns_validator_destroy(&subvalidator);
+	dns_validator_shutdown(subvalidator);
+	dns_validator_detach(&subvalidator);
 	validate_async_done(val, result);
 }
 
@@ -813,7 +816,8 @@ validator_callback_nsec(void *arg) {
 
 cleanup:
 	dns_validator_detach(&subvalidator->parent);
-	dns_validator_destroy(&subvalidator);
+	dns_validator_shutdown(subvalidator);
+	dns_validator_detach(&subvalidator);
 	validate_async_done(val, result);
 }
 
@@ -3427,20 +3431,21 @@ destroy_validator(dns_validator_t *val) {
 }
 
 void
-dns_validator_destroy(dns_validator_t **validatorp) {
-	dns_validator_t *val = NULL;
-
-	REQUIRE(validatorp != NULL);
-
-	val = *validatorp;
-	*validatorp = NULL;
-
+dns_validator_shutdown(dns_validator_t *val) {
 	REQUIRE(VALID_VALIDATOR(val));
+	REQUIRE(COMPLETE(val));
 	REQUIRE(val->tid == isc_tid());
 
-	validator_log(val, ISC_LOG_DEBUG(4), "dns_validator_destroy");
+	validator_log(val, ISC_LOG_DEBUG(4), "dns_validator_shutdown");
 
-	dns_validator_detach(&val);
+	/*
+	 * The validation is now complete and the owner is no longer interested
+	 * in any further results. If there are still callback events queued up
+	 * which hold a validator reference, they should not be allowed to use
+	 * val->name during logging, because the owner may destroy it after this
+	 * function is called.
+	 */
+	val->name = NULL;
 }
 
 static void