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.
This commit is contained in:
Mark Andrews 2025-03-14 15:23:43 +11:00
parent 8dba96d71e
commit bfbaacc9a0
3 changed files with 53 additions and 20 deletions

View file

@ -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);
/*%<

View file

@ -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);

View file

@ -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);
}