Check if the fetch is shutting down in resume_dslookup()

The fetch can be in the shutting down state when resume_dslookup() is
trying to operate on it.

This is also a security issue, because a malicious actor can set up a
name server which delays certain queries in such a way that the fetch
will time out and shut down, which will cause named to crash.

Add a check to see if the fetch has the shutting down attribute set,
and cancel any further operations on it in such case.

A similar bug had been fixed earlier for the resume_qmin() function,
see [GL #966].

diff --git a/lib/dns/resolver.c b/lib/dns/resolver.c
index b57c5308bbe0a38686f966485fc300843d15732f..1816723749a972b0d8fc85bc423dd6bbf4342106 100644 (file)
--- a/lib/dns/resolver.c
+++ b/lib/dns/resolver.c
@@ -7242,18 +7242,21 @@ 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;
-       fetchctx_t *fctx;
+       dns_fetchevent_t *fevent = NULL;
+       dns_resolver_t *res = NULL;
+       fetchctx_t *fctx = NULL;
        isc_result_t result;
        dns_rdataset_t nameservers;
        dns_fixedname_t fixed;
-       dns_name_t *domain;
+       dns_name_t *domain = NULL;
        fetchctx_t *ev_fctx = NULL;
 
        REQUIRE(event->ev_type == DNS_EVENT_FETCHDONE);
        fevent = (dns_fetchevent_t *)event;
        fctx = event->ev_arg;
+
        REQUIRE(VALID_FCTX(fctx));
+       res = fctx->res;
 
        UNUSED(task);
        FCTXTRACE("resume_dslookup");
@@ -7282,6 +7285,15 @@ resume_dslookup(isc_task_t *task, isc_event_t *event) {
                isc_event_free(&event);
 
                dns_resolver_destroyfetch(&fctx->nsfetch);
+
+               LOCK(&res->buckets[fctx->bucketnum].lock);
+               if (SHUTTINGDOWN(fctx)) {
+                       maybe_destroy(fctx, true);
+                       UNLOCK(&res->buckets[fctx->bucketnum].lock);
+                       goto cleanup;
+               }
+               UNLOCK(&res->buckets[fctx->bucketnum].lock);
+
                fctx_done(fctx, ISC_R_CANCELED, __LINE__);
        } else if (fevent->result == ISC_R_SUCCESS) {
                FCTXTRACE("resuming DS lookup");
@@ -7301,6 +7313,14 @@ resume_dslookup(isc_task_t *task, isc_event_t *event) {
                fevent = NULL;
                isc_event_free(&event);
 
+               LOCK(&res->buckets[fctx->bucketnum].lock);
+               if (SHUTTINGDOWN(fctx)) {
+                       maybe_destroy(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);
@@ -7329,8 +7349,17 @@ resume_dslookup(isc_task_t *task, isc_event_t *event) {
                        fevent = NULL;
                        isc_event_free(&event);
 
-                       fctx_done(fctx, DNS_R_SERVFAIL, __LINE__);
                        dns_resolver_destroyfetch(&fctx->nsfetch);
+
+                       LOCK(&res->buckets[fctx->bucketnum].lock);
+                       if (SHUTTINGDOWN(fctx)) {
+                               maybe_destroy(fctx, true);
+                               UNLOCK(&res->buckets[fctx->bucketnum].lock);
+                               goto cleanup;
+                       }
+                       UNLOCK(&res->buckets[fctx->bucketnum].lock);
+
+                       fctx_done(fctx, DNS_R_SERVFAIL, __LINE__);
                        goto cleanup;
                }
                if (dns_rdataset_isassociated(
@@ -7351,13 +7380,21 @@ resume_dslookup(isc_task_t *task, isc_event_t *event) {
                fevent = NULL;
                isc_event_free(&event);
 
+               LOCK(&res->buckets[fctx->bucketnum].lock);
+               if (SHUTTINGDOWN(fctx)) {
+                       maybe_destroy(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");
 
                fctx_attach(fctx, &ev_fctx);
 
                result = dns_resolver_createfetch(
-                       fctx->res, fctx->nsname, dns_rdatatype_ns, domain,
-                       nsrdataset, NULL, NULL, 0, fctx->options, 0, NULL, task,
+                       res, fctx->nsname, dns_rdatatype_ns, domain, nsrdataset,
+                       NULL, NULL, 0, fctx->options, 0, NULL, task,
                        resume_dslookup, ev_fctx, &fctx->nsrrset, NULL,
                        &fctx->nsfetch);
                /*