From aed9cafd5c1c822c4e596eeb73de8ef6cf322ef7 Mon Sep 17 00:00:00 2001 From: Aram Sargsyan Date: Fri, 5 Dec 2025 10:06:28 +0000 Subject: [PATCH] Lock the catalog zone when reconfiguring it A catalog zone is updated in an offloaded thread, which is not stopped during a reconfiguration in an exclusive mode, and so can cause a race condition with it. Waiting for the offloaded threads to complete their work before entering into the exclusive mode can potentially cause unwanted delays, because offloaded threads are generally "allowed" to take a longer amount of time before they complete. Add a dns_catz_zone_prereconfig()/dns_catz_zone_postreconfig() pair of functions which currently just lock the catalog zone when reconfiguring it. The change should eliminate the race. As a side note, there was already a similar pair of functions, dns_catz_prereconfig() and dns_catz_postreconfig() which are called before and after reconfiguring a 'dns_catz_zones_t' object. Below are the stack traces of the reconfiguration thread which has asserted, and a catalog zone update thread which was caught in the middle of its work despite the fact that the exclusive mode is turned on. Stack trace of thread 23859: #0 0x00007f80e7b8e52f raise (libc.so.6) #1 0x00007f80e7b61e65 abort (libc.so.6) #2 0x0000000000422558 assertion_failed (named) #3 0x00007f80eaa6799e isc_assertion_failed (libisc-9.18.41.so) #4 0x00007f80ea5bc788 dns_catz_entry_getname (libdns-9.18.41.so) #5 0x000000000042ce0e catz_reconfigure (named) #6 0x000000000042d3c5 configure_catz_zone (named) #7 0x000000000042d7a4 configure_catz (named) #8 0x0000000000430645 configure_view (named) #9 0x000000000043d998 load_configuration (named) #10 0x000000000044184f loadconfig (named) #11 0x0000000000442525 named_server_reconfigcommand (named) #12 0x000000000041b277 named_control_docommand (named) #13 0x000000000041c74a control_command (named) #14 0x00007f80eaa912ae task_run (libisc-9.18.41.so) #15 0x00007f80eaa914cd isc_task_run (libisc-9.18.41.so) #16 0x00007f80eaa46435 isc__nm_async_task (libisc-9.18.41.so) #17 0x00007f80eaa467aa process_netievent (libisc-9.18.41.so) #18 0x00007f80eaa475a6 process_queue (libisc-9.18.41.so) #19 0x00007f80eaa46227 process_all_queues (libisc-9.18.41.so) #20 0x00007f80eaa462a1 async_cb (libisc-9.18.41.so) #21 0x00007f80e8d01893 uv__async_io.part.3 (libuv.so.1) #22 0x00007f80e8d13ac4 uv__io_poll (libuv.so.1) #23 0x00007f80e8d023fb uv_run (libuv.so.1) #24 0x00007f80eaa45ced nm_thread (libisc-9.18.41.so) #25 0x00007f80eaa9bda3 isc__trampoline_run (libisc-9.18.41.so) #26 0x00007f80e7f1e1ca start_thread (libpthread.so.0) #27 0x00007f80e7b798d3 __clone (libc.so.6) ... ... Stack trace of thread 23912: #0 0x00007f80ea5bc2da dns_catz_options_setdefault (libdns-9.18.41.so) #1 0x00007f80ea5bd411 dns__catz_zones_merge (libdns-9.18.41.so) #2 0x00007f80ea5c3c2f dns__catz_update_cb (libdns-9.18.41.so) #3 0x00007f80eaa4fee9 isc__nm_work_run (libisc-9.18.41.so) #4 0x00007f80eaa9bda3 isc__trampoline_run (libisc-9.18.41.so) #5 0x00007f80eaa4ff48 isc__nm_work_cb (libisc-9.18.41.so) #6 0x00007f80e8cfc75e worker (libuv.so.1) #7 0x00007f80e7f1e1ca start_thread (libpthread.so.0) #8 0x00007f80e7b798d3 __clone (libc.so.6) --- bin/named/server.c | 16 +++++++++++----- lib/dns/catz.c | 10 ++++++++++ lib/dns/include/dns/catz.h | 18 ++++++++++++++++++ 3 files changed, 39 insertions(+), 5 deletions(-) diff --git a/bin/named/server.c b/bin/named/server.c index a8213215e8b..1cabf96e728 100644 --- a/bin/named/server.c +++ b/bin/named/server.c @@ -2716,6 +2716,15 @@ configure_catz_zone(dns_view_t *view, dns_view_t *pview, } result = dns_catz_zone_add(view->catzs, &origin, &zone); + if (result != ISC_R_SUCCESS && result != ISC_R_EXISTS) { + cfg_obj_log(catz_obj, DNS_CATZ_ERROR_LEVEL, + "catz: dns_catz_zone_add failed: %s", + isc_result_totext(result)); + goto cleanup; + } + + dns_catz_zone_prereconfig(zone); + if (result == ISC_R_EXISTS) { catz_reconfig_data_t data = { .catz = zone, @@ -2734,11 +2743,6 @@ configure_catz_zone(dns_view_t *view, dns_view_t *pview, &data); result = ISC_R_SUCCESS; - } else if (result != ISC_R_SUCCESS) { - cfg_obj_log(catz_obj, DNS_CATZ_ERROR_LEVEL, - "catz: dns_catz_zone_add failed: %s", - isc_result_totext(result)); - goto cleanup; } dns_catz_zone_resetdefoptions(zone); @@ -2776,6 +2780,8 @@ configure_catz_zone(dns_view_t *view, dns_view_t *pview, opts->min_update_interval = cfg_obj_asduration(obj); } + dns_catz_zone_postreconfig(zone); + cleanup: dns_name_free(&origin, view->mctx); if (ipkl.count != 0) { diff --git a/lib/dns/catz.c b/lib/dns/catz.c index 305296b0204..569f6037c04 100644 --- a/lib/dns/catz.c +++ b/lib/dns/catz.c @@ -2525,6 +2525,16 @@ dns_catz_postreconfig(dns_catz_zones_t *catzs) { isc_ht_iter_destroy(&iter); } +void +dns_catz_zone_prereconfig(dns_catz_zone_t *catz) { + LOCK(&catz->lock); +} + +void +dns_catz_zone_postreconfig(dns_catz_zone_t *catz) { + UNLOCK(&catz->lock); +} + void dns_catz_zone_for_each_entry2(dns_catz_zone_t *catz, dns_catz_entry_cb2 cb, void *arg1, void *arg2) { diff --git a/lib/dns/include/dns/catz.h b/lib/dns/include/dns/catz.h index 4f9d3b1c851..2338acadcf0 100644 --- a/lib/dns/include/dns/catz.h +++ b/lib/dns/include/dns/catz.h @@ -403,6 +403,24 @@ dns_catz_zones_shutdown(dns_catz_zones_t *catzs); typedef void (*dns_catz_entry_cb2)(dns_catz_entry_t *entry, void *arg1, void *arg2); +void +dns_catz_zone_prereconfig(dns_catz_zone_t *catz); +/*%< + * Must be called before reconfiguring a catalog zone. Locks the catalog zone. + * + * Requires: + * \li 'catz' is a valid, unlocked dns_catz_zone_t. + */ + +void +dns_catz_zone_postreconfig(dns_catz_zone_t *catz); +/*%< + * Must be called after reconfiguring a catalog zone. Unlocks the catalog zone. + * + * Requires: + * \li 'catz' is a valid, locked dns_catz_zone_t. + */ + void dns_catz_zone_for_each_entry2(dns_catz_zone_t *catz, dns_catz_entry_cb2 cb, void *arg1, void *arg2); -- 2.47.3