From f760ee3f8ce205cfe3620e7f2e0f97429f7c2beb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Sur=C3=BD?= Date: Thu, 1 Jun 2023 13:38:42 +0200 Subject: [PATCH 1/7] Disable URCU inlining if inlined rcu_dereference() fails to compile In some cases, the inlined version rcu_dereference() would not compile when working on pointer to opaque struct (namely Ubuntu Jammy). Detect such condition in the autoconf and disable the inlining of the small functions if it breaks the build. --- configure.ac | 13 +++++++++++++ lib/isc/include/isc/urcu.h | 3 --- 2 files changed, 13 insertions(+), 3 deletions(-) diff --git a/configure.ac b/configure.ac index 44e5b201ed..247846f170 100644 --- a/configure.ac +++ b/configure.ac @@ -247,6 +247,19 @@ AC_DEFINE_UNQUOTED([RCU_VERSION], ["$RCU_VERSION"], [Compile-time Userspace-RCU CFLAGS="$CFLAGS $LIBURCU_CFLAGS" LIBS="$LIBS $LIBURCU_LIBS" +# +# Userspace-RCU inlining doesn't work for rcu_deference() with some combination +# of C compiler and library version. +# +AC_MSG_CHECKING([whether we can inline small liburcu functions]) +AC_COMPILE_IFELSE( + [AC_LANG_PROGRAM([[#define URCU_INLINE_SMALL_FUNCTIONS 1 + #include ]], + [[struct opaque *a; struct opaque *b = rcu_dereference(a);]])], + [AC_MSG_RESULT([yes]) + AC_DEFINE([URCU_INLINE_SMALL_FUNCTIONS], [1], [Inline small (less than 10 lines) functions])], + [AC_MSG_RESULT([no])]) + # Fuzzing is not included in pairwise testing as fuzzing tools are # not present in the relevant Docker image. # diff --git a/lib/isc/include/isc/urcu.h b/lib/isc/include/isc/urcu.h index 48a1ac46da..42a410f555 100644 --- a/lib/isc/include/isc/urcu.h +++ b/lib/isc/include/isc/urcu.h @@ -19,9 +19,6 @@ #pragma GCC diagnostic push #pragma GCC diagnostic ignored "-Wunused-parameter" -/* Inline small (less than 10 lines) functions */ -#define URCU_INLINE_SMALL_FUNCTIONS - #if defined(RCU_MEMBARRIER) || defined(RCU_MB) || defined(RCU_SIGNAL) #include #elif defined(RCU_QSBR) From ad747976bbba3a3e0f260ce023a9d3a17cf21ce0 Mon Sep 17 00:00:00 2001 From: Mark Andrews Date: Wed, 31 May 2023 12:40:37 +1000 Subject: [PATCH 2/7] Use rcu methods to lock access view->zonetable dns_view_find* may be called after the final call to dns_view_detach is made which detaches view->zonetable to permit the server to shutdown. We need to detect if view->zonetable is NULL during this stage and appropriately recover. --- lib/dns/view.c | 154 +++++++++++++++++++++++++++++++++++--------- tests/dns/zt_test.c | 21 +++++- 2 files changed, 142 insertions(+), 33 deletions(-) diff --git a/lib/dns/view.c b/lib/dns/view.c index ee2a5c9a06..7349ae6f82 100644 --- a/lib/dns/view.c +++ b/lib/dns/view.c @@ -30,6 +30,7 @@ #include #include #include +#include #include #include @@ -133,7 +134,6 @@ dns_view_create(isc_mem_t *mctx, dns_rdataclass_t rdclass, const char *name, isc_rwlock_init(&view->sfd_lock); - view->zonetable = NULL; dns_zt_create(mctx, view, &view->zonetable); result = dns_fwdtable_create(mctx, &view->fwdtable); @@ -460,7 +460,7 @@ 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_zt_t *zonetable = NULL; dns_resolver_t *resolver = NULL; dns_adb_t *adb = NULL; dns_requestmgr_t *requestmgr = NULL; @@ -481,6 +481,15 @@ dns_view_detach(dns_view_t **viewp) { /* Swap the pointers under the lock */ LOCK(&view->lock); + rcu_read_lock(); + zonetable = rcu_xchg_pointer(&view->zonetable, NULL); + if (zonetable != NULL) { + if (view->flush) { + dns_zt_flush(zonetable); + } + } + rcu_read_unlock(); + if (view->resolver != NULL) { resolver = view->resolver; view->resolver = NULL; @@ -495,14 +504,6 @@ dns_view_detach(dns_view_t **viewp) { requestmgr = view->requestmgr; view->requestmgr = NULL; } - - if (view->zonetable != NULL) { - zt = view->zonetable; - view->zonetable = NULL; - if (view->flush) { - dns_zt_flush(zt); - } - } if (view->managed_keys != NULL) { mkzone = view->managed_keys; view->managed_keys = NULL; @@ -537,8 +538,9 @@ dns_view_detach(dns_view_t **viewp) { dns_requestmgr_detach(&requestmgr); } - if (zt != NULL) { - dns_zt_detach(&zt); + if (zonetable != NULL) { + synchronize_rcu(); + dns_zt_detach(&zonetable); } if (mkzone != NULL) { dns_zone_detach(&mkzone); @@ -560,10 +562,16 @@ dialup(dns_zone_t *zone, void *dummy) { void dns_view_dialup(dns_view_t *view) { - REQUIRE(DNS_VIEW_VALID(view)); - REQUIRE(view->zonetable != NULL); + dns_zt_t *zonetable = NULL; - (void)dns_zt_apply(view->zonetable, false, NULL, dialup, NULL); + REQUIRE(DNS_VIEW_VALID(view)); + + rcu_read_lock(); + zonetable = rcu_dereference(view->zonetable); + if (zonetable != NULL) { + (void)dns_zt_apply(zonetable, false, NULL, dialup, NULL); + } + rcu_read_unlock(); } void @@ -762,11 +770,19 @@ dns_view_thaw(dns_view_t *view) { isc_result_t dns_view_addzone(dns_view_t *view, dns_zone_t *zone) { isc_result_t result; + dns_zt_t *zonetable = NULL; REQUIRE(DNS_VIEW_VALID(view)); REQUIRE(!view->frozen); - result = dns_zt_mount(view->zonetable, zone); + rcu_read_lock(); + zonetable = rcu_dereference(view->zonetable); + if (zonetable != NULL) { + result = dns_zt_mount(zonetable, zone); + } else { + result = ISC_R_SHUTTINGDOWN; + } + rcu_read_unlock(); return (result); } @@ -774,9 +790,21 @@ dns_view_addzone(dns_view_t *view, dns_zone_t *zone) { isc_result_t dns_view_findzone(dns_view_t *view, const dns_name_t *name, dns_zone_t **zonep) { + isc_result_t result; + dns_zt_t *zonetable = NULL; + REQUIRE(DNS_VIEW_VALID(view)); - return (dns_zt_find(view->zonetable, name, DNS_ZTFIND_EXACT, zonep)); + rcu_read_lock(); + zonetable = rcu_dereference(view->zonetable); + if (zonetable != NULL) { + result = dns_zt_find(zonetable, name, DNS_ZTFIND_EXACT, zonep); + } else { + result = ISC_R_NOTFOUND; + } + rcu_read_unlock(); + + return (result); } isc_result_t @@ -791,6 +819,7 @@ dns_view_find(dns_view_t *view, const dns_name_t *name, dns_rdatatype_t type, bool is_cache, is_staticstub_zone; dns_rdataset_t zrdataset, zsigrdataset; dns_zone_t *zone = NULL; + dns_zt_t *zonetable = NULL; /* * Find an rdataset whose owner name is 'name', and whose type is @@ -813,7 +842,14 @@ dns_view_find(dns_view_t *view, const dns_name_t *name, dns_rdatatype_t type, * Find a database to answer the query. */ is_staticstub_zone = false; - result = dns_zt_find(view->zonetable, name, DNS_ZTFIND_MIRROR, &zone); + rcu_read_lock(); + zonetable = rcu_dereference(view->zonetable); + if (zonetable != NULL) { + result = dns_zt_find(zonetable, name, DNS_ZTFIND_MIRROR, &zone); + } else { + result = ISC_R_SHUTTINGDOWN; + } + rcu_read_unlock(); if (zone != NULL && dns_zone_gettype(zone) == dns_zone_staticstub && !use_static_stub) { @@ -1067,6 +1103,7 @@ dns_view_findzonecut(dns_view_t *view, const dns_name_t *name, bool is_cache, use_zone = false, try_hints = false; dns_zone_t *zone = NULL; dns_name_t *zfname = NULL; + dns_zt_t *zonetable = NULL; dns_rdataset_t zrdataset, zsigrdataset; dns_fixedname_t zfixedname; unsigned int ztoptions = DNS_ZTFIND_MIRROR; @@ -1087,7 +1124,14 @@ dns_view_findzonecut(dns_view_t *view, const dns_name_t *name, if ((options & DNS_DBFIND_NOEXACT) != 0) { ztoptions |= DNS_ZTFIND_NOEXACT; } - result = dns_zt_find(view->zonetable, name, ztoptions, &zone); + rcu_read_lock(); + zonetable = rcu_dereference(view->zonetable); + if (zonetable != NULL) { + result = dns_zt_find(zonetable, name, ztoptions, &zone); + } else { + result = ISC_R_SHUTTINGDOWN; + } + rcu_read_unlock(); if (result == ISC_R_SUCCESS || result == DNS_R_PARTIALMATCH) { result = dns_zone_getdb(zone, &db); } @@ -1287,11 +1331,19 @@ dns_viewlist_findzone(dns_viewlist_t *list, const dns_name_t *name, for (view = ISC_LIST_HEAD(*list); view != NULL; view = ISC_LIST_NEXT(view, link)) { + dns_zt_t *zonetable = NULL; if (!allclasses && view->rdclass != rdclass) { continue; } - result = dns_zt_find(view->zonetable, name, DNS_ZTFIND_EXACT, - (zone1 == NULL) ? &zone1 : &zone2); + rcu_read_lock(); + zonetable = rcu_dereference(view->zonetable); + if (zonetable != NULL) { + result = dns_zt_find(zonetable, name, DNS_ZTFIND_EXACT, + (zone1 == NULL) ? &zone1 : &zone2); + } else { + result = ISC_R_NOTFOUND; + } + rcu_read_unlock(); INSIST(result == ISC_R_SUCCESS || result == ISC_R_NOTFOUND); if (zone2 != NULL) { dns_zone_detach(&zone1); @@ -1311,15 +1363,39 @@ dns_viewlist_findzone(dns_viewlist_t *list, const dns_name_t *name, isc_result_t dns_view_load(dns_view_t *view, bool stop, bool newonly) { + isc_result_t result; + dns_zt_t *zonetable = NULL; + REQUIRE(DNS_VIEW_VALID(view)); - return (dns_zt_load(view->zonetable, stop, newonly)); + + rcu_read_lock(); + zonetable = rcu_dereference(view->zonetable); + if (zonetable != NULL) { + result = dns_zt_load(zonetable, stop, newonly); + } else { + result = ISC_R_SUCCESS; + } + rcu_read_unlock(); + return (result); } isc_result_t dns_view_asyncload(dns_view_t *view, bool newonly, dns_zt_callback_t *callback, void *arg) { + isc_result_t result; + dns_zt_t *zonetable = NULL; + REQUIRE(DNS_VIEW_VALID(view)); - return (dns_zt_asyncload(view->zonetable, newonly, callback, arg)); + + rcu_read_lock(); + zonetable = rcu_dereference(view->zonetable); + if (zonetable != NULL) { + result = dns_zt_asyncload(zonetable, newonly, callback, arg); + } else { + result = ISC_R_SUCCESS; + } + rcu_read_unlock(); + return (result); } isc_result_t @@ -1455,10 +1531,21 @@ dns_view_flushnode(dns_view_t *view, const dns_name_t *name, bool tree) { isc_result_t dns_view_freezezones(dns_view_t *view, bool value) { - REQUIRE(DNS_VIEW_VALID(view)); - REQUIRE(view->zonetable != NULL); + isc_result_t result; + dns_zt_t *zonetable = NULL; - return (dns_zt_freezezones(view->zonetable, view, value)); + REQUIRE(DNS_VIEW_VALID(view)); + + rcu_read_lock(); + zonetable = rcu_dereference(view->zonetable); + if (zonetable != NULL) { + result = dns_zt_freezezones(zonetable, view, value); + } else { + result = ISC_R_SUCCESS; + } + rcu_read_unlock(); + + return (result); } isc_result_t @@ -2119,6 +2206,7 @@ cleanup: void dns_view_setviewcommit(dns_view_t *view) { dns_zone_t *redirect = NULL, *managed_keys = NULL; + dns_zt_t *zonetable = NULL; REQUIRE(DNS_VIEW_VALID(view)); @@ -2133,9 +2221,13 @@ dns_view_setviewcommit(dns_view_t *view) { UNLOCK(&view->lock); - if (view->zonetable != NULL) { - dns_zt_setviewcommit(view->zonetable); + rcu_read_lock(); + zonetable = rcu_dereference(view->zonetable); + if (zonetable != NULL) { + dns_zt_setviewcommit(zonetable); } + rcu_read_unlock(); + if (redirect != NULL) { dns_zone_setviewcommit(redirect); dns_zone_detach(&redirect); @@ -2149,7 +2241,7 @@ dns_view_setviewcommit(dns_view_t *view) { void dns_view_setviewrevert(dns_view_t *view) { dns_zone_t *redirect = NULL, *managed_keys = NULL; - dns_zt_t *zonetable; + dns_zt_t *zonetable = NULL; REQUIRE(DNS_VIEW_VALID(view)); @@ -2164,7 +2256,6 @@ dns_view_setviewrevert(dns_view_t *view) { if (view->managed_keys != NULL) { dns_zone_attach(view->managed_keys, &managed_keys); } - zonetable = view->zonetable; UNLOCK(&view->lock); if (redirect != NULL) { @@ -2175,9 +2266,12 @@ dns_view_setviewrevert(dns_view_t *view) { dns_zone_setviewrevert(managed_keys); dns_zone_detach(&managed_keys); } + rcu_read_lock(); + zonetable = rcu_dereference(view->zonetable); if (zonetable != NULL) { dns_zt_setviewrevert(zonetable); } + rcu_read_unlock(); } bool diff --git a/tests/dns/zt_test.c b/tests/dns/zt_test.c index c5ab32dff9..33232f1edb 100644 --- a/tests/dns/zt_test.c +++ b/tests/dns/zt_test.c @@ -28,6 +28,7 @@ #include #include #include +#include #include #include @@ -56,13 +57,18 @@ count_zone(dns_zone_t *zone, void *uap) { ISC_LOOP_TEST_IMPL(apply) { isc_result_t result; dns_zone_t *zone = NULL; + dns_zt_t *zt = NULL; int nzones = 0; result = dns_test_makezone("foo", &zone, NULL, true); assert_int_equal(result, ISC_R_SUCCESS); view = dns_zone_getview(zone); - assert_non_null(view->zonetable); + rcu_read_lock(); + zt = rcu_dereference(view->zonetable); + rcu_read_unlock(); + + assert_non_null(zt); assert_int_equal(nzones, 0); result = dns_zt_apply(view->zonetable, false, NULL, count_zone, @@ -151,6 +157,7 @@ ISC_LOOP_TEST_IMPL(asyncload_zone) { isc_result_t result; int n; dns_zone_t *zone = NULL; + dns_zt_t *zt = NULL; char buf[4096]; result = dns_test_makezone("foo", &zone, NULL, true); @@ -161,7 +168,10 @@ ISC_LOOP_TEST_IMPL(asyncload_zone) { assert_int_equal(result, ISC_R_SUCCESS); view = dns_zone_getview(zone); - assert_non_null(view->zonetable); + rcu_read_lock(); + zt = rcu_dereference(view->zonetable); + rcu_read_unlock(); + assert_non_null(zt); assert_false(dns__zone_loadpending(zone)); zonefile = fopen("./zone.data", "wb"); @@ -239,7 +249,9 @@ ISC_LOOP_TEST_IMPL(asyncload_zt) { dns_zone_setfile(zone3, TESTS_DIR "/testdata/zt/nonexistent.db", dns_masterformat_text, &dns_master_style_default); - zt = view->zonetable; + rcu_read_lock(); + zt = rcu_dereference(view->zonetable); + rcu_read_unlock(); assert_non_null(zt); dns_test_setupzonemgr(); @@ -254,7 +266,10 @@ ISC_LOOP_TEST_IMPL(asyncload_zt) { assert_false(dns__zone_loadpending(zone2)); assert_false(atomic_load(&done)); + rcu_read_lock(); + zt = rcu_dereference(view->zonetable); dns_zt_asyncload(zt, false, all_done, NULL); + rcu_read_unlock(); } ISC_TEST_LIST_START From e0f41259bdcd4acadb5d82a5fd531c92549e734d Mon Sep 17 00:00:00 2001 From: Mark Andrews Date: Wed, 31 May 2023 12:59:03 +1000 Subject: [PATCH 3/7] Add dns_view_delzone dns_view_delzone performs the rcu locking required around accessing view->zonetable. --- bin/named/server.c | 10 +++++----- lib/dns/include/dns/view.h | 10 ++++++++++ lib/dns/view.c | 19 +++++++++++++++++++ 3 files changed, 34 insertions(+), 5 deletions(-) diff --git a/bin/named/server.c b/bin/named/server.c index 071fe0c511..425fdcc871 100644 --- a/bin/named/server.c +++ b/bin/named/server.c @@ -2856,7 +2856,7 @@ catz_addmodzone_cb(void *arg) { } /* Remove the zone from the zone table */ - dns_zt_unmount(cz->view->zonetable, zone); + dns_view_delzone(cz->view, zone); goto cleanup; } @@ -2925,7 +2925,7 @@ catz_delzone_cb(void *arg) { dns_zone_unload(zone); } - CHECK(dns_zt_unmount(cz->view->zonetable, zone)); + CHECK(dns_view_delzone(cz->view, zone)); file = dns_zone_getfile(zone); if (file != NULL) { isc_file_remove(file); @@ -13551,7 +13551,7 @@ do_addzone(named_server_t *server, ns_cfgctx_t *cfg, dns_view_t *view, } /* Remove the zone from the zone table */ - dns_zt_unmount(view->zonetable, zone); + dns_view_delzone(view, zone); goto cleanup; } @@ -13759,7 +13759,7 @@ do_modzone(named_server_t *server, ns_cfgctx_t *cfg, dns_view_t *view, } /* Remove the zone from the zone table */ - dns_zt_unmount(view->zonetable, zone); + dns_view_delzone(view, zone); goto cleanup; } @@ -14147,7 +14147,7 @@ named_server_delzone(named_server_t *server, isc_lex_t *lex, if (dns_zone_gettype(zone) == dns_zone_redirect) { dns_zone_detach(&view->redirect); } else { - CHECK(dns_zt_unmount(view->zonetable, zone)); + CHECK(dns_view_delzone(view, zone)); } /* Send cleanup event */ diff --git a/lib/dns/include/dns/view.h b/lib/dns/include/dns/view.h index e801b3a30f..aa0a650f0b 100644 --- a/lib/dns/include/dns/view.h +++ b/lib/dns/include/dns/view.h @@ -493,6 +493,16 @@ dns_view_addzone(dns_view_t *view, dns_zone_t *zone); *\li 'zone' is a valid zone. */ +isc_result_t +dns_view_delzone(dns_view_t *view, dns_zone_t *zone); +/*%< + * Removes zone 'zone' from 'view'. + * + * Requires: + * + *\li 'zone' is a valid zone. + */ + void dns_view_freeze(dns_view_t *view); /*%< diff --git a/lib/dns/view.c b/lib/dns/view.c index 7349ae6f82..809223f67d 100644 --- a/lib/dns/view.c +++ b/lib/dns/view.c @@ -787,6 +787,25 @@ dns_view_addzone(dns_view_t *view, dns_zone_t *zone) { return (result); } +isc_result_t +dns_view_delzone(dns_view_t *view, dns_zone_t *zone) { + isc_result_t result; + dns_zt_t *zonetable = NULL; + + REQUIRE(DNS_VIEW_VALID(view)); + + rcu_read_lock(); + zonetable = rcu_dereference(view->zonetable); + if (zonetable != NULL) { + result = dns_zt_unmount(zonetable, zone); + } else { + result = ISC_R_SUCCESS; + } + rcu_read_unlock(); + + return (result); +} + isc_result_t dns_view_findzone(dns_view_t *view, const dns_name_t *name, dns_zone_t **zonep) { From ceb3264082a7726c8deda2619873c67203a637da Mon Sep 17 00:00:00 2001 From: Mark Andrews Date: Wed, 31 May 2023 15:52:36 +1000 Subject: [PATCH 4/7] Add dns_view_apply Add dns_view_apply to allow dns_zt_apply to be called on view->zonetable with rcu locking applied. --- bin/named/server.c | 11 +++++------ bin/named/statschannel.c | 8 ++++---- lib/dns/include/dns/view.h | 16 ++++++++++++++++ lib/dns/view.c | 19 +++++++++++++++++++ tests/dns/zt_test.c | 3 +-- 5 files changed, 45 insertions(+), 12 deletions(-) diff --git a/bin/named/server.c b/bin/named/server.c index 425fdcc871..c51f388fbf 100644 --- a/bin/named/server.c +++ b/bin/named/server.c @@ -9740,8 +9740,7 @@ cleanup_viewlist: if (result == ISC_R_SUCCESS && strcmp(view->name, "_bind") != 0) { dns_view_setviewrevert(view); - (void)dns_zt_apply(view->zonetable, false, NULL, - removed, view); + (void)dns_view_apply(view, false, NULL, removed, view); } dns_view_detach(&view); } @@ -11311,8 +11310,8 @@ add_view_tolist(struct dumpcontext *dctx, dns_view_t *view) { ISC_LIST_INIT(vle->zonelist); ISC_LIST_APPEND(dctx->viewlist, vle, link); if (dctx->dumpzones) { - result = dns_zt_apply(view->zonetable, true, NULL, - add_zone_tolist, dctx); + result = dns_view_apply(view, true, NULL, add_zone_tolist, + dctx); } return (result); } @@ -12409,8 +12408,8 @@ named_server_sync(named_server_t *server, isc_lex_t *lex, isc_buffer_t **text) { for (view = ISC_LIST_HEAD(server->viewlist); view != NULL; view = ISC_LIST_NEXT(view, link)) { - result = dns_zt_apply(view->zonetable, false, NULL, - synczone, &cleanup); + result = dns_view_apply(view, false, NULL, synczone, + &cleanup); if (result != ISC_R_SUCCESS && tresult == ISC_R_SUCCESS) { tresult = result; diff --git a/bin/named/statschannel.c b/bin/named/statschannel.c index bbb402262e..0b97cdaa7b 100644 --- a/bin/named/statschannel.c +++ b/bin/named/statschannel.c @@ -1780,8 +1780,8 @@ generatexml(named_server_t *server, uint32_t flags, int *buflen, if ((flags & STATS_XML_ZONES) != 0) { TRY0(xmlTextWriterStartElement(writer, ISC_XMLCHAR "zones")); - CHECK(dns_zt_apply(view->zonetable, true, NULL, - zone_xmlrender, writer)); + CHECK(dns_view_apply(view, true, NULL, zone_xmlrender, + writer)); TRY0(xmlTextWriterEndElement(writer)); /* /zones */ } @@ -2494,8 +2494,8 @@ generatejson(named_server_t *server, size_t *msglen, const char **msg, CHECKMEM(za); if ((flags & STATS_JSON_ZONES) != 0) { - CHECK(dns_zt_apply(view->zonetable, true, NULL, - zone_jsonrender, za)); + CHECK(dns_view_apply(view, true, NULL, + zone_jsonrender, za)); } if (json_object_array_length(za) != 0) { diff --git a/lib/dns/include/dns/view.h b/lib/dns/include/dns/view.h index aa0a650f0b..51f5fceb89 100644 --- a/lib/dns/include/dns/view.h +++ b/lib/dns/include/dns/view.h @@ -1291,4 +1291,20 @@ dns_view_addtrustedkey(dns_view_t *view, dns_rdatatype_t rdtype, * *\li Anything else Failure. */ + +isc_result_t +dns_view_apply(dns_view_t *view, bool stop, isc_result_t *sub, + isc_result_t (*action)(dns_zone_t *, void *), void *uap); +/*%< + * Call dns_zt_apply on the view's zonetable. + * + * Returns: + * \li ISC_R_SUCCESS if action was applied to all nodes. If 'stop' is + * false and 'sub' is non NULL then the first error (if any) + * reported by 'action' is returned in '*sub'. If 'stop' is true, + * the first error code from 'action' is returned. + * + * \li ISC_R_SHUTTINGDOWN if the view is in the process of shutting down. + */ + ISC_LANG_ENDDECLS diff --git a/lib/dns/view.c b/lib/dns/view.c index 809223f67d..55347368e7 100644 --- a/lib/dns/view.c +++ b/lib/dns/view.c @@ -2474,3 +2474,22 @@ dns_view_addtrustedkey(dns_view_t *view, dns_rdatatype_t rdtype, cleanup: return (result); } + +isc_result_t +dns_view_apply(dns_view_t *view, bool stop, isc_result_t *sub, + isc_result_t (*action)(dns_zone_t *, void *), void *uap) { + isc_result_t result; + dns_zt_t *zonetable = NULL; + + REQUIRE(DNS_VIEW_VALID(view)); + + rcu_read_lock(); + zonetable = rcu_dereference(view->zonetable); + if (zonetable != NULL) { + result = dns_zt_apply(zonetable, stop, sub, action, uap); + } else { + result = ISC_R_SHUTTINGDOWN; + } + rcu_read_unlock(); + return (result); +} diff --git a/tests/dns/zt_test.c b/tests/dns/zt_test.c index 33232f1edb..2ca5f14a2d 100644 --- a/tests/dns/zt_test.c +++ b/tests/dns/zt_test.c @@ -71,8 +71,7 @@ ISC_LOOP_TEST_IMPL(apply) { assert_non_null(zt); assert_int_equal(nzones, 0); - result = dns_zt_apply(view->zonetable, false, NULL, count_zone, - &nzones); + result = dns_view_apply(view, false, NULL, count_zone, &nzones); assert_int_equal(result, ISC_R_SUCCESS); assert_int_equal(nzones, 1); From 8d86fa7135136f50e17ef0318aa127b6e7dad232 Mon Sep 17 00:00:00 2001 From: Mark Andrews Date: Wed, 31 May 2023 16:03:56 +1000 Subject: [PATCH 5/7] Extend dns_view_findzone to take an options argument This is in preparation to allow the few remaining direct dns_zt_find(view->zonetable, ...) to use it for rcu mediated access to view->zonetable. --- bin/named/server.c | 46 +++++++++++++++++----------- bin/tests/system/dyndb/driver/zone.c | 2 +- lib/dns/catz.c | 2 +- lib/dns/dlz.c | 2 +- lib/dns/include/dns/view.h | 6 ++-- lib/dns/view.c | 4 +-- lib/ns/notify.c | 3 +- lib/ns/update.c | 3 +- lib/ns/xfrout.c | 3 +- 9 files changed, 42 insertions(+), 29 deletions(-) diff --git a/bin/named/server.c b/bin/named/server.c index c51f388fbf..4456cfb12c 100644 --- a/bin/named/server.c +++ b/bin/named/server.c @@ -2708,7 +2708,7 @@ catz_addmodzone_cb(void *arg) { goto cleanup; } - result = dns_view_findzone(cz->view, name, &zone); + result = dns_view_findzone(cz->view, name, DNS_ZTFIND_EXACT, &zone); if (cz->mod) { dns_catz_zone_t *parentcatz; @@ -2834,7 +2834,7 @@ catz_addmodzone_cb(void *arg) { } /* Is it there yet? */ - CHECK(dns_view_findzone(cz->view, name, &zone)); + CHECK(dns_view_findzone(cz->view, name, DNS_ZTFIND_EXACT, &zone)); /* * Load the zone from the master file. If this fails, we'll @@ -2891,7 +2891,7 @@ catz_delzone_cb(void *arg) { dns_name_format(dns_catz_entry_getname(cz->entry), cname, DNS_NAME_FORMATSIZE); result = dns_view_findzone(cz->view, dns_catz_entry_getname(cz->entry), - &zone); + DNS_ZTFIND_EXACT, &zone); if (result != ISC_R_SUCCESS) { isc_log_write(named_g_lctx, NAMED_LOGCATEGORY_GENERAL, NAMED_LOGMODULE_SERVER, ISC_LOG_WARNING, @@ -3068,7 +3068,8 @@ configure_catz_zone(dns_view_t *view, dns_view_t *pview, isc_ht_iter_current(it, (void **)&entry); name = dns_catz_entry_getname(entry); - tresult = dns_view_findzone(pview, name, &dnszone); + tresult = dns_view_findzone(pview, name, + DNS_ZTFIND_EXACT, &dnszone); if (tresult != ISC_R_SUCCESS) { continue; } @@ -5036,7 +5037,8 @@ configure_view(dns_view_t *view, dns_viewlist_t *viewlist, cfg_obj_t *config, */ if (view->hints == NULL) { dns_zone_t *rootzone = NULL; - (void)dns_view_findzone(view, dns_rootname, &rootzone); + (void)dns_view_findzone(view, dns_rootname, DNS_ZTFIND_EXACT, + &rootzone); if (rootzone != NULL) { dns_zone_detach(&rootzone); need_hints = false; @@ -5768,7 +5770,8 @@ configure_view(dns_view_t *view, dns_viewlist_t *viewlist, cfg_obj_t *config, /* * This zone already exists. */ - (void)dns_view_findzone(view, name, &zone); + (void)dns_view_findzone(view, name, DNS_ZTFIND_EXACT, + &zone); if (zone != NULL) { dns_zone_detach(&zone); continue; @@ -5799,7 +5802,8 @@ configure_view(dns_view_t *view, dns_viewlist_t *viewlist, cfg_obj_t *config, } if (pview != NULL) { - (void)dns_view_findzone(pview, name, &zone); + (void)dns_view_findzone( + pview, name, DNS_ZTFIND_EXACT, &zone); dns_view_detach(&pview); } @@ -5858,7 +5862,8 @@ configure_view(dns_view_t *view, dns_viewlist_t *viewlist, cfg_obj_t *config, CHECK(dns_name_fromstring( name, zones[ipv4only_zone].name, 0, NULL)); - (void)dns_view_findzone(view, name, &zone); + (void)dns_view_findzone(view, name, DNS_ZTFIND_EXACT, + &zone); if (zone != NULL) { dns_zone_detach(&zone); continue; @@ -5888,7 +5893,8 @@ configure_view(dns_view_t *view, dns_viewlist_t *viewlist, cfg_obj_t *config, } if (pview != NULL) { - (void)dns_view_findzone(pview, name, &zone); + (void)dns_view_findzone( + pview, name, DNS_ZTFIND_EXACT, &zone); dns_view_detach(&pview); } @@ -6574,7 +6580,8 @@ configure_zone(const cfg_obj_t *config, const cfg_obj_t *zconfig, goto cleanup; } - result = dns_view_findzone(otherview, origin, &zone); + result = dns_view_findzone(otherview, origin, DNS_ZTFIND_EXACT, + &zone); dns_view_detach(&otherview); if (result != ISC_R_SUCCESS) { cfg_obj_log(zconfig, named_g_lctx, ISC_LOG_ERROR, @@ -6693,7 +6700,8 @@ configure_zone(const cfg_obj_t *config, const cfg_obj_t *zconfig, /* * Check for duplicates in the new zone table. */ - result = dns_view_findzone(view, origin, &dupzone); + result = dns_view_findzone(view, origin, DNS_ZTFIND_EXACT, + &dupzone); if (result == ISC_R_SUCCESS) { /* * We already have this zone! @@ -6749,7 +6757,8 @@ configure_zone(const cfg_obj_t *config, const cfg_obj_t *zconfig, goto cleanup; } if (pview != NULL) { - result = dns_view_findzone(pview, origin, &zone); + result = dns_view_findzone(pview, origin, DNS_ZTFIND_EXACT, + &zone); } if (result != ISC_R_NOTFOUND && result != ISC_R_SUCCESS) { goto cleanup; @@ -7815,7 +7824,7 @@ configure_zone_setviewcommit(isc_result_t result, const cfg_obj_t *zconfig, return; } - result2 = dns_view_findzone(pview, origin, &zone); + result2 = dns_view_findzone(pview, origin, DNS_ZTFIND_EXACT, &zone); if (result2 != ISC_R_SUCCESS) { dns_view_detach(&pview); return; @@ -10571,7 +10580,8 @@ zone_from_args(named_server_t *server, isc_lex_t *lex, const char *zonetxt, result = ISC_R_NOTFOUND; } } else { - result = dns_view_findzone(view, name, zonep); + result = dns_view_findzone(view, name, DNS_ZTFIND_EXACT, + zonep); } if (result != ISC_R_SUCCESS) { snprintf(problem, sizeof(problem), @@ -13438,7 +13448,7 @@ do_addzone(named_server_t *server, ns_cfgctx_t *cfg, dns_view_t *view, result = (view->redirect == NULL) ? ISC_R_NOTFOUND : ISC_R_EXISTS; } else { - result = dns_view_findzone(view, name, &zone); + result = dns_view_findzone(view, name, DNS_ZTFIND_EXACT, &zone); if (result == ISC_R_SUCCESS) { result = ISC_R_EXISTS; } @@ -13502,7 +13512,7 @@ do_addzone(named_server_t *server, ns_cfgctx_t *cfg, dns_view_t *view, } dns_zone_attach(view->redirect, &zone); } else { - result = dns_view_findzone(view, name, &zone); + result = dns_view_findzone(view, name, DNS_ZTFIND_EXACT, &zone); if (result != ISC_R_SUCCESS) { isc_log_write(named_g_lctx, NAMED_LOGCATEGORY_GENERAL, NAMED_LOGMODULE_SERVER, ISC_LOG_ERROR, @@ -13618,7 +13628,7 @@ do_modzone(named_server_t *server, ns_cfgctx_t *cfg, dns_view_t *view, result = ISC_R_NOTFOUND; } } else { - result = dns_view_findzone(view, name, &zone); + result = dns_view_findzone(view, name, DNS_ZTFIND_EXACT, &zone); } if (result != ISC_R_SUCCESS) { goto cleanup; @@ -13687,7 +13697,7 @@ do_modzone(named_server_t *server, ns_cfgctx_t *cfg, dns_view_t *view, } dns_zone_attach(view->redirect, &zone); } else { - CHECK(dns_view_findzone(view, name, &zone)); + CHECK(dns_view_findzone(view, name, DNS_ZTFIND_EXACT, &zone)); } #ifndef HAVE_LMDB diff --git a/bin/tests/system/dyndb/driver/zone.c b/bin/tests/system/dyndb/driver/zone.c index a8d36058d7..595b01da6a 100644 --- a/bin/tests/system/dyndb/driver/zone.c +++ b/bin/tests/system/dyndb/driver/zone.c @@ -139,7 +139,7 @@ publish_zone(sample_instance_t *inst, dns_zone_t *zone) { /* Return success if the zone is already in the view as expected. */ result = dns_view_findzone(inst->view, dns_zone_getorigin(zone), - &zone_in_view); + DNS_ZTFIND_EXACT, &zone_in_view); if (result != ISC_R_SUCCESS && result != ISC_R_NOTFOUND) { goto cleanup; } diff --git a/lib/dns/catz.c b/lib/dns/catz.c index f4b9c962fa..bcf887e3bb 100644 --- a/lib/dns/catz.c +++ b/lib/dns/catz.c @@ -561,7 +561,7 @@ dns__catz_zones_merge(dns_catz_zone_t *catz, dns_catz_zone_t *newcatz) { /* Try to find the zone in the view */ find_result = dns_view_findzone(catz->catzs->view, dns_catz_entry_getname(nentry), - &zone); + DNS_ZTFIND_EXACT, &zone); if (find_result == ISC_R_SUCCESS) { dns_catz_coo_t *coo = NULL; char pczname[DNS_NAME_FORMATSIZE]; diff --git a/lib/dns/dlz.c b/lib/dns/dlz.c index 3cd390600c..de22dd3a83 100644 --- a/lib/dns/dlz.c +++ b/lib/dns/dlz.c @@ -441,7 +441,7 @@ dns_dlz_writeablezone(dns_view_t *view, dns_dlzdb_t *dlzdb, } /* See if the zone already exists */ - result = dns_view_findzone(view, origin, &dupzone); + result = dns_view_findzone(view, origin, DNS_ZTFIND_EXACT, &dupzone); if (result == ISC_R_SUCCESS) { dns_zone_detach(&dupzone); result = ISC_R_EXISTS; diff --git a/lib/dns/include/dns/view.h b/lib/dns/include/dns/view.h index 51f5fceb89..adb8dc7612 100644 --- a/lib/dns/include/dns/view.h +++ b/lib/dns/include/dns/view.h @@ -761,11 +761,11 @@ dns_viewlist_findzone(dns_viewlist_t *list, const dns_name_t *name, */ isc_result_t -dns_view_findzone(dns_view_t *view, const dns_name_t *name, dns_zone_t **zonep); +dns_view_findzone(dns_view_t *view, const dns_name_t *name, + unsigned int options, dns_zone_t **zonep); /*%< * Search for the zone 'name' in the zone table of 'view'. - * If found, 'zonep' is (strongly) attached to it. There - * are no partial matches. + * If found, 'zonep' is (strongly) attached to it. * * Requires: * diff --git a/lib/dns/view.c b/lib/dns/view.c index 55347368e7..49fc8a8d80 100644 --- a/lib/dns/view.c +++ b/lib/dns/view.c @@ -808,7 +808,7 @@ dns_view_delzone(dns_view_t *view, dns_zone_t *zone) { isc_result_t dns_view_findzone(dns_view_t *view, const dns_name_t *name, - dns_zone_t **zonep) { + unsigned int options, dns_zone_t **zonep) { isc_result_t result; dns_zt_t *zonetable = NULL; @@ -817,7 +817,7 @@ dns_view_findzone(dns_view_t *view, const dns_name_t *name, rcu_read_lock(); zonetable = rcu_dereference(view->zonetable); if (zonetable != NULL) { - result = dns_zt_find(zonetable, name, DNS_ZTFIND_EXACT, zonep); + result = dns_zt_find(zonetable, name, options, zonep); } else { result = ISC_R_NOTFOUND; } diff --git a/lib/ns/notify.c b/lib/ns/notify.c index dd3a2882f5..633ed96b8b 100644 --- a/lib/ns/notify.c +++ b/lib/ns/notify.c @@ -146,7 +146,8 @@ ns_notify_start(ns_client_t *client, isc_nmhandle_t *handle) { } dns_name_format(zonename, namebuf, sizeof(namebuf)); - result = dns_view_findzone(client->view, zonename, &zone); + result = dns_view_findzone(client->view, zonename, DNS_ZTFIND_EXACT, + &zone); if (result == ISC_R_SUCCESS) { dns_zonetype_t zonetype = dns_zone_gettype(zone); diff --git a/lib/ns/update.c b/lib/ns/update.c index 3103263366..7d2f570953 100644 --- a/lib/ns/update.c +++ b/lib/ns/update.c @@ -1987,7 +1987,8 @@ ns_update_start(ns_client_t *client, isc_nmhandle_t *handle, "RRs"); } - result = dns_view_findzone(client->view, zonename, &zone); + result = dns_view_findzone(client->view, zonename, DNS_ZTFIND_EXACT, + &zone); if (result != ISC_R_SUCCESS) { FAILN(DNS_R_NOTAUTH, zonename, "not authoritative for update zone"); diff --git a/lib/ns/xfrout.c b/lib/ns/xfrout.c index b782a29654..cad24d2170 100644 --- a/lib/ns/xfrout.c +++ b/lib/ns/xfrout.c @@ -795,7 +795,8 @@ ns_xfr_start(ns_client_t *client, dns_rdatatype_t reqtype) { FAILC(DNS_R_FORMERR, "multiple questions"); } - result = dns_view_findzone(client->view, question_name, &zone); + result = dns_view_findzone(client->view, question_name, + DNS_ZTFIND_EXACT, &zone); if (result != ISC_R_SUCCESS || dns_zone_gettype(zone) == dns_zone_dlz) { /* * The normal zone table does not have a match, or this is From 783c6a9538ac506adbae7904700439db5299e9d2 Mon Sep 17 00:00:00 2001 From: Mark Andrews Date: Wed, 31 May 2023 16:13:29 +1000 Subject: [PATCH 6/7] Use dns_view_findzone instead of dns_zt_find This ensures that rcu locking is properly applied for view->zonetable. --- bin/tests/system/dyndb/driver/syncptr.c | 2 +- lib/dns/resolver.c | 2 +- lib/ns/query.c | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/bin/tests/system/dyndb/driver/syncptr.c b/bin/tests/system/dyndb/driver/syncptr.c index 270c7acca6..4bbca88ce9 100644 --- a/bin/tests/system/dyndb/driver/syncptr.c +++ b/bin/tests/system/dyndb/driver/syncptr.c @@ -166,7 +166,7 @@ syncptr_find_zone(sample_instance_t *inst, dns_rdata_t *rdata, dns_name_t *name, } /* Find a zone containing owner name of the PTR record. */ - result = dns_zt_find(inst->view->zonetable, name, 0, zone); + result = dns_view_findzone(inst->view, name, 0, zone); if (result == DNS_R_PARTIALMATCH) { result = ISC_R_SUCCESS; } else if (result != ISC_R_SUCCESS) { diff --git a/lib/dns/resolver.c b/lib/dns/resolver.c index e7e3c3914d..e4f98c769f 100644 --- a/lib/dns/resolver.c +++ b/lib/dns/resolver.c @@ -6593,7 +6593,7 @@ name_external(const dns_name_t *name, dns_rdatatype_t type, fetchctx_t *fctx) { * then don't cache. */ dns_ztfind_t options = DNS_ZTFIND_NOEXACT | DNS_ZTFIND_MIRROR; - result = dns_zt_find(fctx->res->view->zonetable, name, options, &zone); + result = dns_view_findzone(fctx->res->view, name, options, &zone); if (result == ISC_R_SUCCESS || result == DNS_R_PARTIALMATCH) { dns_name_t *zname = dns_zone_getorigin(zone); dns_namereln_t reln = dns_name_fullcompare( diff --git a/lib/ns/query.c b/lib/ns/query.c index 63479e9a6c..3d5752c8e5 100644 --- a/lib/ns/query.c +++ b/lib/ns/query.c @@ -1116,7 +1116,7 @@ query_getzonedb(ns_client_t *client, const dns_name_t *name, ztoptions |= DNS_ZTFIND_NOEXACT; } - result = dns_zt_find(client->view->zonetable, name, ztoptions, &zone); + result = dns_view_findzone(client->view, name, ztoptions, &zone); if (result == DNS_R_PARTIALMATCH) { partial = true; From 47ed3978069a279742106f10415c59b89afd3d45 Mon Sep 17 00:00:00 2001 From: Mark Andrews Date: Wed, 31 May 2023 17:20:27 +1000 Subject: [PATCH 7/7] Add CHANGES note for [GL #4093] --- CHANGES | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CHANGES b/CHANGES index 6ffefc54e2..a588a3382e 100644 --- a/CHANGES +++ b/CHANGES @@ -1,3 +1,6 @@ +6187. [bug] Address view shutdown INSIST when accessing the + zonetable. [GL #4093] + 6186. [bug] Fix a 'clients-per-query' miscalculation bug. When the 'stale-answer-enable' options was enabled and the 'stale-answer-client-timeout' option was enabled and