Hold a reference to the NTA table for the lifetime of each NTA

Each dns__nta_t now references its parent ntatable in nta_create() and
releases it in dns__nta_destroy().  This avoids a use-after-free in
fetch_done() and other callbacks that dereference nta->ntatable: the
ntatable could otherwise be released by view destruction while an
in-flight resolver fetch still holds a reference to the NTA.

(cherry picked from commit 26c895cc9281d5cc36447d4a2c5464ad31137f76)

diff --git a/lib/dns/nta.c b/lib/dns/nta.c
index 83e9c38e0105e1d916b7c5e43db2f5972ab08305..fcfe8aca711ef90275f515fcdb8e63e9807028c6 100644 (file)
--- a/lib/dns/nta.c
+++ b/lib/dns/nta.c
@@ -153,34 +153,11 @@ ISC_REFCOUNT_IMPL(dns_ntatable, dns__ntatable_destroy);
 #endif
 
 static void
-fetch_done(void *arg) {
-       dns_fetchresponse_t *resp = (dns_fetchresponse_t *)arg;
-       dns__nta_t *nta = resp->arg;
-       isc_result_t eresult = resp->result;
+dns__nta_update_expiry(dns__nta_t *nta, isc_result_t eresult) {
        dns_ntatable_t *ntatable = nta->ntatable;
        dns_view_t *view = ntatable->view;
        isc_stdtime_t now = isc_stdtime_now();
 
-       if (dns_rdataset_isassociated(&nta->rdataset)) {
-               dns_rdataset_disassociate(&nta->rdataset);
-       }
-       if (dns_rdataset_isassociated(&nta->sigrdataset)) {
-               dns_rdataset_disassociate(&nta->sigrdataset);
-       }
-       if (nta->fetch == resp->fetch) {
-               nta->fetch = NULL;
-       }
-       dns_resolver_destroyfetch(&resp->fetch);
-
-       if (resp->node != NULL) {
-               dns_db_detachnode(resp->db, &resp->node);
-       }
-       if (resp->db != NULL) {
-               dns_db_detach(&resp->db);
-       }
-
-       dns_resolver_freefresp(&resp);
-
        switch (eresult) {
        case ISC_R_SUCCESS:
        case DNS_R_NCACHENXDOMAIN:
@@ -206,6 +183,37 @@ fetch_done(void *arg) {
                isc_timer_stop(nta->timer);
        }
        RWUNLOCK(&ntatable->rwlock, isc_rwlocktype_read);
+}
+
+static void
+fetch_done(void *arg) {
+       dns_fetchresponse_t *resp = (dns_fetchresponse_t *)arg;
+       dns__nta_t *nta = resp->arg;
+       isc_result_t eresult = resp->result;
+
+       if (dns_rdataset_isassociated(&nta->rdataset)) {
+               dns_rdataset_disassociate(&nta->rdataset);
+       }
+       if (dns_rdataset_isassociated(&nta->sigrdataset)) {
+               dns_rdataset_disassociate(&nta->sigrdataset);
+       }
+       if (nta->fetch == resp->fetch) {
+               nta->fetch = NULL;
+       }
+       dns_resolver_destroyfetch(&resp->fetch);
+
+       if (resp->node != NULL) {
+               dns_db_detachnode(resp->db, &resp->node);
+       }
+       if (resp->db != NULL) {
+               dns_db_detach(&resp->db);
+       }
+
+       dns_resolver_freefresp(&resp);
+
+       if (!nta->shuttingdown) {
+               dns__nta_update_expiry(nta, eresult);
+       }
 
        dns__nta_detach(&nta); /* for dns_resolver_createfetch() */
 }
@@ -228,7 +236,7 @@ checkbogus(void *arg) {
                dns_rdataset_disassociate(&nta->sigrdataset);
        }
 
-       if (atomic_load(&ntatable->shuttingdown)) {
+       if (nta->shuttingdown || atomic_load_acquire(&ntatable->shuttingdown)) {
                isc_timer_stop(nta->timer);
                return;
        }
@@ -278,7 +286,7 @@ nta_create(dns_ntatable_t *ntatable, const dns_name_t *name,
 
        nta = isc_mem_get(ntatable->mctx, sizeof(dns__nta_t));
        *nta = (dns__nta_t){
-               .ntatable = ntatable,
+               .ntatable = dns_ntatable_ref(ntatable),
                .name = DNS_NAME_INITEMPTY,
                .magic = NTA_MAGIC,
        };
@@ -305,7 +313,7 @@ dns_ntatable_add(dns_ntatable_t *ntatable, const dns_name_t *name, bool force,
 
        REQUIRE(VALID_NTATABLE(ntatable));
 
-       if (atomic_load(&ntatable->shuttingdown)) {
+       if (atomic_load_acquire(&ntatable->shuttingdown)) {
                return ISC_R_SUCCESS;
        }
 
@@ -366,14 +374,23 @@ dns_ntatable_delete(dns_ntatable_t *ntatable, const dns_name_t *name) {
        return result;
 }
 
+typedef struct dns__nta_async_data {
+       dns__nta_t *nta;
+       dns_ntatable_t *ntatable;
+} dns__nta_async_data_t;
+
 static void
 delete_expired(void *arg) {
-       dns__nta_t *nta = arg;
-       dns_ntatable_t *ntatable = nta->ntatable;
+       dns__nta_async_data_t *data = arg;
+       dns__nta_t *nta = data->nta;
+       dns_ntatable_t *ntatable = data->ntatable;
        isc_result_t result;
        dns_qp_t *qp = NULL;
        void *pval = NULL;
 
+       isc_mem_put(nta->mctx, data, sizeof(*data));
+
+       REQUIRE(VALID_NTA(nta));
        REQUIRE(VALID_NTATABLE(ntatable));
 
        RWLOCK(&ntatable->rwlock, isc_rwlocktype_write);
@@ -435,9 +452,13 @@ dns_ntatable_covered(dns_ntatable_t *ntatable, isc_stdtime_t now,
 
        if (nta->expiry <= now) {
                /* NTA is expired */
-               dns__nta_ref(nta);
-               dns_ntatable_ref(nta->ntatable);
-               isc_async_current(delete_expired, nta);
+               dns__nta_async_data_t *data = isc_mem_get(nta->mctx,
+                                                         sizeof(*data));
+               *data = (dns__nta_async_data_t){
+                       .nta = dns__nta_ref(nta),
+                       .ntatable = dns_ntatable_ref(nta->ntatable),
+               };
+               isc_async_current(delete_expired, data);
                goto done;
        }
 
@@ -594,6 +615,7 @@ dns__nta_shutdown_cb(void *arg) {
                isc_timer_destroy(&nta->timer);
        }
 
+       dns_ntatable_detach(&nta->ntatable);
        dns__nta_detach(&nta);
 }
 
@@ -616,7 +638,7 @@ dns_ntatable_shutdown(dns_ntatable_t *ntatable) {
 
        RWLOCK(&ntatable->rwlock, isc_rwlocktype_write);
        dns_qpmulti_query(ntatable->table, &qpr);
-       ntatable->shuttingdown = true;
+       atomic_store_release(&ntatable->shuttingdown, true);
 
        dns_qpiter_init(&qpr, &iter);
        while (dns_qpiter_next(&iter, NULL, &pval, NULL) == ISC_R_SUCCESS) {