2
0
mirror of https://gitlab.isc.org/isc-projects/bind9 synced 2025-08-30 22:15:20 +00:00

Fix a race in resume_dslookup() (#45168)

This commit is contained in:
Mukund Sivaraman
2017-08-08 12:15:01 +05:30
parent 0ad72b96d2
commit c88efb83b3
2 changed files with 34 additions and 5 deletions

View File

@@ -1,3 +1,8 @@
4671. [bug] Fix a race condition that could cause the
resolver to crash with assertion failure when
chasing DS in specific conditions with a very
short RTT to the upstream nameserver. [RT #45168]
4670. [cleanup] Ensure that a request MAC is never sent back 4670. [cleanup] Ensure that a request MAC is never sent back
in an XFR response unless the signature was in an XFR response unless the signature was
verified. [RT #45494] verified. [RT #45494]

View File

@@ -1556,7 +1556,7 @@ fctx_sendevents(fetchctx_t *fctx, isc_result_t result, int line) {
if (!HAVE_ANSWER(fctx)) if (!HAVE_ANSWER(fctx))
event->result = result; event->result = result;
INSIST(result != ISC_R_SUCCESS || INSIST(event->result != ISC_R_SUCCESS ||
dns_rdataset_isassociated(event->rdataset) || dns_rdataset_isassociated(event->rdataset) ||
fctx->type == dns_rdatatype_any || fctx->type == dns_rdatatype_any ||
fctx->type == dns_rdatatype_rrsig || fctx->type == dns_rdatatype_rrsig ||
@@ -6651,6 +6651,12 @@ resume_dslookup(isc_task_t *task, isc_event_t *event) {
bucketnum = fctx->bucketnum; bucketnum = fctx->bucketnum;
if (fevent->result == ISC_R_CANCELED) { if (fevent->result == ISC_R_CANCELED) {
if (dns_rdataset_isassociated(fevent->rdataset))
dns_rdataset_disassociate(fevent->rdataset);
fevent = NULL;
isc_event_free(&event);
dns_resolver_destroyfetch(&fctx->nsfetch); dns_resolver_destroyfetch(&fctx->nsfetch);
fctx_done(fctx, ISC_R_CANCELED, __LINE__); fctx_done(fctx, ISC_R_CANCELED, __LINE__);
} else if (fevent->result == ISC_R_SUCCESS) { } else if (fevent->result == ISC_R_SUCCESS) {
@@ -6665,6 +6671,11 @@ resume_dslookup(isc_task_t *task, isc_event_t *event) {
fctx->ns_ttl_ok = ISC_TRUE; fctx->ns_ttl_ok = ISC_TRUE;
log_ns_ttl(fctx, "resume_dslookup"); log_ns_ttl(fctx, "resume_dslookup");
if (dns_rdataset_isassociated(fevent->rdataset))
dns_rdataset_disassociate(fevent->rdataset);
fevent = NULL;
isc_event_free(&event);
fcount_decr(fctx); fcount_decr(fctx);
dns_name_free(&fctx->domain, fctx->mctx); dns_name_free(&fctx->domain, fctx->mctx);
dns_name_init(&fctx->domain, NULL); dns_name_init(&fctx->domain, NULL);
@@ -6693,6 +6704,12 @@ resume_dslookup(isc_task_t *task, isc_event_t *event) {
domain = dns_fixedname_name(&fixed); domain = dns_fixedname_name(&fixed);
dns_name_copy(&fctx->nsfetch->private->domain, domain, NULL); dns_name_copy(&fctx->nsfetch->private->domain, domain, NULL);
if (dns_name_equal(&fctx->nsname, domain)) { if (dns_name_equal(&fctx->nsname, domain)) {
if (dns_rdataset_isassociated(fevent->rdataset))
dns_rdataset_disassociate(fevent->rdataset);
fevent = NULL;
isc_event_free(&event);
fctx_done(fctx, DNS_R_SERVFAIL, __LINE__); fctx_done(fctx, DNS_R_SERVFAIL, __LINE__);
dns_resolver_destroyfetch(&fctx->nsfetch); dns_resolver_destroyfetch(&fctx->nsfetch);
goto cleanup; goto cleanup;
@@ -6712,7 +6729,11 @@ resume_dslookup(isc_task_t *task, isc_event_t *event) {
if (dns_rdataset_isassociated(fevent->rdataset)) if (dns_rdataset_isassociated(fevent->rdataset))
dns_rdataset_disassociate(fevent->rdataset); dns_rdataset_disassociate(fevent->rdataset);
fevent = NULL;
isc_event_free(&event);
FCTXTRACE("continuing to look for parent's NS records"); FCTXTRACE("continuing to look for parent's NS records");
result = dns_resolver_createfetch(fctx->res, &fctx->nsname, result = dns_resolver_createfetch(fctx->res, &fctx->nsname,
dns_rdatatype_ns, domain, dns_rdatatype_ns, domain,
nsrdataset, NULL, nsrdataset, NULL,
@@ -6720,6 +6741,11 @@ resume_dslookup(isc_task_t *task, isc_event_t *event) {
resume_dslookup, fctx, resume_dslookup, fctx,
&fctx->nsrrset, NULL, &fctx->nsrrset, NULL,
&fctx->nsfetch); &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 != ISC_R_SUCCESS)
fctx_done(fctx, result, __LINE__); fctx_done(fctx, result, __LINE__);
else { else {
@@ -6730,12 +6756,10 @@ resume_dslookup(isc_task_t *task, isc_event_t *event) {
} }
cleanup: cleanup:
INSIST(event == NULL);
INSIST(fevent == NULL);
if (dns_rdataset_isassociated(&nameservers)) if (dns_rdataset_isassociated(&nameservers))
dns_rdataset_disassociate(&nameservers); dns_rdataset_disassociate(&nameservers);
if (dns_rdataset_isassociated(fevent->rdataset))
dns_rdataset_disassociate(fevent->rdataset);
INSIST(fevent->sigrdataset == NULL);
isc_event_free(&event);
if (!locked) if (!locked)
LOCK(&res->buckets[bucketnum].lock); LOCK(&res->buckets[bucketnum].lock);
bucket_empty = fctx_decreference(fctx); bucket_empty = fctx_decreference(fctx);