From: Mark Andrews Date: Fri, 14 Mar 2025 04:23:43 +0000 (+1100) Subject: Use reference counted counters for nfail and nvalidations X-Git-Tag: v9.21.7~40^2 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=bfbaacc9a0466395df6dafd2ddddfd9a53698187;p=thirdparty%2Fbind9.git Use reference counted counters for nfail and nvalidations The fetch context that held these values could be freed while there were still active pointers to the memory. Using a reference counted pointer avoids this. --- diff --git a/lib/dns/include/dns/validator.h b/lib/dns/include/dns/validator.h index 5ee6206b736..51440b3caba 100644 --- a/lib/dns/include/dns/validator.h +++ b/lib/dns/include/dns/validator.h @@ -151,8 +151,8 @@ struct dns_validator { uint8_t unsupported_digest; dns_rdata_t rdata; bool resume; - uint32_t *nvalidations; - uint32_t *nfails; + isc_counter_t *nvalidations; + isc_counter_t *nfails; isc_counter_t *qc; isc_counter_t *gqc; @@ -172,7 +172,7 @@ dns_validator_create(dns_view_t *view, dns_name_t *name, dns_rdatatype_t type, dns_rdataset_t *rdataset, dns_rdataset_t *sigrdataset, dns_message_t *message, unsigned int options, isc_loop_t *loop, isc_job_cb cb, void *arg, - uint32_t *nvalidations, uint32_t *nfails, + isc_counter_t *nvalidations, isc_counter_t *nfails, isc_counter_t *qc, isc_counter_t *gqc, dns_edectx_t *edectx, dns_validator_t **validatorp); /*%< diff --git a/lib/dns/resolver.c b/lib/dns/resolver.c index 504fc0254fc..410141f0526 100644 --- a/lib/dns/resolver.c +++ b/lib/dns/resolver.c @@ -471,8 +471,8 @@ struct fetchctx { unsigned int depth; char clientstr[ISC_SOCKADDR_FORMATSIZE]; - uint32_t nvalidations; - uint32_t nfails; + isc_counter_t *nvalidations; + isc_counter_t *nfails; }; #define FCTX_MAGIC ISC_MAGIC('F', '!', '!', '!') @@ -973,8 +973,8 @@ valcreate(fetchctx_t *fctx, dns_message_t *message, dns_adbaddrinfo_t *addrinfo, result = dns_validator_create( fctx->res->view, name, type, rdataset, sigrdataset, message, - valoptions, fctx->loop, validated, valarg, &fctx->nvalidations, - &fctx->nfails, fctx->qc, fctx->gqc, &fctx->edectx, &validator); + valoptions, fctx->loop, validated, valarg, fctx->nvalidations, + fctx->nfails, fctx->qc, fctx->gqc, &fctx->edectx, &validator); RUNTIME_CHECK(result == ISC_R_SUCCESS); inc_stats(fctx->res, dns_resstatscounter_val); ISC_LIST_APPEND(fctx->validators, validator, link); @@ -4500,6 +4500,12 @@ fctx_destroy(fetchctx_t *fctx) { isc_mem_put(fctx->mctx, sa, sizeof(*sa)); } + if (fctx->nfails != NULL) { + isc_counter_detach(&fctx->nfails); + } + if (fctx->nvalidations != NULL) { + isc_counter_detach(&fctx->nvalidations); + } isc_counter_detach(&fctx->qc); if (fctx->gqc != NULL) { isc_counter_detach(&fctx->gqc); @@ -4669,6 +4675,8 @@ fctx_create(dns_resolver_t *res, isc_loop_t *loop, const dns_name_t *name, char buf[DNS_NAME_FORMATSIZE + DNS_RDATATYPE_FORMATSIZE + 1]; isc_mem_t *mctx = isc_loop_getmctx(loop); size_t p; + uint32_t nvalidations = atomic_load_relaxed(&res->maxvalidations); + uint32_t nfails = atomic_load_relaxed(&res->maxvalidationfails); /* * Caller must be holding the lock for 'bucket' @@ -4687,8 +4695,6 @@ fctx_create(dns_resolver_t *res, isc_loop_t *loop, const dns_name_t *name, .fwdpolicy = dns_fwdpolicy_none, .result = ISC_R_FAILURE, .loop = loop, - .nvalidations = atomic_load_relaxed(&res->maxvalidations), - .nfails = atomic_load_relaxed(&res->maxvalidationfails), }; isc_mem_attach(mctx, &fctx->mctx); @@ -4710,6 +4716,14 @@ fctx_create(dns_resolver_t *res, isc_loop_t *loop, const dns_name_t *name, FCTXTRACE("create"); + if (nfails > 0) { + isc_counter_create(mctx, nfails, &fctx->nfails); + } + + if (nvalidations > 0) { + isc_counter_create(mctx, nvalidations, &fctx->nvalidations); + } + if (qc != NULL) { isc_counter_attach(qc, &fctx->qc); isc_log_write(DNS_LOGCATEGORY_RESOLVER, DNS_LOGMODULE_RESOLVER, @@ -4942,6 +4956,12 @@ cleanup_nameservers: dns_rdataset_disassociate(&fctx->nameservers); } isc_mem_free(fctx->mctx, fctx->info); + if (fctx->nfails != NULL) { + isc_counter_detach(&fctx->nfails); + } + if (fctx->nvalidations != NULL) { + isc_counter_detach(&fctx->nvalidations); + } isc_counter_detach(&fctx->qc); if (fctx->gqc != NULL) { isc_counter_detach(&fctx->gqc); diff --git a/lib/dns/validator.c b/lib/dns/validator.c index a9ea0454639..770a549ad0a 100644 --- a/lib/dns/validator.c +++ b/lib/dns/validator.c @@ -1245,7 +1245,10 @@ compute_keytag(dns_rdata_t *rdata) { static bool over_max_validations(dns_validator_t *val) { - if (val->nvalidations == NULL || (*val->nvalidations) > 0) { + if (val->nvalidations == NULL || + isc_counter_used(val->nvalidations) < + isc_counter_getlimit(val->nvalidations)) + { return false; } @@ -1259,14 +1262,14 @@ consume_validation(dns_validator_t *val) { if (val->nvalidations == NULL) { return; } - INSIST((*val->nvalidations) > 0); - - (*val->nvalidations)--; + (void)isc_counter_increment(val->nvalidations); } static bool over_max_fails(dns_validator_t *val) { - if (val->nfails == NULL || (*val->nfails) > 0) { + if (val->nfails == NULL || + isc_counter_used(val->nfails) < isc_counter_getlimit(val->nfails)) + { return false; } @@ -1280,9 +1283,7 @@ consume_validation_fail(dns_validator_t *val) { if (val->nfails == NULL) { return; } - INSIST((*val->nfails) > 0); - - (*val->nfails)--; + (void)isc_counter_increment(val->nfails); } /*% @@ -3392,7 +3393,7 @@ dns_validator_create(dns_view_t *view, dns_name_t *name, dns_rdatatype_t type, dns_rdataset_t *rdataset, dns_rdataset_t *sigrdataset, dns_message_t *message, unsigned int options, isc_loop_t *loop, isc_job_cb cb, void *arg, - uint32_t *nvalidations, uint32_t *nfails, + isc_counter_t *nvalidations, isc_counter_t *nfails, isc_counter_t *qc, isc_counter_t *gqc, dns_edectx_t *edectx, dns_validator_t **validatorp) { isc_result_t result = ISC_R_FAILURE; @@ -3424,8 +3425,6 @@ dns_validator_create(dns_view_t *view, dns_name_t *name, dns_rdatatype_t type, .cb = cb, .arg = arg, .rdata = DNS_RDATA_INIT, - .nvalidations = nvalidations, - .nfails = nfails, .edectx = edectx, }; @@ -3435,6 +3434,14 @@ dns_validator_create(dns_view_t *view, dns_name_t *name, dns_rdatatype_t type, dns_message_attach(message, &val->message); } + if (nfails != NULL) { + isc_counter_attach(nfails, &val->nfails); + } + + if (nvalidations != NULL) { + isc_counter_attach(nvalidations, &val->nvalidations); + } + if (qc != NULL) { isc_counter_attach(qc, &val->qc); } @@ -3527,6 +3534,12 @@ destroy_validator(dns_validator_t *val) { if (val->message != NULL) { dns_message_detach(&val->message); } + if (val->nfails != NULL) { + isc_counter_detach(&val->nfails); + } + if (val->nvalidations != NULL) { + isc_counter_detach(&val->nvalidations); + } if (val->qc != NULL) { isc_counter_detach(&val->qc); }