From: Mark Andrews Date: Fri, 7 Aug 2020 08:00:41 +0000 (+1000) Subject: Address use after free between view, resolver and nta. X-Git-Tag: v9.17.5~58^2 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=0b2555e8cfc187cb481080c06229a4eac416355d;p=thirdparty%2Fbind9.git 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 21c9103b5b2..12e59095d96 100644 --- 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 619e5edadea..174c25c2a3a 100644 --- 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 b0f6c32e93f..2b31408eeff 100644 --- 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 84404633727..f4a9ca5849d 100644 --- 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 d7b9f149c3b..eb3195ca0e5 100644 --- 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 d1f450fb50f..8b5eecb70f7 100644 --- 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