diff --git a/CHANGES b/CHANGES index ecdc3070f3..2d0ca13c0d 100644 --- a/CHANGES +++ b/CHANGES @@ -1,3 +1,6 @@ +6226. [bug] Attach dispatchmgr in the dns_view object to prevent + use-after-free when shutting down. [GL #4228] + 6225. [func] Convert dns_nta, dns_forward and dns_keytable units to use QP trie instead of an RBT. [GL !7811] diff --git a/bin/delv/delv.c b/bin/delv/delv.c index e947abedec..d8f71d7e24 100644 --- a/bin/delv/delv.c +++ b/bin/delv/delv.c @@ -2148,7 +2148,8 @@ run_server(void *arg) { CHECK(ns_interfacemgr_create(mctx, sctx, loopmgr, netmgr, dispatchmgr, NULL, false, &interfacemgr)); - CHECK(dns_view_create(mctx, dns_rdataclass_in, "_default", &view)); + CHECK(dns_view_create(mctx, dispatchmgr, dns_rdataclass_in, "_default", + &view)); CHECK(dns_cache_create(loopmgr, dns_rdataclass_in, "", &cache)); dns_view_setcache(view, cache, false); dns_cache_detach(&cache); @@ -2164,7 +2165,6 @@ run_server(void *arg) { dns_view_initsecroots(view); CHECK(setup_dnsseckeys(NULL, view)); - dns_view_setdispatchmgr(view, dispatchmgr); CHECK(dns_view_createresolver(view, loopmgr, 1, netmgr, 0, tlsctx_client_cache, dispatch, NULL)); diff --git a/bin/named/server.c b/bin/named/server.c index c8b1bdbf81..6575d3cbcf 100644 --- a/bin/named/server.c +++ b/bin/named/server.c @@ -6423,13 +6423,12 @@ create_view(const cfg_obj_t *vconfig, dns_viewlist_t *viewlist, } INSIST(view == NULL); - result = dns_view_create(named_g_mctx, viewclass, viewname, &view); + result = dns_view_create(named_g_mctx, named_g_dispatchmgr, viewclass, + viewname, &view); if (result != ISC_R_SUCCESS) { return (result); } - dns_view_setdispatchmgr(view, named_g_dispatchmgr); - isc_nonce_buf(view->secret, sizeof(view->secret)); ISC_LIST_APPEND(*viewlist, view, link); diff --git a/bin/tests/system/pipelined/pipequeries.c b/bin/tests/system/pipelined/pipequeries.c index b242118fc2..bfe0c1b84a 100644 --- a/bin/tests/system/pipelined/pipequeries.c +++ b/bin/tests/system/pipelined/pipequeries.c @@ -282,7 +282,7 @@ main(int argc, char *argv[]) { RUNCHECK(dns_requestmgr_create(mctx, loopmgr, dispatchmgr, dispatchv4, NULL, &requestmgr)); - RUNCHECK(dns_view_create(mctx, 0, "_test", &view)); + RUNCHECK(dns_view_create(mctx, NULL, 0, "_test", &view)); isc_loopmgr_setup(loopmgr, sendqueries, NULL); isc_loopmgr_teardown(loopmgr, teardown_view, view); diff --git a/bin/tools/mdig.c b/bin/tools/mdig.c index 24ac710a56..f88edf49f6 100644 --- a/bin/tools/mdig.c +++ b/bin/tools/mdig.c @@ -2167,7 +2167,7 @@ main(int argc, char *argv[]) { mctx, loopmgr, dispatchmgr, have_ipv4 ? dispatchvx : NULL, have_ipv6 ? dispatchvx : NULL, &requestmgr)); - RUNCHECK(dns_view_create(mctx, 0, "_mdig", &view)); + RUNCHECK(dns_view_create(mctx, NULL, 0, "_mdig", &view)); query = ISC_LIST_HEAD(queries); isc_loopmgr_setup(loopmgr, sendqueries, query); diff --git a/fuzz/dns_message_checksig.c b/fuzz/dns_message_checksig.c index f3385ab4a8..a253fa7027 100644 --- a/fuzz/dns_message_checksig.c +++ b/fuzz/dns_message_checksig.c @@ -183,7 +183,7 @@ LLVMFuzzerInitialize(int *argc ISC_ATTR_UNUSED, char ***argv ISC_ATTR_UNUSED) { isc_loopmgr_create(mctx, 1, &loopmgr); - result = dns_view_create(mctx, dns_rdataclass_in, "view", &view); + result = dns_view_create(mctx, NULL, dns_rdataclass_in, "view", &view); if (result != ISC_R_SUCCESS) { fprintf(stderr, "dns_view_create failed: %s\n", isc_result_totext(result)); diff --git a/lib/dns/client.c b/lib/dns/client.c index 2eee2d3480..205c9764bc 100644 --- a/lib/dns/client.c +++ b/lib/dns/client.c @@ -206,13 +206,12 @@ createview(isc_mem_t *mctx, dns_rdataclass_t rdclass, isc_loopmgr_t *loopmgr, isc_result_t result; dns_view_t *view = NULL; - result = dns_view_create(mctx, rdclass, DNS_CLIENTVIEW_NAME, &view); + result = dns_view_create(mctx, dispatchmgr, rdclass, + DNS_CLIENTVIEW_NAME, &view); if (result != ISC_R_SUCCESS) { return (result); } - dns_view_setdispatchmgr(view, dispatchmgr); - /* Initialize view security roots */ dns_view_initsecroots(view); diff --git a/lib/dns/include/dns/view.h b/lib/dns/include/dns/view.h index f22cb876a2..aed9761679 100644 --- a/lib/dns/include/dns/view.h +++ b/lib/dns/include/dns/view.h @@ -259,8 +259,8 @@ struct dns_view { #endif /* HAVE_LMDB */ isc_result_t -dns_view_create(isc_mem_t *mctx, dns_rdataclass_t rdclass, const char *name, - dns_view_t **viewp); +dns_view_create(isc_mem_t *mctx, dns_dispatchmgr_t *dispmgr, + dns_rdataclass_t rdclass, const char *name, dns_view_t **viewp); /*%< * Create a view. * @@ -378,9 +378,6 @@ dns_view_createresolver(dns_view_t *view, isc_loopmgr_t *loopmgr, * *\li 'view' does not have a resolver already. * - *\li A dispatch manager has been associated with the view by calling - * dns_view_setdispatchmgr(). - * *\li The requirements of dns_resolver_create() apply to 'ndisp', * 'netmgr', 'options', 'tlsctx_cache', 'dispatchv4', and 'dispatchv6'. * @@ -1254,12 +1251,10 @@ dns_view_getudpsize(dns_view_t *view); * Get the current EDNS UDP buffer size. */ -void -dns_view_setdispatchmgr(dns_view_t *view, dns_dispatchmgr_t *dispatchmgr); dns_dispatchmgr_t * dns_view_getdispatchmgr(dns_view_t *view); /*%< - * Set/get the dispatch manager for the view; this wil be used + * Get the attached dispatch manager for the view; this will be used * by the resolver and request managers to send and receive DNS * messages. */ diff --git a/lib/dns/view.c b/lib/dns/view.c index 8f08e108d7..982c7518bc 100644 --- a/lib/dns/view.c +++ b/lib/dns/view.c @@ -79,7 +79,8 @@ #define DEFAULT_EDNS_BUFSIZE 1232 isc_result_t -dns_view_create(isc_mem_t *mctx, dns_rdataclass_t rdclass, const char *name, +dns_view_create(isc_mem_t *mctx, dns_dispatchmgr_t *dispatchmgr, + dns_rdataclass_t rdclass, const char *name, dns_view_t **viewp) { dns_view_t *view = NULL; isc_result_t result; @@ -129,6 +130,10 @@ dns_view_create(isc_mem_t *mctx, dns_rdataclass_t rdclass, const char *name, isc_mem_attach(mctx, &view->mctx); + if (dispatchmgr != NULL) { + dns_dispatchmgr_attach(dispatchmgr, &view->dispatchmgr); + } + isc_mutex_init(&view->lock); isc_rwlock_init(&view->sfd_lock); @@ -442,6 +447,7 @@ dns_view_detach(dns_view_t **viewp) { dns_resolver_t *resolver = NULL; dns_adb_t *adb = NULL; dns_requestmgr_t *requestmgr = NULL; + dns_dispatchmgr_t *dispatchmgr = NULL; isc_refcount_destroy(&view->references); @@ -477,6 +483,7 @@ dns_view_detach(dns_view_t **viewp) { } } adb = rcu_xchg_pointer(&view->adb, NULL); + dispatchmgr = rcu_xchg_pointer(&view->dispatchmgr, NULL); rcu_read_unlock(); if (view->requestmgr != NULL) { @@ -510,14 +517,15 @@ dns_view_detach(dns_view_t **viewp) { if (resolver != NULL) { dns_resolver_detach(&resolver); } - if (adb != NULL || zonetable != NULL) { - synchronize_rcu(); - if (adb != NULL) { - dns_adb_detach(&adb); - } - if (zonetable != NULL) { - dns_zt_detach(&zonetable); - } + synchronize_rcu(); + if (dispatchmgr != NULL) { + dns_dispatchmgr_detach(&dispatchmgr); + } + if (adb != NULL) { + dns_adb_detach(&adb); + } + if (zonetable != NULL) { + dns_zt_detach(&zonetable); } if (requestmgr != NULL) { dns_requestmgr_detach(&requestmgr); @@ -2411,16 +2419,18 @@ dns_view_getudpsize(dns_view_t *view) { return (view->udpsize); } -void -dns_view_setdispatchmgr(dns_view_t *view, dns_dispatchmgr_t *dispatchmgr) { - REQUIRE(DNS_VIEW_VALID(view)); - view->dispatchmgr = dispatchmgr; -} - dns_dispatchmgr_t * dns_view_getdispatchmgr(dns_view_t *view) { REQUIRE(DNS_VIEW_VALID(view)); - return (view->dispatchmgr); + + rcu_read_lock(); + dns_dispatchmgr_t *dispatchmgr = rcu_dereference(view->dispatchmgr); + if (dispatchmgr != NULL) { + dns_dispatchmgr_ref(dispatchmgr); + } + rcu_read_unlock(); + + return (dispatchmgr); } isc_result_t diff --git a/lib/dns/xfrin.c b/lib/dns/xfrin.c index b144c46663..8e04d23e77 100644 --- a/lib/dns/xfrin.c +++ b/lib/dns/xfrin.c @@ -928,9 +928,15 @@ xfrin_start(dns_xfrin_t *xfr) { if (xfr->transport == NULL || dns_transport_get_type(xfr->transport) == DNS_TRANSPORT_TCP) { - result = dns_dispatch_gettcp(dns_view_getdispatchmgr(xfr->view), - &xfr->primaryaddr, - &xfr->sourceaddr, &xfr->disp); + dns_dispatchmgr_t *dispmgr = dns_view_getdispatchmgr(xfr->view); + if (dispmgr == NULL) { + result = ISC_R_SHUTTINGDOWN; + } else { + result = dns_dispatch_gettcp(dispmgr, &xfr->primaryaddr, + &xfr->sourceaddr, + &xfr->disp); + dns_dispatchmgr_detach(&dispmgr); + } } if (result == ISC_R_SUCCESS) { char peer[ISC_SOCKADDR_FORMATSIZE]; @@ -938,9 +944,16 @@ xfrin_start(dns_xfrin_t *xfr) { xfrin_log(xfr, ISC_LOG_DEBUG(1), "attached to TCP connection to %s", peer); } else { - CHECK(dns_dispatch_createtcp(dns_view_getdispatchmgr(xfr->view), - &xfr->sourceaddr, - &xfr->primaryaddr, &xfr->disp)); + dns_dispatchmgr_t *dispmgr = dns_view_getdispatchmgr(xfr->view); + if (dispmgr == NULL) { + result = ISC_R_SHUTTINGDOWN; + } else { + result = dns_dispatch_createtcp( + dispmgr, &xfr->sourceaddr, &xfr->primaryaddr, + &xfr->disp); + dns_dispatchmgr_detach(&dispmgr); + } + CHECK(result); } /* Set the maximum timer */ diff --git a/tests/dns/dnstap_test.c b/tests/dns/dnstap_test.c index ead4a83ef7..e25a5dfeff 100644 --- a/tests/dns/dnstap_test.c +++ b/tests/dns/dnstap_test.c @@ -146,7 +146,7 @@ ISC_RUN_TEST_IMPL(dns_dt_send) { isc_time_t p, f; struct fstrm_iothr_options *fopt; - result = dns_test_makeview("test", false, &view); + result = dns_test_makeview("test", false, false, &view); assert_int_equal(result, ISC_R_SUCCESS); fopt = fstrm_iothr_options_init(); diff --git a/tests/dns/keytable_test.c b/tests/dns/keytable_test.c index f3657c6649..2442876340 100644 --- a/tests/dns/keytable_test.c +++ b/tests/dns/keytable_test.c @@ -170,7 +170,7 @@ create_tables(void) { dns_name_t *keyname = dns_fixedname_name(&fn); isc_stdtime_t now = isc_stdtime_now(); - assert_int_equal(dns_test_makeview("view", false, &view), + assert_int_equal(dns_test_makeview("view", false, false, &view), ISC_R_SUCCESS); dns_keytable_create(view, &keytable); @@ -602,7 +602,7 @@ ISC_LOOP_TEST_IMPL(nta) { dns_view_t *myview = NULL; isc_stdtime_t now = isc_stdtime_now(); - result = dns_test_makeview("view", false, &myview); + result = dns_test_makeview("view", false, false, &myview); assert_int_equal(result, ISC_R_SUCCESS); dns_view_initsecroots(myview); diff --git a/tests/dns/resolver_test.c b/tests/dns/resolver_test.c index e9d133e3fc..09106aadd6 100644 --- a/tests/dns/resolver_test.c +++ b/tests/dns/resolver_test.c @@ -35,7 +35,6 @@ #include -static dns_dispatchmgr_t *dispatchmgr = NULL; static dns_dispatch_t *dispatch = NULL; static dns_view_t *view = NULL; static isc_tlsctx_cache_t *tlsctx_cache = NULL; @@ -44,21 +43,22 @@ static int setup_test(void **state) { isc_result_t result; isc_sockaddr_t local; + dns_dispatchmgr_t *dispatchmgr = NULL; setup_managers(state); - result = dns_dispatchmgr_create(mctx, netmgr, &dispatchmgr); + result = dns_test_makeview("view", true, false, &view); assert_int_equal(result, ISC_R_SUCCESS); - result = dns_test_makeview("view", false, &view); - assert_int_equal(result, ISC_R_SUCCESS); - - dns_view_setdispatchmgr(view, dispatchmgr); + dispatchmgr = dns_view_getdispatchmgr(view); + assert_non_null(dispatchmgr); isc_sockaddr_any(&local); result = dns_dispatch_createudp(dispatchmgr, &local, &dispatch); assert_int_equal(result, ISC_R_SUCCESS); + dns_dispatchmgr_detach(&dispatchmgr); + return (0); } @@ -66,7 +66,6 @@ static int teardown_test(void **state) { dns_dispatch_detach(&dispatch); dns_view_detach(&view); - dns_dispatchmgr_detach(&dispatchmgr); teardown_managers(state); return (0); diff --git a/tests/include/tests/dns.h b/tests/include/tests/dns.h index 4a3896e4e0..22a626f35e 100644 --- a/tests/include/tests/dns.h +++ b/tests/include/tests/dns.h @@ -50,7 +50,8 @@ typedef struct { } isc_result_t -dns_test_makeview(const char *name, bool with_cache, dns_view_t **viewp); +dns_test_makeview(const char *name, bool with_dispatchmgr, bool with_cache, + dns_view_t **viewp); /*% * Create a zone with origin 'name', return a pointer to the zone object in diff --git a/tests/libtest/dns.c b/tests/libtest/dns.c index 4c8880fea7..de0545be8b 100644 --- a/tests/libtest/dns.c +++ b/tests/libtest/dns.c @@ -45,6 +45,7 @@ #include #include +#include #include #include #include @@ -59,12 +60,27 @@ dns_zonemgr_t *zonemgr = NULL; * Create a view. */ isc_result_t -dns_test_makeview(const char *name, bool with_cache, dns_view_t **viewp) { +dns_test_makeview(const char *name, bool with_dispatchmgr, bool with_cache, + dns_view_t **viewp) { isc_result_t result; dns_view_t *view = NULL; dns_cache_t *cache = NULL; + dns_dispatchmgr_t *dispatchmgr = NULL; + + if (with_dispatchmgr) { + result = dns_dispatchmgr_create(mctx, netmgr, &dispatchmgr); + if (result != ISC_R_SUCCESS) { + return (result); + } + } + + result = dns_view_create(mctx, dispatchmgr, dns_rdataclass_in, name, + &view); + + if (dispatchmgr != NULL) { + dns_dispatchmgr_detach(&dispatchmgr); + } - result = dns_view_create(mctx, dns_rdataclass_in, name, &view); if (result != ISC_R_SUCCESS) { return (result); } @@ -124,7 +140,7 @@ dns_test_makezone(const char *name, dns_zone_t **zonep, dns_view_t *view, * If requested, create a view. */ if (createview) { - result = dns_test_makeview("view", false, &view); + result = dns_test_makeview("view", false, false, &view); if (result != ISC_R_SUCCESS) { goto detach_zone; } diff --git a/tests/libtest/ns.c b/tests/libtest/ns.c index 0952de6f36..8f7f130253 100644 --- a/tests/libtest/ns.c +++ b/tests/libtest/ns.c @@ -439,7 +439,8 @@ ns_test_qctx_create(const ns_test_qctx_create_params_t *params, /* * Every client needs to belong to a view. */ - result = dns_test_makeview("view", params->with_cache, &client->view); + result = dns_test_makeview("view", false, params->with_cache, + &client->view); if (result != ISC_R_SUCCESS) { goto detach_client; } diff --git a/tests/ns/notify_test.c b/tests/ns/notify_test.c index 863a4a0692..96fb3c26a4 100644 --- a/tests/ns/notify_test.c +++ b/tests/ns/notify_test.c @@ -70,7 +70,7 @@ ISC_LOOP_TEST_IMPL(notify_start) { result = ns_test_getclient(NULL, false, &client); assert_int_equal(result, ISC_R_SUCCESS); - result = dns_test_makeview("view", false, &client->view); + result = dns_test_makeview("view", false, false, &client->view); assert_int_equal(result, ISC_R_SUCCESS); result = ns_test_serve_zone("example.com",