diff --git a/CHANGES b/CHANGES index ddf889645e..bd72ded702 100644 --- a/CHANGES +++ b/CHANGES @@ -1,3 +1,10 @@ +6367. [bug] 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. Implement an expected + behavior of the function to release a name associated + with the validator. [GL #4654] + 6366. [bug] An assertion could be triggered in the QPDB cache when encountering a delegation below a DNAME. [GL #4652] diff --git a/lib/dns/include/dns/validator.h b/lib/dns/include/dns/validator.h index 0b0222c5c6..c68c5555b9 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 c30cea1724..6fe8ba448b 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 5681c89a36..2ba8ce77d0 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); } @@ -1638,11 +1642,7 @@ validate_answer_finish(void *arg) { dns_validator_t *val = arg; isc_result_t result = ISC_R_UNSET; - if (val->result != ISC_R_SUCCESS) { - validator_log(val, ISC_LOG_DEBUG(3), - "failed to verify rdataset: %s", - isc_result_totext(val->result)); - } else { + if (val->result == ISC_R_SUCCESS) { dns_rdataset_trimttl(val->rdataset, val->sigrdataset, val->siginfo, val->start, val->view->acceptexpired); @@ -1659,7 +1659,6 @@ validate_answer_finish(void *arg) { switch (val->result) { case ISC_R_CANCELED: - /* The validation was canceled */ validator_log(val, ISC_LOG_DEBUG(3), "validation was canceled"); validate_async_done(val, val->result); return; @@ -3432,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