Have dns_zt_apply lock the zone table

There were a number of places where the zone table should have been
locked, but wasn't, when dns_zt_apply was called.

Added a isc_rwlocktype_t type parameter to dns_zt_apply and adjusted
all calls to using it.  Removed locks in callers.

(cherry picked from commit f053d5b41453aa2099030315b8faebd731e7789a)

diff --git a/bin/named/server.c b/bin/named/server.c
index aae666df30412ef0d64aa3e945da669f5405b5f2..57939151101682559a9ce0c41c41f6b36760d229 100644 (file)
--- a/bin/named/server.c
+++ b/bin/named/server.c
@@ -9563,8 +9563,8 @@ cleanup:
                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_zt_apply(view->zonetable, isc_rwlocktype_read,
+                                          false, NULL, removed, view);
                }
                dns_view_detach(&view);
        }
@@ -10952,8 +10952,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_zt_apply(view->zonetable, isc_rwlocktype_read,
+                                     true, NULL, add_zone_tolist, dctx);
        }
        return (result);
 }
@@ -12299,7 +12299,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,
+                       result = dns_zt_apply(view->zonetable,
+                                             isc_rwlocktype_none, false, NULL,
                                              synczone, &cleanup);
                        if (result != ISC_R_SUCCESS && tresult == ISC_R_SUCCESS)
                        {

diff --git a/bin/named/statschannel.c b/bin/named/statschannel.c
index 3c8e28429badf0ef85d93befdfa163d462107cdd..5ff56ec31896686e6d30301fdb9da66ddd2503ad 100644 (file)
--- a/bin/named/statschannel.c
+++ b/bin/named/statschannel.c
@@ -56,6 +56,13 @@
 
 #include "bind9.xsl.h"
 
+#define CHECK(m)                             \
+       do {                                 \
+               result = (m);                \
+               if (result != ISC_R_SUCCESS) \
+                       goto error;          \
+       } while (0)
+
 struct named_statschannel {
        /* Unlocked */
        isc_httpdmgr_t *httpdmgr;
@@ -2298,11 +2305,8 @@ generatexml(named_server_t *server, uint32_t flags, int *buflen,
                if ((flags & STATS_XML_ZONES) != 0) {
                        TRY0(xmlTextWriterStartElement(writer,
                                                       ISC_XMLCHAR "zones"));
-                       result = dns_zt_apply(view->zonetable, true, NULL,
-                                             zone_xmlrender, writer);
-                       if (result != ISC_R_SUCCESS) {
-                               goto error;
-                       }
+                       CHECK(dns_zt_apply(view->zonetable, isc_rwlocktype_read,
+                                          true, NULL, zone_xmlrender, writer));
                        TRY0(xmlTextWriterEndElement(writer)); /* /zones */
                }
 
@@ -2575,13 +2579,6 @@ render_xml_traffic(const char *url, isc_httpdurl_t *urlinfo,
 #define STATS_JSON_TRAFFIC 0x20
 #define STATS_JSON_ALL    0xff
 
-#define CHECK(m)                             \
-       do {                                 \
-               result = (m);                \
-               if (result != ISC_R_SUCCESS) \
-                       goto error;          \
-       } while (0)
-
 #define CHECKMEM(m)                              \
        do {                                     \
                if (m == NULL) {                 \
@@ -3071,12 +3068,9 @@ generatejson(named_server_t *server, size_t *msglen, const char **msg,
                        CHECKMEM(za);
 
                        if ((flags & STATS_JSON_ZONES) != 0) {
-                               result = dns_zt_apply(view->zonetable, true,
-                                                     NULL, zone_jsonrender,
-                                                     za);
-                               if (result != ISC_R_SUCCESS) {
-                                       goto error;
-                               }
+                               CHECK(dns_zt_apply(view->zonetable,
+                                                  isc_rwlocktype_read, true,
+                                                  NULL, zone_jsonrender, za));
                        }
 
                        if (json_object_array_length(za) != 0) {

diff --git a/lib/dns/include/dns/zt.h b/lib/dns/include/dns/zt.h
index c330809fd7b5d53dcf1cc1caa978ccc4e86f20b1..189092bc3b98364226ed0347482fabf8f0f835bc 100644 (file)
--- a/lib/dns/include/dns/zt.h
+++ b/lib/dns/include/dns/zt.h
@@ -170,7 +170,7 @@ dns_zt_freezezones(dns_zt_t *zt, dns_view_t *view, bool freeze);
  */
 
 isc_result_t
-dns_zt_apply(dns_zt_t *zt, bool stop, isc_result_t *sub,
+dns_zt_apply(dns_zt_t *zt, isc_rwlocktype_t lock, bool stop, isc_result_t *sub,
             isc_result_t (*action)(dns_zone_t *, void *), void *uap);
 /*%<
  * Apply a given 'action' to all zone zones in the table.

diff --git a/lib/dns/tests/zt_test.c b/lib/dns/tests/zt_test.c
index aa56216cb1f0dd0b0e222faf352a379929da2502..e79adb2813804de928366f794bf1e787b47638ce 100644 (file)
--- a/lib/dns/tests/zt_test.c
+++ b/lib/dns/tests/zt_test.c
@@ -138,8 +138,8 @@ apply(void **state) {
        assert_non_null(view->zonetable);
 
        assert_int_equal(nzones, 0);
-       result = dns_zt_apply(view->zonetable, false, NULL, count_zone,
-                             &nzones);
+       result = dns_zt_apply(view->zonetable, isc_rwlocktype_read, false, NULL,
+                             count_zone, &nzones);
        assert_int_equal(result, ISC_R_SUCCESS);
        assert_int_equal(nzones, 1);
 

diff --git a/lib/dns/view.c b/lib/dns/view.c
index 8f30f7a87bf7da74c3ace0e561eeeaa51d80d1f7..299528b6cf9fd6abef7c329e2779b32037b6c846 100644 (file)
--- a/lib/dns/view.c
+++ b/lib/dns/view.c
@@ -706,7 +706,8 @@ dns_view_dialup(dns_view_t *view) {
        REQUIRE(DNS_VIEW_VALID(view));
        REQUIRE(view->zonetable != NULL);
 
-       (void)dns_zt_apply(view->zonetable, false, NULL, dialup, NULL);
+       (void)dns_zt_apply(view->zonetable, isc_rwlocktype_read, false, NULL,
+                          dialup, NULL);
 }
 
 void

diff --git a/lib/dns/zt.c b/lib/dns/zt.c
index b51ab6984a0e1929fc7e82081e3a145083960e9f..4b16eab42332ccabbcdb89e7ed2b804aed5bdd76 100644 (file)
--- a/lib/dns/zt.c
+++ b/lib/dns/zt.c
@@ -225,7 +225,8 @@ flush(dns_zone_t *zone, void *uap) {
 static void
 zt_destroy(dns_zt_t *zt) {
        if (atomic_load_acquire(&zt->flush)) {
-               (void)dns_zt_apply(zt, false, NULL, flush, NULL);
+               (void)dns_zt_apply(zt, isc_rwlocktype_none, false, NULL, flush,
+                                  NULL);
        }
        dns_rbt_destroy(&zt->table);
        isc_rwlock_destroy(&zt->rwlock);
@@ -267,9 +268,8 @@ dns_zt_load(dns_zt_t *zt, bool stop, bool newonly) {
        struct zt_load_params params;
        REQUIRE(VALID_ZT(zt));
        params.newonly = newonly;
-       RWLOCK(&zt->rwlock, isc_rwlocktype_read);
-       result = dns_zt_apply(zt, stop, NULL, load, &params);
-       RWUNLOCK(&zt->rwlock, isc_rwlocktype_read);
+       result = dns_zt_apply(zt, isc_rwlocktype_read, stop, NULL, load,
+                             &params);
        return (result);
 }
 
@@ -340,9 +340,8 @@ dns_zt_asyncload(dns_zt_t *zt, bool newonly, dns_zt_allloaded_t alldone,
        zt->loaddone = alldone;
        zt->loaddone_arg = arg;
 
-       RWLOCK(&zt->rwlock, isc_rwlocktype_read);
-       result = dns_zt_apply(zt, false, NULL, asyncload, zt);
-       RWUNLOCK(&zt->rwlock, isc_rwlocktype_read);
+       result = dns_zt_apply(zt, isc_rwlocktype_read, false, NULL, asyncload,
+                             zt);
 
        /*
         * Have all the loads completed?
@@ -388,9 +387,8 @@ dns_zt_freezezones(dns_zt_t *zt, dns_view_t *view, bool freeze) {
 
        REQUIRE(VALID_ZT(zt));
 
-       RWLOCK(&zt->rwlock, isc_rwlocktype_read);
-       result = dns_zt_apply(zt, false, &tresult, freezezones, &params);
-       RWUNLOCK(&zt->rwlock, isc_rwlocktype_read);
+       result = dns_zt_apply(zt, isc_rwlocktype_read, false, &tresult,
+                             freezezones, &params);
        if (tresult == ISC_R_NOTFOUND) {
                tresult = ISC_R_SUCCESS;
        }
@@ -524,7 +522,7 @@ dns_zt_setviewrevert(dns_zt_t *zt) {
 }
 
 isc_result_t
-dns_zt_apply(dns_zt_t *zt, bool stop, isc_result_t *sub,
+dns_zt_apply(dns_zt_t *zt, isc_rwlocktype_t lock, bool stop, isc_result_t *sub,
             isc_result_t (*action)(dns_zone_t *, void *), void *uap) {
        dns_rbtnode_t *node;
        dns_rbtnodechain_t chain;
@@ -534,6 +532,10 @@ dns_zt_apply(dns_zt_t *zt, bool stop, isc_result_t *sub,
        REQUIRE(VALID_ZT(zt));
        REQUIRE(action != NULL);
 
+       if (lock != isc_rwlocktype_none) {
+               RWLOCK(&zt->rwlock, lock);
+       }
+
        dns_rbtnodechain_init(&chain);
        result = dns_rbtnodechain_first(&chain, zt->table, NULL, NULL);
        if (result == ISC_R_NOTFOUND) {
@@ -570,6 +572,10 @@ cleanup:
                *sub = tresult;
        }
 
+       if (lock != isc_rwlocktype_none) {
+               RWUNLOCK(&zt->rwlock, lock);
+       }
+
        return (result);
 }