From 900127c4601382d028af1042db44c36aae54d9fa Mon Sep 17 00:00:00 2001 From: Matthijs Mekking Date: Fri, 13 Mar 2026 11:13:35 +0100 Subject: [PATCH 1/3] Test showzone and modzone on configured zone Add test cases for 'rndc showzone' and 'rndc modzone' on a zone that was configured in named.conf. This should not crash. --- bin/tests/system/addzone/tests.sh | 23 +++++++++++++++++++++-- 1 file changed, 21 insertions(+), 2 deletions(-) diff --git a/bin/tests/system/addzone/tests.sh b/bin/tests/system/addzone/tests.sh index a59bc355d9..0cf8be2a75 100755 --- a/bin/tests/system/addzone/tests.sh +++ b/bin/tests/system/addzone/tests.sh @@ -36,6 +36,26 @@ n=$((n + 1)) if [ $ret != 0 ]; then echo_i "failed"; fi status=$((status + ret)) +# showzone +echo_i "showzone normally loaded zone ($n)" +ret=0 +$RNDCCMD 10.53.0.2 showzone normal.example >rndc.out.ns2.$n +expected='zone "normal.example" { type primary; file "normal.db"; };' +[ "$(cat rndc.out.ns2.$n)" = "$expected" ] || ret=1 +n=$((n + 1)) +if [ $ret != 0 ]; then echo_i "failed"; fi +status=$((status + ret)) +# modzone +echo_i "modzone normally loaded zone ($n)" +ret=0 +$RNDCCMD 10.53.0.2 modzone normal.example '{ type primary; file "normal.db"; };' >rndc.out.ns2.$n +grep "" rndc.out.ns2.$n >/dev/null || ret=1 +grep "zone 'normal.example' must also be reconfigured in" rndc.out.ns2.$n >/dev/null || ret=1 +grep "named.conf to make changes permanent." rndc.out.ns2.$n >/dev/null || ret=1 +n=$((n + 1)) +if [ $ret != 0 ]; then echo_i "failed"; fi +status=$((status + ret)) + # When LMDB support is compiled in, this tests that migration from # NZF to NZD occurs during named startup echo_i "checking previously added zone ($n)" @@ -263,7 +283,7 @@ status=$((status + ret)) echo_i "delete a normally-loaded zone ($n)" ret=0 $RNDCCMD 10.53.0.2 delzone normal.example >rndc.out.ns2.$n 2>&1 -grep "is no longer active and will be deleted" rndc.out.ns2.$n >/dev/null || ret=11 +grep "is no longer active and will be deleted" rndc.out.ns2.$n >/dev/null || ret=1 grep "To keep it from returning when the server is restarted" rndc.out.ns2.$n >/dev/null || ret=1 grep "must also be removed from named.conf." rndc.out.ns2.$n >/dev/null || ret=1 _check_delete_normally_loaded_zone() ( @@ -271,7 +291,6 @@ _check_delete_normally_loaded_zone() ( && grep 'status: REFUSED' dig.out.ns2.$n >/dev/null ) retry_quiet 5 _check_delete_normally_loaded_zone || ret=1 - n=$((n + 1)) if [ $ret != 0 ]; then echo_i "failed"; fi status=$((status + ret)) From 71587b0816a9458895aaa14a85539acd91afda38 Mon Sep 17 00:00:00 2001 From: Matthijs Mekking Date: Fri, 13 Mar 2026 11:52:47 +0100 Subject: [PATCH 2/3] Only lock view->newzone.lock if not already locked Some code paths try to lock an already locked view->newzone.lock. For example, do_modzone() aqcuires the lock and then calls delete_zoneconf(), that wants to acquire the same lock. Add a parameter to delete_zoneconf() that informs the function if the lock has already been acquired. --- bin/named/server.c | 39 ++++++++++++++++++++++++--------------- 1 file changed, 24 insertions(+), 15 deletions(-) diff --git a/bin/named/server.c b/bin/named/server.c index fa51bd260f..8d857eaa60 100644 --- a/bin/named/server.c +++ b/bin/named/server.c @@ -12723,7 +12723,7 @@ cleanup: static isc_result_t delete_zoneconf(dns_view_t *view, const cfg_obj_t *config, - const dns_name_t *zname, nzfwriter_t nzfwriter) { + const dns_name_t *zname, nzfwriter_t nzfwriter, bool locked) { isc_result_t result = ISC_R_NOTFOUND; const cfg_obj_t *zl = NULL; @@ -12731,7 +12731,9 @@ delete_zoneconf(dns_view_t *view, const cfg_obj_t *config, REQUIRE(config != NULL); REQUIRE(zname != NULL); - LOCK(&view->newzone.lock); + if (!locked) { + LOCK(&view->newzone.lock); + } cfg_map_get(config, "zone", &zl); @@ -12766,7 +12768,9 @@ delete_zoneconf(dns_view_t *view, const cfg_obj_t *config, } cleanup: - UNLOCK(&view->newzone.lock); + if (!locked) { + UNLOCK(&view->newzone.lock); + } return result; } @@ -12777,13 +12781,13 @@ do_addzone(named_server_t *server, dns_view_t *view, dns_name_t *name, isc_result_t result, tresult; dns_zone_t *zone = NULL; const cfg_obj_t *voptions = NULL; + bool locked = false; #ifndef HAVE_LMDB FILE *fp = NULL; bool cleanup_config = false; #else /* HAVE_LMDB */ MDB_txn *txn = NULL; MDB_dbi dbi; - bool locked = false; UNUSED(zoneconf); #endif @@ -12946,7 +12950,7 @@ cleanup: } if (result != ISC_R_SUCCESS && cleanup_config) { tresult = delete_zoneconf(view, view->newzone.nzconfig, name, - NULL); + NULL, locked); RUNTIME_CHECK(tresult == ISC_R_SUCCESS); } #else /* HAVE_LMDB */ @@ -12973,13 +12977,13 @@ do_modzone(named_server_t *server, dns_view_t *view, dns_name_t *name, dns_zone_t *zone = NULL; const cfg_obj_t *voptions = NULL; bool added; + bool locked = false; #ifndef HAVE_LMDB FILE *fp = NULL; cfg_obj_t *z; #else /* HAVE_LMDB */ MDB_txn *txn = NULL; MDB_dbi dbi; - bool locked = false; #endif /* HAVE_LMDB */ if (!view->newzone.allowed) { @@ -13080,7 +13084,7 @@ do_modzone(named_server_t *server, dns_view_t *view, dns_name_t *name, if (added) { result = delete_zoneconf(view, view->newzone.nzconfig, dns_zone_getorigin(zone), - nzf_writeconf); + nzf_writeconf, locked); if (result != ISC_R_SUCCESS) { TCHECK(putstr(text, "former zone configuration " "not deleted: ")); @@ -13093,13 +13097,14 @@ do_modzone(named_server_t *server, dns_view_t *view, dns_name_t *name, if (!added) { if (view->newzone.vconfig == NULL) { result = delete_zoneconf(view, server->effectiveconfig, - dns_zone_getorigin(zone), - NULL); + dns_zone_getorigin(zone), NULL, + locked); } else { voptions = cfg_tuple_get(server->effectiveconfig, "options"); - result = delete_zoneconf( - view, voptions, dns_zone_getorigin(zone), NULL); + result = delete_zoneconf(view, voptions, + dns_zone_getorigin(zone), NULL, + locked); } if (result != ISC_R_SUCCESS) { @@ -13316,6 +13321,7 @@ rmzone(void *arg) { dns_view_t *view = NULL; dns_db_t *dbp = NULL; bool added; + bool locked = false; isc_result_t result; #ifdef HAVE_LMDB MDB_txn *txn = NULL; @@ -13344,6 +13350,7 @@ rmzone(void *arg) { #ifdef HAVE_LMDB /* Make sure we can open the NZD database */ LOCK(&view->newzone.lock); + locked = true; result = nzd_open(view, 0, &txn, &dbi); if (result != ISC_R_SUCCESS) { isc_log_write(NAMED_LOGCATEGORY_GENERAL, @@ -13365,10 +13372,11 @@ rmzone(void *arg) { (void)nzd_close(&txn, false); } UNLOCK(&view->newzone.lock); + locked = false; #else /* ifdef HAVE_LMDB */ result = delete_zoneconf(view, view->newzone.nzconfig, dns_zone_getorigin(zone), - nzf_writeconf); + nzf_writeconf, locked); if (result != ISC_R_SUCCESS) { isc_log_write(NAMED_LOGCATEGORY_GENERAL, NAMED_LOGMODULE_SERVER, ISC_LOG_ERROR, @@ -13382,12 +13390,13 @@ rmzone(void *arg) { if (view->newzone.vconfig != NULL) { const cfg_obj_t *voptions = cfg_tuple_get(view->newzone.vconfig, "options"); - result = delete_zoneconf( - view, voptions, dns_zone_getorigin(zone), NULL); + result = delete_zoneconf(view, voptions, + dns_zone_getorigin(zone), NULL, + locked); } else { result = delete_zoneconf( view, dz->server->effectiveconfig, - dns_zone_getorigin(zone), NULL); + dns_zone_getorigin(zone), NULL, locked); } if (result != ISC_R_SUCCESS) { From 780872e07eaa735c215765da570d2f1f78bfade7 Mon Sep 17 00:00:00 2001 From: Matthijs Mekking Date: Fri, 13 Mar 2026 11:56:31 +0100 Subject: [PATCH 3/3] Don't call dns_zone_setadded() on modify If we are modifiying the zone, the zone must have been added before. Don't overwrite this value on modifications. Also it feels cleaner to pass added=false to configure_zone() in do_modzone(). --- bin/named/server.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/bin/named/server.c b/bin/named/server.c index 8d857eaa60..dcc2cfe40f 100644 --- a/bin/named/server.c +++ b/bin/named/server.c @@ -6250,7 +6250,9 @@ configure_zone(const cfg_obj_t *config, const cfg_obj_t *zconfig, /* * Mark whether the zone was originally added at runtime or not */ - dns_zone_setadded(zone, added); + if (!modify) { + dns_zone_setadded(zone, added); + } /* * Determine if we need to set up inline signing. @@ -13057,7 +13059,7 @@ do_modzone(named_server_t *server, dns_view_t *view, dns_name_t *name, dns_view_thaw(view); result = configure_zone(server->effectiveconfig, zoneobj, view->newzone.vconfig, view, &server->viewlist, - &server->kasplist, server->aclctx, true, false, + &server->kasplist, server->aclctx, false, false, false, true); dns_view_freeze(view);