refactor resume_dsfetch()

clean up resume_dsfetch() so that the fctx reference counting is
saner and easier to follow.

(cherry picked from commit 7b2ea97e46)
This commit is contained in:
Evan Hunt 2022-04-22 16:41:10 -07:00
parent 759e0b030e
commit 9296be2130

View file

@ -7298,20 +7298,18 @@ fctx__detach(fetchctx_t **fctxp, const char *file, unsigned int line,
static void
resume_dslookup(isc_task_t *task, isc_event_t *event) {
dns_fetchevent_t *fevent = NULL;
dns_resolver_t *res = NULL;
fetchctx_t *fctx = NULL, *ds_fctx = NULL;
isc_result_t result;
dns_fetchevent_t *fevent = (dns_fetchevent_t *)event;
fetchctx_t *fctx = event->ev_arg;
dns_resolver_t *res = NULL;
dns_rdataset_t *frdataset = NULL, *nsrdataset = NULL;
dns_rdataset_t nameservers;
dns_fixedname_t fixed;
dns_name_t *domain = NULL;
unsigned int n;
REQUIRE(event->ev_type == DNS_EVENT_FETCHDONE);
fevent = (dns_fetchevent_t *)event;
fctx = event->ev_arg;
ds_fctx = fctx;
REQUIRE(VALID_FCTX(fctx));
res = fctx->res;
@ -7325,52 +7323,57 @@ resume_dslookup(isc_task_t *task, isc_event_t *event) {
dns_db_detach(&fevent->db);
}
dns_rdataset_init(&nameservers);
/* Preserve data from fevent before freeing it. */
frdataset = fevent->rdataset;
result = fevent->result;
isc_event_free(&event);
/*
* Note: fevent->rdataset must be disassociated and
* isc_event_free(&event) be called before resuming
* processing of the 'fctx' to prevent use-after-free.
*/
if (fevent->result == ISC_R_CANCELED) {
if (dns_rdataset_isassociated(fevent->rdataset)) {
dns_rdataset_disassociate(fevent->rdataset);
LOCK(&res->buckets[fctx->bucketnum].lock);
if (SHUTTINGDOWN(fctx)) {
maybe_cancel_validators(fctx, true);
UNLOCK(&res->buckets[fctx->bucketnum].lock);
if (dns_rdataset_isassociated(frdataset)) {
dns_rdataset_disassociate(frdataset);
}
dns_resolver_destroyfetch(&fctx->nsfetch);
fctx_detach(&fctx);
return;
}
UNLOCK(&res->buckets[fctx->bucketnum].lock);
LOCK(&res->buckets[fctx->bucketnum].lock);
if (SHUTTINGDOWN(fctx)) {
maybe_cancel_validators(fctx, true);
UNLOCK(&res->buckets[fctx->bucketnum].lock);
goto cleanup;
/*
* Detach the extra reference that was set in rctx_chaseds()
* or a prior iteration of this function.
*/
fctx_unref(fctx);
switch (result) {
case ISC_R_CANCELED:
dns_resolver_destroyfetch(&fctx->nsfetch);
if (dns_rdataset_isassociated(frdataset)) {
dns_rdataset_disassociate(frdataset);
}
UNLOCK(&res->buckets[fctx->bucketnum].lock);
fctx_done_detach(&fctx, ISC_R_CANCELED);
} else if (fevent->result == ISC_R_SUCCESS) {
break;
case ISC_R_SUCCESS:
FCTXTRACE("resuming DS lookup");
dns_resolver_destroyfetch(&fctx->nsfetch);
if (dns_rdataset_isassociated(&fctx->nameservers)) {
dns_rdataset_disassociate(&fctx->nameservers);
}
dns_rdataset_clone(fevent->rdataset, &fctx->nameservers);
dns_rdataset_clone(frdataset, &fctx->nameservers);
if (dns_rdataset_isassociated(frdataset)) {
dns_rdataset_disassociate(frdataset);
}
fctx->ns_ttl = fctx->nameservers.ttl;
fctx->ns_ttl_ok = true;
log_ns_ttl(fctx, "resume_dslookup");
if (dns_rdataset_isassociated(fevent->rdataset)) {
dns_rdataset_disassociate(fevent->rdataset);
}
LOCK(&res->buckets[fctx->bucketnum].lock);
if (SHUTTINGDOWN(fctx)) {
maybe_cancel_validators(fctx, true);
UNLOCK(&res->buckets[fctx->bucketnum].lock);
goto cleanup;
}
UNLOCK(&res->buckets[fctx->bucketnum].lock);
fcount_decr(fctx);
dns_name_copy(fctx->nsname, fctx->domain);
result = fcount_incr(fctx, true);
@ -7382,71 +7385,46 @@ resume_dslookup(isc_task_t *task, isc_event_t *event) {
} else {
fctx_done_detach(&fctx, DNS_R_SERVFAIL);
}
} else {
unsigned int n;
dns_rdataset_t *nsrdataset = NULL;
break;
default:
if (dns_rdataset_isassociated(frdataset)) {
dns_rdataset_disassociate(frdataset);
}
/*
* Get domain and nameservers from fctx->nsfetch
* before we destroy it.
*/
domain = dns_fixedname_initname(&fixed);
dns_name_copy(fctx->nsfetch->private->domain, domain);
if (dns_name_equal(fctx->nsname, domain)) {
if (dns_rdataset_isassociated(fevent->rdataset)) {
dns_rdataset_disassociate(fevent->rdataset);
}
dns_resolver_destroyfetch(&fctx->nsfetch);
LOCK(&res->buckets[fctx->bucketnum].lock);
if (SHUTTINGDOWN(fctx)) {
maybe_cancel_validators(fctx, true);
UNLOCK(&res->buckets[fctx->bucketnum].lock);
goto cleanup;
}
UNLOCK(&res->buckets[fctx->bucketnum].lock);
fctx_done(&fctx, DNS_R_SERVFAIL);
goto cleanup;
}
dns_rdataset_init(&nameservers);
if (dns_rdataset_isassociated(
&fctx->nsfetch->private->nameservers)) {
domain = dns_fixedname_initname(&fixed);
dns_name_copy(fctx->nsfetch->private->domain, domain);
if (dns_name_equal(fctx->nsname, domain)) {
dns_resolver_destroyfetch(&fctx->nsfetch);
fctx_done_detach(&fctx, DNS_R_SERVFAIL);
return;
}
dns_rdataset_clone(&fctx->nsfetch->private->nameservers,
&nameservers);
nsrdataset = &nameservers;
} else {
domain = NULL;
}
dns_resolver_destroyfetch(&fctx->nsfetch);
n = dns_name_countlabels(fctx->nsname);
dns_name_getlabelsequence(fctx->nsname, 1, n - 1, fctx->nsname);
if (dns_rdataset_isassociated(fevent->rdataset)) {
dns_rdataset_disassociate(fevent->rdataset);
}
LOCK(&res->buckets[fctx->bucketnum].lock);
if (SHUTTINGDOWN(fctx)) {
maybe_cancel_validators(fctx, true);
UNLOCK(&res->buckets[fctx->bucketnum].lock);
goto cleanup;
}
UNLOCK(&res->buckets[fctx->bucketnum].lock);
FCTXTRACE("continuing to look for parent's NS records");
/* Starting a new fetch, so restore the extra reference */
fctx_addref(fctx);
result = dns_resolver_createfetch(
res, fctx->nsname, dns_rdatatype_ns, domain, nsrdataset,
NULL, NULL, 0, fctx->options, 0, NULL, task,
resume_dslookup, fctx, &fctx->nsrrset, NULL,
&fctx->nsfetch);
/*
* fevent->rdataset (a.k.a. fctx->nsrrset) must not be
* accessed below this point to prevent races with
* another thread concurrently processing the fetch.
*/
if (result != ISC_R_SUCCESS) {
if (result == DNS_R_DUPLICATE) {
result = DNS_R_SERVFAIL;
@ -7454,15 +7432,11 @@ resume_dslookup(isc_task_t *task, isc_event_t *event) {
fctx_unref(fctx);
fctx_done_detach(&fctx, result);
}
}
cleanup:
if (dns_rdataset_isassociated(&nameservers)) {
dns_rdataset_disassociate(&nameservers);
if (dns_rdataset_isassociated(&nameservers)) {
dns_rdataset_disassociate(&nameservers);
}
}
isc_event_free(&event);
fctx_detach(&ds_fctx);
}
static void