Address use after free between view, resolver and nta.

Hold a weak reference to the view so that it can't go away while
nta is performing its lookups.  Cancel nta timers once all external
references to the view have gone to prevent them triggering new work.

(cherry picked from commit 0b2555e8cfc187cb481080c06229a4eac416355d)

diff --git a/CHANGES b/CHANGES
index 50390bb0c0e253468f1c27f67746fe0cad2af606..769e5676d3dda6064675cbef30b69868d0d373da 100644 (file)
--- a/CHANGES
+++ b/CHANGES
@@ -1,3 +1,7 @@
+5488.  [bug]           nta needed to have a weak reference on view to prevent
+                       the view being deleted while nta tests are being
+                       performed. [GL #2067]
+
 5474.  [bug]           dns_rdata_hip_next() failed to return ISC_R_NOMORE
                        when it should have. [GL !3880]
 

diff --git a/lib/dns/include/dns/nta.h b/lib/dns/include/dns/nta.h
index edbaa77ccf053c61b914ddc063f89659046f0f49..59d4765f9f8767cfe2a55893ae16722ded927c2b 100644 (file)
--- a/lib/dns/include/dns/nta.h
+++ b/lib/dns/include/dns/nta.h
@@ -52,6 +52,7 @@ struct dns_ntatable {
        /* Locked by rwlock. */
        uint32_t                references;
        dns_rbt_t               *table;
+       bool                    shuttingdown;
 };
 
 #define NTATABLE_MAGIC         ISC_MAGIC('N', 'T', 'A', 't')
@@ -199,6 +200,13 @@ dns_ntatable_save(dns_ntatable_t *ntatable, FILE *fp);
 /*%<
  * Save the NTA table to the file opened as 'fp', for later loading.
  */
+
+void
+dns_ntatable_shutdown(dns_ntatable_t *ntatable);
+/*%<
+ * Cancel future checks to see if NTAs can be removed.
+ */
+
 ISC_LANG_ENDDECLS
 
 #endif /* DNS_NTA_H */

diff --git a/lib/dns/nta.c b/lib/dns/nta.c
index 7c790db229f32229f90f35c04a7bf7cb45556a6e..fef1b2da0f659e4d7194d40141091e4e1e0a5426 100644 (file)
--- a/lib/dns/nta.c
+++ b/lib/dns/nta.c
@@ -132,6 +132,7 @@ dns_ntatable_create(dns_view_t *view,
        if (result != ISC_R_SUCCESS)
                goto cleanup_rbt;
 
+       ntatable->shuttingdown = false;
        ntatable->timermgr = timermgr;
        ntatable->taskmgr = taskmgr;
 
@@ -248,27 +249,31 @@ fetch_done(isc_task_t *task, isc_event_t *event) {
                (void) isc_timer_reset(nta->timer, isc_timertype_inactive,
                                       NULL, NULL, true);
        nta_detach(view->mctx, &nta);
+       dns_view_weakdetach(&view);
 }
 
 static void
 checkbogus(isc_task_t *task, isc_event_t *event) {
        dns_nta_t *nta = event->ev_arg;
        dns_ntatable_t *ntatable = nta->ntatable;
-       dns_view_t *view = ntatable->view;
+       dns_view_t *view = NULL;
        isc_result_t result;
 
        if (nta->fetch != NULL) {
                dns_resolver_cancelfetch(nta->fetch);
                nta->fetch = NULL;
        }
-       if (dns_rdataset_isassociated(&nta->rdataset))
+       if (dns_rdataset_isassociated(&nta->rdataset)) {
                dns_rdataset_disassociate(&nta->rdataset);
-       if (dns_rdataset_isassociated(&nta->sigrdataset))
+       }
+       if (dns_rdataset_isassociated(&nta->sigrdataset)) {
                dns_rdataset_disassociate(&nta->sigrdataset);
+       }
 
        isc_event_free(&event);
 
        nta_ref(nta);
+       dns_view_weakattach(ntatable->view, &view);
        result = dns_resolver_createfetch(view->resolver, nta->name,
                                          dns_rdatatype_nsec,
                                          NULL, NULL, NULL,
@@ -277,8 +282,10 @@ checkbogus(isc_task_t *task, isc_event_t *event) {
                                          &nta->rdataset,
                                          &nta->sigrdataset,
                                          &nta->fetch);
-       if (result != ISC_R_SUCCESS)
+       if (result != ISC_R_SUCCESS) {
+               dns_view_weakdetach(&view);
                nta_detach(view->mctx, &nta);
+       }
 }
 
 static isc_result_t
@@ -342,11 +349,10 @@ nta_create(dns_ntatable_t *ntatable, dns_name_t *name, dns_nta_t **target) {
 }
 
 isc_result_t
-dns_ntatable_add(dns_ntatable_t *ntatable, dns_name_t *name,
-                bool force, isc_stdtime_t now,
-                uint32_t lifetime)
+dns_ntatable_add(dns_ntatable_t *ntatable, dns_name_t *name, bool force,
+                isc_stdtime_t now, uint32_t lifetime)
 {
-       isc_result_t result;
+       isc_result_t result = ISC_R_SUCCESS;
        dns_nta_t *nta = NULL;
        dns_rbtnode_t *node;
        dns_view_t *view;
@@ -355,27 +361,34 @@ dns_ntatable_add(dns_ntatable_t *ntatable, dns_name_t *name,
 
        view = ntatable->view;
 
+       RWLOCK(&ntatable->rwlock, isc_rwlocktype_write);
+
+       if (ntatable->shuttingdown) {
+               goto unlock;
+       }
+
        result = nta_create(ntatable, name, &nta);
-       if (result != ISC_R_SUCCESS)
-               return (result);
+       if (result != ISC_R_SUCCESS) {
+               goto unlock;
+       }
 
        nta->expiry = now + lifetime;
        nta->forced = force;
 
-       RWLOCK(&ntatable->rwlock, isc_rwlocktype_write);
-
        node = NULL;
        result = dns_rbt_addnode(ntatable->table, name, &node);
        if (result == ISC_R_SUCCESS) {
-               if (!force)
+               if (!force) {
                        (void)settimer(ntatable, nta, lifetime);
+               }
                node->data = nta;
                nta = NULL;
        } else if (result == ISC_R_EXISTS) {
                dns_nta_t *n = node->data;
                if (n == NULL) {
-                       if (!force)
+                       if (!force) {
                                (void)settimer(ntatable, nta, lifetime);
+                       }
                        node->data = nta;
                        nta = NULL;
                } else {
@@ -385,10 +398,12 @@ dns_ntatable_add(dns_ntatable_t *ntatable, dns_name_t *name,
                result = ISC_R_SUCCESS;
        }
 
+unlock:
        RWUNLOCK(&ntatable->rwlock, isc_rwlocktype_write);
 
-       if (nta != NULL)
+       if (nta != NULL) {
                nta_detach(view->mctx, &nta);
+       }
 
        return (result);
 }
@@ -720,3 +735,33 @@ dns_ntatable_save(dns_ntatable_t *ntatable, FILE *fp) {
        else
                return (written ? ISC_R_SUCCESS : ISC_R_NOTFOUND);
 }
+
+void
+dns_ntatable_shutdown(dns_ntatable_t *ntatable) {
+       isc_result_t result;
+       dns_rbtnode_t *node;
+       dns_rbtnodechain_t chain;
+
+       REQUIRE(VALID_NTATABLE(ntatable));
+
+       RWLOCK(&ntatable->rwlock, isc_rwlocktype_write);
+       ntatable->shuttingdown = true;
+
+       dns_rbtnodechain_init(&chain, ntatable->view->mctx);
+       result = dns_rbtnodechain_first(&chain, ntatable->table, NULL, NULL);
+       while (result == ISC_R_SUCCESS || result == DNS_R_NEWORIGIN) {
+               dns_rbtnodechain_current(&chain, NULL, NULL, &node);
+               if (node->data != NULL) {
+                       dns_nta_t *nta = (dns_nta_t *)node->data;
+                       if (nta->timer != NULL) {
+                               (void)isc_timer_reset(nta->timer,
+                                                     isc_timertype_inactive,
+                                                     NULL, NULL, true);
+                       }
+               }
+               result = dns_rbtnodechain_next(&chain, NULL, NULL);
+       }
+
+       dns_rbtnodechain_invalidate(&chain);
+       RWUNLOCK(&ntatable->rwlock, isc_rwlocktype_write);
+}

diff --git a/lib/dns/view.c b/lib/dns/view.c
index 0fca1d934d3630d78bd73fb062514df1e5acc019..c1ece2e682699d1bad94363d1f9c19da6ee29e64 100644 (file)
--- a/lib/dns/view.c
+++ b/lib/dns/view.c
@@ -626,6 +626,9 @@ view_flushanddetach(dns_view_t **viewp, bool flush) {
                if (view->catzs != NULL) {
                        dns_catz_catzs_detach(&view->catzs);
                }
+               if (view->ntatable_priv != NULL) {
+                       dns_ntatable_shutdown(view->ntatable_priv);
+               }
                UNLOCK(&view->lock);
 
                /* Need to detach zones outside view lock */

diff --git a/lib/dns/win32/libdns.def.in b/lib/dns/win32/libdns.def.in
index 63be97345798f1dfbaf39b8b6bca43800c95bfe2..b426f150dd3ca1d2ce97de5a856fef381aff9925 100644 (file)
--- a/lib/dns/win32/libdns.def.in
+++ b/lib/dns/win32/libdns.def.in
@@ -668,6 +668,7 @@ dns_ntatable_delete
 dns_ntatable_detach
 dns_ntatable_dump
 dns_ntatable_save
+dns_ntatable_shutdown
 dns_ntatable_totext
 dns_opcode_totext
 dns_opcodestats_create