diff --git a/bin/named/server.c b/bin/named/server.c index 26ad34a1e2..8c948ef6f4 100644 --- a/bin/named/server.c +++ b/bin/named/server.c @@ -4931,7 +4931,7 @@ configure_view(dns_view_t *view, dns_viewlist_t *viewlist, cfg_obj_t *config, if (udpsize > 4096) { udpsize = 4096; } - dns_resolver_setudpsize(view->resolver, (uint16_t)udpsize); + dns_view_setudpsize(view, (uint16_t)udpsize); /* * Set the maximum UDP response size. diff --git a/lib/dns/include/dns/resolver.h b/lib/dns/include/dns/resolver.h index 2046f9421a..206c3f78ff 100644 --- a/lib/dns/include/dns/resolver.h +++ b/lib/dns/include/dns/resolver.h @@ -421,18 +421,6 @@ dns_resolver_addalternate(dns_resolver_t *resolver, const isc_sockaddr_t *alt, * \li only one of 'name' or 'alt' to be valid. */ -void -dns_resolver_setudpsize(dns_resolver_t *resolver, uint16_t udpsize); -/*%< - * Set the EDNS UDP buffer size advertised by the server. - */ - -uint16_t -dns_resolver_getudpsize(dns_resolver_t *resolver); -/*%< - * Get the current EDNS UDP buffer size. - */ - void dns_resolver_reset_algorithms(dns_resolver_t *resolver); /*%< diff --git a/lib/dns/include/dns/view.h b/lib/dns/include/dns/view.h index eff09b660e..e4dd0d178f 100644 --- a/lib/dns/include/dns/view.h +++ b/lib/dns/include/dns/view.h @@ -185,6 +185,7 @@ struct dns_view { dns_dlzdblist_t dlz_unsearched; uint32_t fail_ttl; dns_badcache_t *failcache; + unsigned int udpsize; /* * Configurable data for server use only, @@ -1299,4 +1300,22 @@ dns_view_sfd_find(dns_view_t *view, const dns_name_t *name, *\li 'foundname' to be valid with a buffer sufficient to hold the name. */ +isc_result_t +dns_view_getresolver(dns_view_t *view, dns_resolver_t **resolverp); +/*%< + * Return the resolver associated with the view. + */ + +void +dns_view_setudpsize(dns_view_t *view, uint16_t udpsize); +/*%< + * Set the EDNS UDP buffer size advertised by the server. + */ + +uint16_t +dns_view_getudpsize(dns_view_t *view); +/*%< + * Get the current EDNS UDP buffer size. + */ + ISC_LANG_ENDDECLS diff --git a/lib/dns/nta.c b/lib/dns/nta.c index 1d141ea27c..44714cfa71 100644 --- a/lib/dns/nta.c +++ b/lib/dns/nta.c @@ -44,6 +44,7 @@ struct dns_ntatable { /* Unlocked. */ unsigned int magic; + isc_mem_t *mctx; dns_view_t *view; isc_rwlock_t rwlock; isc_loopmgr_t *loopmgr; @@ -52,7 +53,7 @@ struct dns_ntatable { isc_refcount_t references; /* Locked by rwlock. */ dns_rbt_t *table; - bool shuttingdown; + atomic_bool shuttingdown; }; struct dns__nta { @@ -118,16 +119,18 @@ dns_ntatable_create(dns_view_t *view, isc_taskmgr_t *taskmgr, ntatable = isc_mem_get(view->mctx, sizeof(*ntatable)); *ntatable = (dns_ntatable_t){ .loopmgr = loopmgr, - .view = view, }; + isc_mem_attach(view->mctx, &ntatable->mctx); + dns_view_weakattach(view, &ntatable->view); + result = isc_task_create(taskmgr, &ntatable->task, 0); if (result != ISC_R_SUCCESS) { goto cleanup_ntatable; } isc_task_setname(ntatable->task, "ntatable", ntatable); - result = dns_rbt_create(view->mctx, dns__nta_free, NULL, + result = dns_rbt_create(ntatable->mctx, dns__nta_free, NULL, &ntatable->table); if (result != ISC_R_SUCCESS) { goto cleanup_task; @@ -146,7 +149,7 @@ cleanup_task: isc_task_detach(&ntatable->task); cleanup_ntatable: - isc_mem_put(view->mctx, ntatable, sizeof(*ntatable)); + isc_mem_putanddetach(&ntatable->mctx, ntatable, sizeof(*ntatable)); return (result); } @@ -158,7 +161,8 @@ dns__ntatable_destroy(dns_ntatable_t *ntatable) { dns_rbt_destroy(&ntatable->table); isc_rwlock_destroy(&ntatable->rwlock); isc_task_detach(&ntatable->task); - isc_mem_put(ntatable->view->mctx, ntatable, sizeof(*ntatable)); + INSIST(ntatable->view == NULL); + isc_mem_putanddetach(&ntatable->mctx, ntatable, sizeof(*ntatable)); } ISC_REFCOUNT_IMPL(dns_ntatable, dns__ntatable_destroy); @@ -222,14 +226,13 @@ fetch_done(isc_task_t *task, isc_event_t *event) { RWUNLOCK(&ntatable->rwlock, isc_rwlocktype_read); dns__nta_detach(&nta); /* for dns_resolver_createfetch() */ - dns_view_weakdetach(&view); } static void checkbogus(void *arg) { dns__nta_t *nta = arg; dns_ntatable_t *ntatable = nta->ntatable; - dns_view_t *view = NULL; + dns_resolver_t *resolver = NULL; isc_result_t result; if (nta->fetch != NULL) { @@ -243,17 +246,25 @@ checkbogus(void *arg) { dns_rdataset_disassociate(&nta->sigrdataset); } + if (atomic_load(&ntatable->shuttingdown)) { + isc_timer_stop(nta->timer); + return; + } + + result = dns_view_getresolver(ntatable->view, &resolver); + if (result != ISC_R_SUCCESS) { + return; + } + dns__nta_ref(nta); /* for dns_resolver_createfetch */ - 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, ntatable->task, - fetch_done, nta, &nta->rdataset, &nta->sigrdataset, - &nta->fetch); + resolver, nta->name, dns_rdatatype_nsec, NULL, NULL, NULL, NULL, + 0, DNS_FETCHOPT_NONTA, 0, NULL, ntatable->task, fetch_done, nta, + &nta->rdataset, &nta->sigrdataset, &nta->fetch); if (result != ISC_R_SUCCESS) { dns__nta_detach(&nta); /* for dns_resolver_createfetch() */ - dns_view_weakdetach(&view); } + dns_resolver_detach(&resolver); } static void @@ -278,21 +289,18 @@ static void nta_create(dns_ntatable_t *ntatable, const dns_name_t *name, dns__nta_t **target) { dns__nta_t *nta = NULL; - dns_view_t *view; REQUIRE(VALID_NTATABLE(ntatable)); REQUIRE(target != NULL && *target == NULL); - view = ntatable->view; - - nta = isc_mem_get(view->mctx, sizeof(dns__nta_t)); + nta = isc_mem_get(ntatable->mctx, sizeof(dns__nta_t)); *nta = (dns__nta_t){ .ntatable = ntatable, .loop = isc_loop_current(ntatable->loopmgr), .magic = NTA_MAGIC, }; - isc_mem_attach(view->mctx, &nta->mctx); + isc_mem_attach(ntatable->mctx, &nta->mctx); dns_rdataset_init(&nta->rdataset); dns_rdataset_init(&nta->sigrdataset); @@ -314,12 +322,12 @@ dns_ntatable_add(dns_ntatable_t *ntatable, const dns_name_t *name, bool force, REQUIRE(VALID_NTATABLE(ntatable)); - RWLOCK(&ntatable->rwlock, isc_rwlocktype_write); - - if (ntatable->shuttingdown) { - goto unlock; + if (atomic_load(&ntatable->shuttingdown)) { + return (ISC_R_SUCCESS); } + RWLOCK(&ntatable->rwlock, isc_rwlocktype_write); + nta_create(ntatable, name, &nta); nta->expiry = now + lifetime; @@ -350,7 +358,6 @@ dns_ntatable_add(dns_ntatable_t *ntatable, const dns_name_t *name, bool force, break; } -unlock: RWUNLOCK(&ntatable->rwlock, isc_rwlocktype_write); return (result); @@ -683,5 +690,7 @@ dns_ntatable_shutdown(dns_ntatable_t *ntatable) { } dns_rbtnodechain_invalidate(&chain); + + dns_view_weakdetach(&ntatable->view); RWUNLOCK(&ntatable->rwlock, isc_rwlocktype_write); } diff --git a/lib/dns/resolver.c b/lib/dns/resolver.c index 6cb02c410d..f6b3171b79 100644 --- a/lib/dns/resolver.c +++ b/lib/dns/resolver.c @@ -240,11 +240,6 @@ STATIC_ASSERT(NS_PROCESSING_LIMIT > NS_RR_LIMIT, */ #define RECV_BUFFER_SIZE 4096 /* XXXRTH Constant. */ -/*% - * Default EDNS0 buffer size - */ -#define DEFAULT_EDNS_BUFSIZE 1232 - /*% * This defines the maximum number of timeouts we will permit before we * disable EDNS0 on the query. @@ -561,7 +556,6 @@ struct dns_resolver { isc_task_t **tasks; uint32_t lame_ttl; ISC_LIST(alternate_t) alternates; - uint16_t udpsize; dns_rbt_t *algorithms; dns_rbt_t *digests; dns_rbt_t *mustbesecure; @@ -2661,7 +2655,7 @@ resquery_send(resquery_t *query) { * Set the default UDP size to what was * configured as 'edns-buffer-size' */ - udpsize = res->udpsize; + udpsize = res->view->udpsize; /* * This server timed out for the first time in @@ -10274,7 +10268,6 @@ dns_resolver_create(dns_view_t *view, isc_loopmgr_t *loopmgr, .taskmgr = taskmgr, .dispatchmgr = dispatchmgr, .options = options, - .udpsize = DEFAULT_EDNS_BUFSIZE, .spillatmin = 10, .spillat = 10, .spillatmax = 100, @@ -11040,18 +11033,6 @@ dns_resolver_addalternate(dns_resolver_t *resolver, const isc_sockaddr_t *alt, ISC_LIST_APPEND(resolver->alternates, a, link); } -void -dns_resolver_setudpsize(dns_resolver_t *resolver, uint16_t udpsize) { - REQUIRE(VALID_RESOLVER(resolver)); - resolver->udpsize = udpsize; -} - -uint16_t -dns_resolver_getudpsize(dns_resolver_t *resolver) { - REQUIRE(VALID_RESOLVER(resolver)); - return (resolver->udpsize); -} - void dns_resolver_flushbadcache(dns_resolver_t *resolver, const dns_name_t *name) { if (name != NULL) { diff --git a/lib/dns/view.c b/lib/dns/view.c index b7e3fe83b3..5a8d1c8072 100644 --- a/lib/dns/view.c +++ b/lib/dns/view.c @@ -75,6 +75,11 @@ #define DNS_VIEW_DELONLYHASH 111 #define DNS_VIEW_FAILCACHESIZE 1021 +/*% + * Default EDNS0 buffer size + */ +#define DEFAULT_EDNS_BUFSIZE 1232 + isc_result_t dns_view_create(isc_mem_t *mctx, dns_rdataclass_t rdclass, const char *name, dns_view_t **viewp) { @@ -110,6 +115,7 @@ dns_view_create(isc_mem_t *mctx, dns_rdataclass_t rdclass, const char *name, .synthfromdnssec = true, .trust_anchor_telemetry = true, .root_key_sentinel = true, + .udpsize = DEFAULT_EDNS_BUFSIZE, }; isc_refcount_init(&view->references, 1); @@ -507,23 +513,46 @@ dns_view_detach(dns_view_t **viewp) { if (isc_refcount_decrement(&view->references) == 1) { dns_zone_t *mkzone = NULL, *rdzone = NULL; dns_zt_t *zt = NULL; + dns_resolver_t *resolver = NULL; + dns_adb_t *adb = NULL; + dns_requestmgr_t *requestmgr = NULL; isc_refcount_destroy(&view->references); + /* Swap the pointers under the lock */ + LOCK(&view->lock); if (view->resolver != NULL) { - dns_resolver_shutdown(view->resolver); - dns_resolver_detach(&view->resolver); - } - if (view->adb != NULL) { - dns_adb_shutdown(view->adb); - dns_adb_detach(&view->adb); - } - if (view->requestmgr != NULL) { - dns_requestmgr_shutdown(view->requestmgr); - dns_requestmgr_detach(&view->requestmgr); + resolver = view->resolver; + view->resolver = NULL; + UNLOCK(&view->lock); + + dns_resolver_shutdown(resolver); + dns_resolver_detach(&resolver); + + LOCK(&view->lock); } - LOCK(&view->lock); + if (view->adb != NULL) { + adb = view->adb; + view->adb = NULL; + UNLOCK(&view->lock); + + dns_adb_shutdown(adb); + dns_adb_detach(&adb); + + LOCK(&view->lock); + } + + if (view->requestmgr != NULL) { + requestmgr = view->requestmgr; + view->requestmgr = NULL; + UNLOCK(&view->lock); + + dns_requestmgr_shutdown(requestmgr); + dns_requestmgr_detach(&requestmgr); + + LOCK(&view->lock); + } if (view->zonetable != NULL) { zt = view->zonetable; @@ -2396,3 +2425,31 @@ dns_view_sfd_find(dns_view_t *view, const dns_name_t *name, dns_name_copy(dns_rootname, foundname); } } + +isc_result_t +dns_view_getresolver(dns_view_t *view, dns_resolver_t **resolverp) { + isc_result_t result; + REQUIRE(DNS_VIEW_VALID(view)); + REQUIRE(resolverp != NULL && *resolverp == NULL); + LOCK(&view->lock); + if (view->resolver != NULL) { + dns_resolver_attach(view->resolver, resolverp); + result = ISC_R_SUCCESS; + } else { + result = ISC_R_SHUTTINGDOWN; + } + UNLOCK(&view->lock); + return (result); +} + +void +dns_view_setudpsize(dns_view_t *view, uint16_t udpsize) { + REQUIRE(DNS_VIEW_VALID(view)); + view->udpsize = udpsize; +} + +uint16_t +dns_view_getudpsize(dns_view_t *view) { + REQUIRE(DNS_VIEW_VALID(view)); + return (view->udpsize); +} diff --git a/lib/dns/zone.c b/lib/dns/zone.c index 8cd9765895..7e7355598c 100644 --- a/lib/dns/zone.c +++ b/lib/dns/zone.c @@ -11298,16 +11298,27 @@ zone_refreshkeys(dns_zone_t *zone) { #ifdef ENABLE_AFL if (!dns_fuzzing_resolver) { #endif /* ifdef ENABLE_AFL */ + /* + * We need to unlock and lock zone here because view + * gets locked in the dns_resolver_createfetch() via + * dns_view_findzonecut() and this violates the locking + * hierarchy (view -> zone). + */ UNLOCK_ZONE(zone); - result = dns_resolver_createfetch( - zone->view->resolver, kname, - dns_rdatatype_dnskey, NULL, NULL, NULL, NULL, 0, - DNS_FETCHOPT_NOVALIDATE | - DNS_FETCHOPT_UNSHARED | - DNS_FETCHOPT_NOCACHED, - 0, NULL, zone->task, keyfetch_done, kfetch, - &kfetch->dnskeyset, &kfetch->dnskeysigset, - &kfetch->fetch); + dns_resolver_t *resolver = NULL; + result = dns_view_getresolver(zone->view, &resolver); + if (result == ISC_R_SUCCESS) { + result = dns_resolver_createfetch( + resolver, kname, dns_rdatatype_dnskey, + NULL, NULL, NULL, NULL, 0, + DNS_FETCHOPT_NOVALIDATE | + DNS_FETCHOPT_UNSHARED | + DNS_FETCHOPT_NOCACHED, + 0, NULL, zone->task, keyfetch_done, + kfetch, &kfetch->dnskeyset, + &kfetch->dnskeysigset, &kfetch->fetch); + dns_resolver_detach(&resolver); + } LOCK_ZONE(zone); #ifdef ENABLE_AFL } else { @@ -14662,10 +14673,7 @@ again: if (dscp != -1) { have_xfrdscp = true; } - if (zone->view->resolver != NULL) { - udpsize = dns_resolver_getudpsize( - zone->view->resolver); - } + udpsize = dns_view_getudpsize(zone->view); (void)dns_peer_getudpsize(peer, &udpsize); (void)dns_peer_getrequestnsid(peer, &reqnsid); (void)dns_peer_getrequestexpire(peer, &reqexpire); @@ -14961,10 +14969,7 @@ ns_query(dns_zone_t *zone, dns_rdataset_t *soardataset, dns_stub_t *stub) { if (result == ISC_R_SUCCESS && dscp != -1) { have_xfrdscp = true; } - if (zone->view->resolver != NULL) { - udpsize = dns_resolver_getudpsize( - zone->view->resolver); - } + udpsize = dns_view_getudpsize(zone->view); (void)dns_peer_getudpsize(peer, &udpsize); (void)dns_peer_getrequestnsid(peer, &reqnsid); } diff --git a/lib/ns/client.c b/lib/ns/client.c index 8b83c3908c..60da425efb 100644 --- a/lib/ns/client.c +++ b/lib/ns/client.c @@ -963,7 +963,6 @@ ns_client_addopt(ns_client_t *client, dns_message_t *message, unsigned char cookie[COOKIE_SIZE]; isc_result_t result; dns_view_t *view = NULL; - dns_resolver_t *resolver = NULL; uint16_t udpsize; dns_ednsopt_t ednsopts[DNS_EDNSOPTIONS]; int count = 0; @@ -978,9 +977,8 @@ ns_client_addopt(ns_client_t *client, dns_message_t *message, env = client->manager->aclenv; view = client->view; - resolver = (view != NULL) ? view->resolver : NULL; - if (resolver != NULL) { - udpsize = dns_resolver_getudpsize(resolver); + if (view != NULL) { + udpsize = dns_view_getudpsize(view); } else { udpsize = client->manager->sctx->udpsize; } diff --git a/tests/dns/keytable_test.c b/tests/dns/keytable_test.c index 1ac1eb042e..656bb118bb 100644 --- a/tests/dns/keytable_test.c +++ b/tests/dns/keytable_test.c @@ -211,6 +211,7 @@ create_tables(void) { static void destroy_tables(void) { if (ntatable != NULL) { + dns_ntatable_shutdown(ntatable); dns_ntatable_detach(&ntatable); } if (keytable != NULL) {