81c2ba
From fff2960981a3294ac641968a17558c8d7eecf74d Mon Sep 17 00:00:00 2001
81c2ba
From: Mark Andrews <marka@isc.org>
81c2ba
Date: Wed, 24 Aug 2022 12:21:50 +1000
81c2ba
Subject: [PATCH] Have dns_zt_apply lock the zone table
81c2ba
81c2ba
There where a number of places where the zone table should have
81c2ba
been locked, but wasn't, when dns_zt_apply was called.
81c2ba
81c2ba
Added a isc_rwlocktype_t type parameter to dns_zt_apply and adjusted
81c2ba
all calls to using it.  Removed locks in callers.
81c2ba
81c2ba
Modified upstream commit for v9_11
81c2ba
---
81c2ba
 bin/named/server.c       | 11 ++++++-----
81c2ba
 bin/named/statschannel.c |  8 ++++----
81c2ba
 lib/dns/include/dns/zt.h |  4 ++--
81c2ba
 lib/dns/tests/zt_test.c  |  3 ++-
81c2ba
 lib/dns/view.c           |  3 ++-
81c2ba
 lib/dns/zt.c             | 34 +++++++++++++++++++---------------
81c2ba
 6 files changed, 35 insertions(+), 28 deletions(-)
81c2ba
81c2ba
diff --git a/bin/named/server.c b/bin/named/server.c
81c2ba
index 9826588e6d..0b4b309461 100644
81c2ba
--- a/bin/named/server.c
81c2ba
+++ b/bin/named/server.c
81c2ba
@@ -8723,8 +8723,8 @@ load_configuration(const char *filename, ns_server_t *server,
81c2ba
 		    strcmp(view->name, "_bind") != 0)
81c2ba
 		{
81c2ba
 			dns_view_setviewrevert(view);
81c2ba
-			(void)dns_zt_apply(view->zonetable, false,
81c2ba
-					   removed, view);
81c2ba
+			(void)dns_zt_apply(view->zonetable, isc_rwlocktype_read,
81c2ba
+					   false, removed, view);
81c2ba
 		}
81c2ba
 		dns_view_detach(&view);
81c2ba
 	}
81c2ba
@@ -10090,8 +10090,8 @@ add_view_tolist(struct dumpcontext *dctx, dns_view_t *view) {
81c2ba
 	ISC_LIST_INIT(vle->zonelist);
81c2ba
 	ISC_LIST_APPEND(dctx->viewlist, vle, link);
81c2ba
 	if (dctx->dumpzones)
81c2ba
-		result = dns_zt_apply(view->zonetable, true,
81c2ba
-				      add_zone_tolist, dctx);
81c2ba
+		result = dns_zt_apply(view->zonetable, isc_rwlocktype_read,
81c2ba
+				      true, add_zone_tolist, dctx);
81c2ba
 	return (result);
81c2ba
 }
81c2ba
 
81c2ba
@@ -11367,7 +11367,8 @@ ns_server_sync(ns_server_t *server, isc_lex_t *lex, isc_buffer_t **text) {
81c2ba
 		for (view = ISC_LIST_HEAD(server->viewlist);
81c2ba
 		     view != NULL;
81c2ba
 		     view = ISC_LIST_NEXT(view, link)) {
81c2ba
-			result = dns_zt_apply(view->zonetable, false,
81c2ba
+			result = dns_zt_apply(view->zonetable,
81c2ba
+					      isc_rwlocktype_none, false,
81c2ba
 					      synczone, &cleanup);
81c2ba
 			if (result != ISC_R_SUCCESS &&
81c2ba
 			    tresult == ISC_R_SUCCESS)
81c2ba
diff --git a/bin/named/statschannel.c b/bin/named/statschannel.c
81c2ba
index 12ab048469..9828df0f4e 100644
81c2ba
--- a/bin/named/statschannel.c
81c2ba
+++ b/bin/named/statschannel.c
81c2ba
@@ -1833,8 +1833,8 @@ generatexml(ns_server_t *server, uint32_t flags,
81c2ba
 		if ((flags & STATS_XML_ZONES) != 0) {
81c2ba
 			TRY0(xmlTextWriterStartElement(writer,
81c2ba
 						       ISC_XMLCHAR "zones"));
81c2ba
-			result = dns_zt_apply(view->zonetable, true,
81c2ba
-					      zone_xmlrender, writer);
81c2ba
+			result = dns_zt_apply(view->zonetable, isc_rwlocktype_read,
81c2ba
+					      true, zone_xmlrender, writer);
81c2ba
 			if (result != ISC_R_SUCCESS)
81c2ba
 				goto error;
81c2ba
 			TRY0(xmlTextWriterEndElement(writer)); /* /zones */
81c2ba
@@ -2489,8 +2489,8 @@ generatejson(ns_server_t *server, size_t *msglen,
81c2ba
 			CHECKMEM(za);
81c2ba
 
81c2ba
 			if ((flags & STATS_JSON_ZONES) != 0) {
81c2ba
-				result = dns_zt_apply(view->zonetable, true,
81c2ba
-						      zone_jsonrender, za);
81c2ba
+				result = dns_zt_apply(view->zonetable, isc_rwlocktype_read,
81c2ba
+						      true, zone_jsonrender, za);
81c2ba
 				if (result != ISC_R_SUCCESS) {
81c2ba
 					goto error;
81c2ba
 				}
81c2ba
diff --git a/lib/dns/include/dns/zt.h b/lib/dns/include/dns/zt.h
81c2ba
index e658e5bb67..94212250da 100644
81c2ba
--- a/lib/dns/include/dns/zt.h
81c2ba
+++ b/lib/dns/include/dns/zt.h
81c2ba
@@ -177,11 +177,11 @@ dns_zt_freezezones(dns_zt_t *zt, bool freeze);
81c2ba
  */
81c2ba
 
81c2ba
 isc_result_t
81c2ba
-dns_zt_apply(dns_zt_t *zt, bool stop,
81c2ba
+dns_zt_apply(dns_zt_t *zt, isc_rwlocktype_t lock, bool stop,
81c2ba
 	     isc_result_t (*action)(dns_zone_t *, void *), void *uap);
81c2ba
 
81c2ba
 isc_result_t
81c2ba
-dns_zt_apply2(dns_zt_t *zt, bool stop, isc_result_t *sub,
81c2ba
+dns_zt_apply2(dns_zt_t *zt, isc_rwlocktype_t lock, bool stop, isc_result_t *sub,
81c2ba
 	      isc_result_t (*action)(dns_zone_t *, void *), void *uap);
81c2ba
 /*%<
81c2ba
  * Apply a given 'action' to all zone zones in the table.
81c2ba
diff --git a/lib/dns/tests/zt_test.c b/lib/dns/tests/zt_test.c
81c2ba
index 3f1e812d60..ee75303a50 100644
81c2ba
--- a/lib/dns/tests/zt_test.c
81c2ba
+++ b/lib/dns/tests/zt_test.c
81c2ba
@@ -145,7 +145,8 @@ apply(void **state) {
81c2ba
 	assert_non_null(view->zonetable);
81c2ba
 
81c2ba
 	assert_int_equal(nzones, 0);
81c2ba
-	result = dns_zt_apply(view->zonetable, false, count_zone, &nzones);
81c2ba
+	result = dns_zt_apply2(view->zonetable, isc_rwlocktype_read, false, NULL,
81c2ba
+			      count_zone, &nzones);
81c2ba
 	assert_int_equal(result, ISC_R_SUCCESS);
81c2ba
 	assert_int_equal(nzones, 1);
81c2ba
 
81c2ba
diff --git a/lib/dns/view.c b/lib/dns/view.c
81c2ba
index f01b4dea0f..bd1ced2863 100644
81c2ba
--- a/lib/dns/view.c
81c2ba
+++ b/lib/dns/view.c
81c2ba
@@ -676,7 +676,8 @@ dns_view_dialup(dns_view_t *view) {
81c2ba
 	REQUIRE(DNS_VIEW_VALID(view));
81c2ba
 	REQUIRE(view->zonetable != NULL);
81c2ba
 
81c2ba
-	(void)dns_zt_apply(view->zonetable, false, dialup, NULL);
81c2ba
+	(void)dns_zt_apply2(view->zonetable, isc_rwlocktype_read, false, NULL,
81c2ba
+			   dialup, NULL);
81c2ba
 }
81c2ba
 
81c2ba
 void
81c2ba
diff --git a/lib/dns/zt.c b/lib/dns/zt.c
81c2ba
index 3f12e247e0..af65740325 100644
81c2ba
--- a/lib/dns/zt.c
81c2ba
+++ b/lib/dns/zt.c
81c2ba
@@ -202,7 +202,8 @@ flush(dns_zone_t *zone, void *uap) {
81c2ba
 static void
81c2ba
 zt_destroy(dns_zt_t *zt) {
81c2ba
 	if (zt->flush) {
81c2ba
-		(void)dns_zt_apply(zt, false, flush, NULL);
81c2ba
+		(void)dns_zt_apply(zt, isc_rwlocktype_none,
81c2ba
+				   false, flush, NULL);
81c2ba
 	}
81c2ba
 	isc_refcount_destroy(&zt->references);
81c2ba
 	dns_rbt_destroy(&zt->table);
81c2ba
@@ -249,9 +250,7 @@ dns_zt_load(dns_zt_t *zt, bool stop) {
81c2ba
 
81c2ba
 	REQUIRE(VALID_ZT(zt));
81c2ba
 
81c2ba
-	RWLOCK(&zt->rwlock, isc_rwlocktype_read);
81c2ba
-	result = dns_zt_apply(zt, stop, load, NULL);
81c2ba
-	RWUNLOCK(&zt->rwlock, isc_rwlocktype_read);
81c2ba
+	result = dns_zt_apply2(zt, isc_rwlocktype_read, stop, NULL, load, NULL);
81c2ba
 	return (result);
81c2ba
 }
81c2ba
 
81c2ba
@@ -293,7 +292,7 @@ dns_zt_asyncload2(dns_zt_t *zt, dns_zt_allloaded_t alldone, void *arg,
81c2ba
 	 * Prevent loads_pending going to zero while kicking off the loads.
81c2ba
 	 */
81c2ba
 	zt->loads_pending++;
81c2ba
-	result = dns_zt_apply2(zt, false, NULL, asyncload, &params);
81c2ba
+	result = dns_zt_apply2(zt, isc_rwlocktype_none, false, NULL, asyncload, &params);
81c2ba
 	pending = --zt->loads_pending;
81c2ba
 	if (pending != 0) {
81c2ba
 		zt->loaddone = alldone;
81c2ba
@@ -342,9 +341,7 @@ dns_zt_loadnew(dns_zt_t *zt, bool stop) {
81c2ba
 
81c2ba
 	REQUIRE(VALID_ZT(zt));
81c2ba
 
81c2ba
-	RWLOCK(&zt->rwlock, isc_rwlocktype_read);
81c2ba
-	result = dns_zt_apply(zt, stop, loadnew, NULL);
81c2ba
-	RWUNLOCK(&zt->rwlock, isc_rwlocktype_read);
81c2ba
+	result = dns_zt_apply(zt, isc_rwlocktype_read, stop, loadnew, NULL);
81c2ba
 	return (result);
81c2ba
 }
81c2ba
 
81c2ba
@@ -366,9 +363,7 @@ dns_zt_freezezones(dns_zt_t *zt, bool freeze) {
81c2ba
 
81c2ba
 	REQUIRE(VALID_ZT(zt));
81c2ba
 
81c2ba
-	RWLOCK(&zt->rwlock, isc_rwlocktype_read);
81c2ba
-	result = dns_zt_apply2(zt, false, &tresult, freezezones, &freeze);
81c2ba
-	RWUNLOCK(&zt->rwlock, isc_rwlocktype_read);
81c2ba
+	result = dns_zt_apply2(zt, isc_rwlocktype_read, false, &tresult, freezezones, &freeze);
81c2ba
 	if (tresult == ISC_R_NOTFOUND)
81c2ba
 		tresult = ISC_R_SUCCESS;
81c2ba
 	return ((result == ISC_R_SUCCESS) ? tresult : result);
81c2ba
@@ -490,14 +485,14 @@ dns_zt_setviewrevert(dns_zt_t *zt) {
81c2ba
 }
81c2ba
 
81c2ba
 isc_result_t
81c2ba
-dns_zt_apply(dns_zt_t *zt, bool stop,
81c2ba
+dns_zt_apply(dns_zt_t *zt, isc_rwlocktype_t lock, bool stop,
81c2ba
 	     isc_result_t (*action)(dns_zone_t *, void *), void *uap)
81c2ba
 {
81c2ba
-	return (dns_zt_apply2(zt, stop, NULL, action, uap));
81c2ba
+	return (dns_zt_apply2(zt, lock, stop, NULL, action, uap));
81c2ba
 }
81c2ba
 
81c2ba
 isc_result_t
81c2ba
-dns_zt_apply2(dns_zt_t *zt, bool stop, isc_result_t *sub,
81c2ba
+dns_zt_apply2(dns_zt_t *zt, isc_rwlocktype_t lock, bool stop, isc_result_t *sub,
81c2ba
 	      isc_result_t (*action)(dns_zone_t *, void *), void *uap)
81c2ba
 {
81c2ba
 	dns_rbtnode_t *node;
81c2ba
@@ -508,6 +503,10 @@ dns_zt_apply2(dns_zt_t *zt, bool stop, isc_result_t *sub,
81c2ba
 	REQUIRE(VALID_ZT(zt));
81c2ba
 	REQUIRE(action != NULL);
81c2ba
 
81c2ba
+	if (lock != isc_rwlocktype_none) {
81c2ba
+		RWLOCK(&zt->rwlock, lock);
81c2ba
+	}
81c2ba
+
81c2ba
 	dns_rbtnodechain_init(&chain, zt->mctx);
81c2ba
 	result = dns_rbtnodechain_first(&chain, zt->table, NULL, NULL);
81c2ba
 	if (result == ISC_R_NOTFOUND) {
81c2ba
@@ -538,8 +537,13 @@ dns_zt_apply2(dns_zt_t *zt, bool stop, isc_result_t *sub,
81c2ba
 
81c2ba
  cleanup:
81c2ba
 	dns_rbtnodechain_invalidate(&chain);
81c2ba
-	if (sub != NULL)
81c2ba
+	if (sub != NULL) {
81c2ba
 		*sub = tresult;
81c2ba
+	}
81c2ba
+
81c2ba
+	if (lock != isc_rwlocktype_none) {
81c2ba
+		RWUNLOCK(&zt->rwlock, lock);
81c2ba
+	}
81c2ba
 
81c2ba
 	return (result);
81c2ba
 }
81c2ba
-- 
81c2ba
2.37.2
81c2ba