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.

diff --git a/CHANGES b/CHANGES
index 21c9103b5b2a384d47c8a86428899b19ef9f7d9e..12e59095d96962db6ea58cb4f9d00359c0fd17b5 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]
+
 5487.  [cleanup]       Update managed keys log messages to be less confusing.
                        [GL #2027]
 

diff --git a/doc/man/dnssec-importkey.1in b/doc/man/dnssec-importkey.1in
index 619e5edadea50e134603f9fa9620bbd89803aca9..174c25c2a3ab6eca10e0857f70a0679133856833 100644 (file)
--- a/doc/man/dnssec-importkey.1in
+++ b/doc/man/dnssec-importkey.1in
@@ -39,7 +39,7 @@ level margin: \\n[rst2man-indent\\n[rst2man-indent-level]]
 .sp
 \fBdnssec\-importkey\fP reads a public DNSKEY record and generates a pair
 of .key/.private files. The DNSKEY record may be read from an existing
-.key file, in which case a corresponding .private file is
+\&.key file, in which case a corresponding .private file is
 generated, or it may be read from any other file or from the standard
 input, in which case both .key and .private files are generated.
 .sp

diff --git a/lib/dns/include/dns/nta.h b/lib/dns/include/dns/nta.h
index b0f6c32e93fa166b9c509e36bb2b03d2fba8b51b..2b31408eeffc7b19afdd6cd7a48e604d7f995862 100644 (file)
--- a/lib/dns/include/dns/nta.h
+++ b/lib/dns/include/dns/nta.h
@@ -54,6 +54,7 @@ struct dns_ntatable {
        isc_refcount_t references;
        /* Locked by rwlock. */
        dns_rbt_t *table;
+       bool       shuttingdown;
 };
 
 #define NTATABLE_MAGIC    ISC_MAGIC('N', 'T', 'A', 't')
@@ -197,6 +198,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 84404633727d906b27cba309199e0a65816ae217..f4a9ca5849d3fc93989c5d1c38a3623a1eadb2f2 100644 (file)
--- a/lib/dns/nta.c
+++ b/lib/dns/nta.c
@@ -128,6 +128,7 @@ dns_ntatable_create(dns_view_t *view, isc_taskmgr_t *taskmgr,
                goto cleanup_rbt;
        }
 
+       ntatable->shuttingdown = false;
        ntatable->timermgr = timermgr;
        ntatable->taskmgr = taskmgr;
 
@@ -239,13 +240,14 @@ fetch_done(isc_task_t *task, isc_event_t *event) {
                                      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) {
@@ -262,11 +264,13 @@ checkbogus(isc_task_t *task, isc_event_t *event) {
        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,
                NULL, 0, DNS_FETCHOPT_NONTA, 0, NULL, task, fetch_done, nta,
                &nta->rdataset, &nta->sigrdataset, &nta->fetch);
        if (result != ISC_R_SUCCESS) {
+               dns_view_weakdetach(&view);
                nta_detach(view->mctx, &nta);
        }
 }
@@ -330,7 +334,7 @@ nta_create(dns_ntatable_t *ntatable, const dns_name_t *name,
 isc_result_t
 dns_ntatable_add(dns_ntatable_t *ntatable, const 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;
@@ -339,16 +343,20 @@ dns_ntatable_add(dns_ntatable_t *ntatable, const dns_name_t *name, bool force,
 
        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);
+               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) {
@@ -372,6 +380,7 @@ dns_ntatable_add(dns_ntatable_t *ntatable, const dns_name_t *name, bool force,
                result = ISC_R_SUCCESS;
        }
 
+unlock:
        RWUNLOCK(&ntatable->rwlock, isc_rwlocktype_write);
 
        if (nta != NULL) {
@@ -663,3 +672,33 @@ cleanup:
                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);
+       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 d7b9f149c3bdb54a1ed4238041b92644593ff4c1..eb3195ca0e5adef4964d90beda6e3be542a83a6a 100644 (file)
--- a/lib/dns/view.c
+++ b/lib/dns/view.c
@@ -661,6 +661,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 d1f450fb50fee276c9f056d6f1f7eba5787d3afc..8b5eecb70f7dfe5790cd932f1ff9831c35c2d98e 100644 (file)
--- a/lib/dns/win32/libdns.def.in
+++ b/lib/dns/win32/libdns.def.in
@@ -675,6 +675,7 @@ dns_ntatable_create
 dns_ntatable_delete
 dns_ntatable_detach
 dns_ntatable_save
+dns_ntatable_shutdown
 dns_ntatable_totext
 dns_opcode_totext
 dns_opcodestats_create