[master] further RPZ fixes

4131.	[bug]		Addressed further problems with reloading RPZ
			zones. [RT #39649]

diff --git a/CHANGES b/CHANGES
index d0821d388400656bc79b5e4c382a2665d0d62eaf..64fa271f31e53eaad0684bb69ecf3ad8cc783f1d 100644 (file)
--- a/CHANGES
+++ b/CHANGES
@@ -1,3 +1,6 @@
+4131.  [bug]           Addressed further problems with reloading RPZ
+                       zones. [RT #39649]
+
 4130.  [bug]           The compatability shim for *printf() misprinted some
                        large numbers. [RT #39586]
 

diff --git a/bin/named/query.c b/bin/named/query.c
index b23d7521d8dab36fbdb8c76dc8f28e120a09ed4b..fce9efa7594f718e07f180ea09edb7e9ab8e7b9e 100644 (file)
--- a/bin/named/query.c
+++ b/bin/named/query.c
@@ -24,6 +24,7 @@
 #include <isc/hex.h>
 #include <isc/mem.h>
 #include <isc/print.h>
+#include <isc/rwlock.h>
 #include <isc/serial.h>
 #include <isc/stats.h>
 #include <isc/util.h>
@@ -4246,9 +4247,6 @@ rpz_rrset_find(ns_client_t *client, dns_name_t *name, dns_rdatatype_t type,
 
        CTRACE(ISC_LOG_DEBUG(3), "rpz_rrset_find");
 
-       dns_clientinfomethods_init(&cm, ns_client_sourceip);
-       dns_clientinfo_init(&ci, client, NULL);
-
        st = client->query.rpz_st;
        if ((st->state & DNS_RPZ_RECURSING) != 0) {
                INSIST(st->r.r_type == type);
@@ -4640,19 +4638,6 @@ rpz_rewrite_ip(ns_client_t *client, const isc_netaddr_t *netaddr,
                        break;
                zbits &= (DNS_RPZ_ZMASK(rpz_num) >> 1);
 
-               /*
-                * In case num_zones has changed since zbits was
-                * originally calculated
-                */
-               if (rpz_num >= rpzs->p.num_zones) {
-                       CTRACE(ISC_LOG_ERROR,
-                              "rpz_rewrite_ip: rpz_num is higher than "
-                              "number of zones");
-                       rpz_clean(&p_zone, &p_db, &p_node, p_rdatasetp);
-                       st->m.policy = DNS_RPZ_POLICY_ERROR;
-                       return (DNS_R_SERVFAIL);
-               }
-
                /*
                 * Do not try applying policy zones that cannot replace a
                 * previously found policy zone.
@@ -4973,19 +4958,6 @@ rpz_rewrite_name(ns_client_t *client, dns_name_t *trig_name,
                if ((zbits & 1) == 0)
                        continue;
 
-               /*
-                * In case num_zones has changed since the 'have'
-                * originally calculated
-                */
-               if (rpz_num >= rpzs->p.num_zones) {
-                       CTRACE(ISC_LOG_ERROR,
-                              "rpz_rewrite_name: rpz_num is higher than "
-                              "number of zones");
-                       rpz_clean(&p_zone, &p_db, &p_node, rdatasetp);
-                       st->m.policy = DNS_RPZ_POLICY_ERROR;
-                       return (DNS_R_SERVFAIL);
-               }
-
                /*
                 * Do not check policy zones that cannot replace a previously
                 * found policy.
@@ -5152,24 +5124,27 @@ rpz_rewrite(ns_client_t *client, dns_rdatatype_t qtype,
            (st != NULL && (st->state & DNS_RPZ_REWRITTEN) != 0))
                return (DNS_R_DISALLOWED);
 
-       LOCK(&rpzs->maint_lock);
+       RWLOCK(&rpzs->search_lock, isc_rwlocktype_read);
        if (rpzs->p.num_zones == 0 ||
            (!RECURSIONOK(client) && rpzs->p.no_rd_ok == 0) ||
            !rpz_ck_dnssec(client, qresult, ordataset, osigset))
        {
-               UNLOCK(&rpzs->maint_lock);
+               RWUNLOCK(&rpzs->search_lock, isc_rwlocktype_read);
                return (DNS_R_DISALLOWED);
        }
        have = rpzs->have;
        popt = rpzs->p;
        rpz_ver = rpzs->rpz_ver;
-       UNLOCK(&rpzs->maint_lock);
+       RWUNLOCK(&rpzs->search_lock, isc_rwlocktype_read);
 
        if (st == NULL) {
                st = isc_mem_get(client->mctx, sizeof(*st));
                if (st == NULL)
                        return (ISC_R_NOMEMORY);
                st->state = 0;
+       }
+       if (st->state == 0) {
+               st->state |= DNS_RPZ_ACTIVE;
                memset(&st->m, 0, sizeof(st->m));
                st->m.type = DNS_RPZ_TYPE_BAD;
                st->m.policy = DNS_RPZ_POLICY_MISS;

diff --git a/doc/arm/notes.xml b/doc/arm/notes.xml
index 34951a53f542b25d270aa2354160077fb68d08d4..c141e0c250e5a2ff11bcda4eddb4ca5cfcccd1f9 100644 (file)
--- a/doc/arm/notes.xml
+++ b/doc/arm/notes.xml
@@ -662,6 +662,13 @@
              corrected. [RT #39481]
            </para>
          </listitem>
+         <listitem>
+           <para>
+             The server could crash if a reload of an RPZ zone was
+             initiated while another reload of the same zone was
+             already in progress. [RT #39649]
+           </para>
+         </listitem>
        </itemizedlist>
       </listitem>
     </itemizedlist>

diff --git a/lib/dns/include/dns/rpz.h b/lib/dns/include/dns/rpz.h
index 6205b852961f31693dbaa6895e7a16a994e3c989..cae90cbec9b306d3ccd04745718fc05072675f65 100644 (file)
--- a/lib/dns/include/dns/rpz.h
+++ b/lib/dns/include/dns/rpz.h
@@ -21,11 +21,12 @@
 #define DNS_RPZ_H 1
 
 #include <isc/lang.h>
+#include <isc/refcount.h>
+#include <isc/rwlock.h>
 
 #include <dns/fixedname.h>
 #include <dns/rdata.h>
 #include <dns/types.h>
-#include <isc/refcount.h>
 
 ISC_LANG_BEGINDECLS
 
@@ -227,7 +228,7 @@ struct dns_rpz_zones {
         * A second lock for maintenance that guarantees no other thread
         * is adding or deleting nodes.
         */
-       isc_mutex_t             search_lock;
+       isc_rwlock_t            search_lock;
        isc_mutex_t             maint_lock;
 
        dns_rpz_cidr_node_t     *cidr;
@@ -247,6 +248,7 @@ typedef struct {
 # define DNS_RPZ_DONE_NSDNAME  0x0010  /* NS name missed; checking addresses */
 # define DNS_RPZ_DONE_IPv4     0x0020
 # define DNS_RPZ_RECURSING     0x0040
+# define DNS_RPZ_ACTIVE                0x0080
        /*
         * Best match so far.
         */

diff --git a/lib/dns/include/dns/zone.h b/lib/dns/include/dns/zone.h
index 8bdcf6c19e31c0b80468943006be84bd37083daa..4f44dd8cbd81c085d4f8dd333efd59220104394b 100644 (file)
--- a/lib/dns/include/dns/zone.h
+++ b/lib/dns/include/dns/zone.h
@@ -1653,8 +1653,8 @@ dns_zonemgr_managezone(dns_zonemgr_t *zmgr, dns_zone_t *zone);
 isc_result_t
 dns_zonemgr_forcemaint(dns_zonemgr_t *zmgr);
 /*%<
- * Force zone maintenance of all zones managed by 'zmgr' at its
- * earliest convenience.
+ * Force zone maintenance of all loaded zones managed by 'zmgr'
+ * to take place at the system's earliest convenience.
  */
 
 void

diff --git a/lib/dns/rbtdb.c b/lib/dns/rbtdb.c
index 5f3a33e485624e455a13ed6019f7735502a6f8d2..df67bbc947645840529eaba1b800ee91d724ec9b 100644 (file)
--- a/lib/dns/rbtdb.c
+++ b/lib/dns/rbtdb.c
@@ -1134,8 +1134,10 @@ adjust_quantum(unsigned int old, isc_time_t *start) {
        /* Smooth */
        new = (new + old * 3) / 4;
 
-       isc_log_write(dns_lctx, DNS_LOGCATEGORY_DATABASE, DNS_LOGMODULE_CACHE,
-                     ISC_LOG_DEBUG(1), "adjust_quantum -> %d", new);
+       if (new != old)
+               isc_log_write(dns_lctx, DNS_LOGCATEGORY_DATABASE,
+                             DNS_LOGMODULE_CACHE, ISC_LOG_DEBUG(1),
+                             "adjust_quantum: old=%d, new=%d", old, new);
 
        return (new);
 }

diff --git a/lib/dns/rpz.c b/lib/dns/rpz.c
index a49d362643ec612720fdb78acf5a5815e9093419..d72ca0fb16a3b1e4fb79a91e164daf2bd7fe21af 100644 (file)
--- a/lib/dns/rpz.c
+++ b/lib/dns/rpz.c
@@ -23,6 +23,7 @@
 #include <isc/net.h>
 #include <isc/netaddr.h>
 #include <isc/print.h>
+#include <isc/rwlock.h>
 #include <isc/stdlib.h>
 #include <isc/string.h>
 #include <isc/util.h>
@@ -1390,7 +1391,7 @@ dns_rpz_new_zones(dns_rpz_zones_t **rpzsp, isc_mem_t *mctx) {
                return (ISC_R_NOMEMORY);
        memset(new, 0, sizeof(*new));
 
-       result = isc_mutex_init(&new->search_lock);
+       result = isc_rwlock_init(&new->search_lock, 0, 0);
        if (result != ISC_R_SUCCESS) {
                isc_mem_put(mctx, new, sizeof(*new));
                return (result);
@@ -1398,7 +1399,7 @@ dns_rpz_new_zones(dns_rpz_zones_t **rpzsp, isc_mem_t *mctx) {
 
        result = isc_mutex_init(&new->maint_lock);
        if (result != ISC_R_SUCCESS) {
-               DESTROYLOCK(&new->search_lock);
+               isc_rwlock_destroy(&new->search_lock);
                isc_mem_put(mctx, new, sizeof(*new));
                return (result);
        }
@@ -1406,7 +1407,7 @@ dns_rpz_new_zones(dns_rpz_zones_t **rpzsp, isc_mem_t *mctx) {
        result = isc_refcount_init(&new->refs, 1);
        if (result != ISC_R_SUCCESS) {
                DESTROYLOCK(&new->maint_lock);
-               DESTROYLOCK(&new->search_lock);
+               isc_rwlock_destroy(&new->search_lock);
                isc_mem_put(mctx, new, sizeof(*new));
                return (result);
        }
@@ -1416,7 +1417,7 @@ dns_rpz_new_zones(dns_rpz_zones_t **rpzsp, isc_mem_t *mctx) {
                isc_refcount_decrement(&new->refs, NULL);
                isc_refcount_destroy(&new->refs);
                DESTROYLOCK(&new->maint_lock);
-               DESTROYLOCK(&new->search_lock);
+               isc_rwlock_destroy(&new->search_lock);
                isc_mem_put(mctx, new, sizeof(*new));
                return (result);
        }
@@ -1535,7 +1536,7 @@ dns_rpz_detach_rpzs(dns_rpz_zones_t **rpzsp) {
                cidr_free(rpzs);
                dns_rbt_destroy(&rpzs->rbt);
                DESTROYLOCK(&rpzs->maint_lock);
-               DESTROYLOCK(&rpzs->search_lock);
+               isc_rwlock_destroy(&rpzs->search_lock);
                isc_refcount_destroy(&rpzs->refs);
                isc_mem_putanddetach(&rpzs->mctx, rpzs, sizeof(*rpzs));
        }
@@ -1580,7 +1581,7 @@ dns_rpz_beginload(dns_rpz_zones_t **load_rpzsp,
         */
        tgt = DNS_RPZ_ZBIT(rpz_num);
        LOCK(&rpzs->maint_lock);
-       LOCK(&rpzs->search_lock);
+       RWLOCK(&rpzs->search_lock, isc_rwlocktype_write);
        if ((rpzs->load_begun & tgt) == 0) {
                /*
                 * There is no existing version of the target zone.
@@ -1604,7 +1605,7 @@ dns_rpz_beginload(dns_rpz_zones_t **load_rpzsp,
                isc_refcount_increment(&rpz->refs, NULL);
        }
 
-       UNLOCK(&rpzs->search_lock);
+       RWUNLOCK(&rpzs->search_lock, isc_rwlocktype_write);
        UNLOCK(&rpzs->maint_lock);
 
        return (ISC_R_SUCCESS);
@@ -1612,7 +1613,7 @@ dns_rpz_beginload(dns_rpz_zones_t **load_rpzsp,
 
 /*
  * This function updates "have" bits and also the qname_skip_recurse
- * mask. It must be called when holding rpzs->search_lock.
+ * mask. It must be called when holding a write lock on rpzs->search_lock.
  */
 static void
 fix_triggers(dns_rpz_zones_t *rpzs, dns_rpz_num_t rpz_num) {
@@ -1740,16 +1741,16 @@ dns_rpz_ready(dns_rpz_zones_t *rpzs,
                 * This is a successful initial zone loading, perhaps
                 * for a new instance of a view.
                 */
-               LOCK(&rpzs->search_lock);
+               RWLOCK(&rpzs->search_lock, isc_rwlocktype_write);
                fix_triggers(rpzs, rpz_num);
-               UNLOCK(&rpzs->search_lock);
+               RWUNLOCK(&rpzs->search_lock, isc_rwlocktype_write);
                UNLOCK(&rpzs->maint_lock);
                dns_rpz_detach_rpzs(load_rpzsp);
                return (ISC_R_SUCCESS);
        }
 
        LOCK(&load_rpzs->maint_lock);
-       LOCK(&load_rpzs->search_lock);
+       RWLOCK(&load_rpzs->search_lock, isc_rwlocktype_write);
 
        /*
         * Unless there is only one policy zone, copy the other policy zones
@@ -1848,7 +1849,7 @@ dns_rpz_ready(dns_rpz_zones_t *rpzs,
        /*
         * Exchange the summary databases.
         */
-       LOCK(&rpzs->search_lock);
+       RWLOCK(&rpzs->search_lock, isc_rwlocktype_write);
 
        rpzs->triggers[rpz_num] = load_rpzs->triggers[rpz_num];
        fix_triggers(rpzs, rpz_num);
@@ -1861,13 +1862,13 @@ dns_rpz_ready(dns_rpz_zones_t *rpzs,
        rpzs->rbt = load_rpzs->rbt;
        load_rpzs->rbt = rbt;
 
-       UNLOCK(&rpzs->search_lock);
+       RWUNLOCK(&rpzs->search_lock, isc_rwlocktype_write);
 
        result = ISC_R_SUCCESS;
 
  unlock_and_detach:
        UNLOCK(&rpzs->maint_lock);
-       UNLOCK(&load_rpzs->search_lock);
+       RWUNLOCK(&load_rpzs->search_lock, isc_rwlocktype_write);
        UNLOCK(&load_rpzs->maint_lock);
        dns_rpz_detach_rpzs(load_rpzsp);
        return (result);
@@ -1890,7 +1891,7 @@ dns_rpz_add(dns_rpz_zones_t *rpzs, dns_rpz_num_t rpz_num, dns_name_t *src_name)
        rpz_type = type_from_name(rpz, src_name);
 
        LOCK(&rpzs->maint_lock);
-       LOCK(&rpzs->search_lock);
+       RWLOCK(&rpzs->search_lock, isc_rwlocktype_write);
 
        switch (rpz_type) {
        case DNS_RPZ_TYPE_QNAME:
@@ -1906,7 +1907,7 @@ dns_rpz_add(dns_rpz_zones_t *rpzs, dns_rpz_num_t rpz_num, dns_name_t *src_name)
                break;
        }
 
-       UNLOCK(&rpzs->search_lock);
+       RWUNLOCK(&rpzs->search_lock, isc_rwlocktype_write);
        UNLOCK(&rpzs->maint_lock);
        return (result);
 }
@@ -2096,7 +2097,7 @@ dns_rpz_delete(dns_rpz_zones_t *rpzs, dns_rpz_num_t rpz_num,
        rpz_type = type_from_name(rpz, src_name);
 
        LOCK(&rpzs->maint_lock);
-       LOCK(&rpzs->search_lock);
+       RWLOCK(&rpzs->search_lock, isc_rwlocktype_write);
 
        switch (rpz_type) {
        case DNS_RPZ_TYPE_QNAME:
@@ -2112,7 +2113,7 @@ dns_rpz_delete(dns_rpz_zones_t *rpzs, dns_rpz_num_t rpz_num,
                break;
        }
 
-       UNLOCK(&rpzs->search_lock);
+       RWUNLOCK(&rpzs->search_lock, isc_rwlocktype_write);
        UNLOCK(&rpzs->maint_lock);
 }
 
@@ -2197,13 +2198,13 @@ dns_rpz_find_ip(dns_rpz_zones_t *rpzs, dns_rpz_type_t rpz_type,
                return (DNS_RPZ_INVALID_NUM);
        make_addr_set(&tgt_set, zbits, rpz_type);
 
-       LOCK(&rpzs->search_lock);
+       RWLOCK(&rpzs->search_lock, isc_rwlocktype_read);
        result = search(rpzs, &tgt_ip, 128, &tgt_set, ISC_FALSE, &found);
        if (result == ISC_R_NOTFOUND) {
                /*
                 * There are no eligible zones for this IP address.
                 */
-               UNLOCK(&rpzs->search_lock);
+               RWUNLOCK(&rpzs->search_lock, isc_rwlocktype_read);
                return (DNS_RPZ_INVALID_NUM);
        }
 
@@ -2227,7 +2228,7 @@ dns_rpz_find_ip(dns_rpz_zones_t *rpzs, dns_rpz_type_t rpz_type,
                break;
        }
        result = ip2name(&found->ip, found->prefix, dns_rootname, ip_name);
-       UNLOCK(&rpzs->search_lock);
+       RWUNLOCK(&rpzs->search_lock, isc_rwlocktype_read);
        if (result != ISC_R_SUCCESS) {
                /*
                 * bin/tests/system/rpz/tests.sh looks for "rpz.*failed".
@@ -2260,7 +2261,7 @@ dns_rpz_find_name(dns_rpz_zones_t *rpzs, dns_rpz_type_t rpz_type,
 
        found_zbits = 0;
 
-       LOCK(&rpzs->search_lock);
+       RWLOCK(&rpzs->search_lock, isc_rwlocktype_read);
 
        nmnode = NULL;
        result = dns_rbt_findnode(rpzs->rbt, trig_name, NULL, &nmnode, NULL,
@@ -2304,7 +2305,7 @@ dns_rpz_find_name(dns_rpz_zones_t *rpzs, dns_rpz_type_t rpz_type,
                break;
        }
 
-       UNLOCK(&rpzs->search_lock);
+       RWUNLOCK(&rpzs->search_lock, isc_rwlocktype_read);
        return (zbits & found_zbits);
 }
 

diff --git a/lib/dns/zone.c b/lib/dns/zone.c
index 6c2b05c3b9d72ed4d2aa54f123d32f6105d60a56..7cb3cfcb7a2b762d937581d95182081b365dfcdc 100644 (file)
--- a/lib/dns/zone.c
+++ b/lib/dns/zone.c
@@ -5752,7 +5752,6 @@ dns_zone_setdb(dns_zone_t *zone, dns_db_t *db) {
 /*
  * Co-ordinates the starting of routine jobs.
  */
-
 void
 dns_zone_maintenance(dns_zone_t *zone) {
        const char me[] = "dns_zone_maintenance";
@@ -12296,8 +12295,9 @@ zone_settimer(dns_zone_t *zone, isc_time_t *now) {
        isc_time_t next;
        isc_result_t result;
 
-       ENTER;
        REQUIRE(DNS_ZONE_VALID(zone));
+       ENTER;
+
        if (DNS_ZONE_FLAG(zone, DNS_ZONEFLG_EXITING))
                return;
 
@@ -12361,12 +12361,12 @@ zone_settimer(dns_zone_t *zone, isc_time_t *now) {
                if (!DNS_ZONE_FLAG(zone, DNS_ZONEFLG_REFRESH) &&
                    !DNS_ZONE_FLAG(zone, DNS_ZONEFLG_NOMASTERS) &&
                    !DNS_ZONE_FLAG(zone, DNS_ZONEFLG_NOREFRESH) &&
-                   !DNS_ZONE_FLAG(zone, DNS_ZONEFLG_LOADING)) {
-                       INSIST(!isc_time_isepoch(&zone->refreshtime));
-                       if (isc_time_isepoch(&next) ||
-                           isc_time_compare(&zone->refreshtime, &next) < 0)
-                               next = zone->refreshtime;
-               }
+                   !DNS_ZONE_FLAG(zone, DNS_ZONEFLG_LOADING) &&
+                   !DNS_ZONE_FLAG(zone, DNS_ZONEFLG_LOADPENDING) &&
+                   !isc_time_isepoch(&zone->refreshtime) &&
+                   (isc_time_isepoch(&next) ||
+                    isc_time_compare(&zone->refreshtime, &next) < 0))
+                       next = zone->refreshtime;
                if (DNS_ZONE_FLAG(zone, DNS_ZONEFLG_LOADED) &&
                    !isc_time_isepoch(&zone->expiretime)) {
                        if (isc_time_isepoch(&next) ||

diff --git a/lib/dns/zt.c b/lib/dns/zt.c
index 7a39d0d02277c9ec0833e25fbd9b16712577101c..28e285542f5abeed8992c7da58ce3f9cc212b19d 100644 (file)
--- a/lib/dns/zt.c
+++ b/lib/dns/zt.c
@@ -314,12 +314,15 @@ asyncload(dns_zone_t *zone, void *callback) {
        zt = dns_zone_getview(zone)->zonetable;
        INSIST(VALID_ZT(zt));
 
+       INSIST(zt->references > 0);
+       zt->references++;
+       zt->loads_pending++;
+
        result = dns_zone_asyncload(zone, *loaded, zt);
-       if (result == ISC_R_SUCCESS) {
+       if (result != ISC_R_SUCCESS) {
+               zt->references--;
+               zt->loads_pending--;
                INSIST(zt->references > 0);
-               zt->references++;
-               INSIST(zt->references != 0);
-               zt->loads_pending++;
        }
        return (ISC_R_SUCCESS);
 }