3314. [bug] The masters list could be updated while refesh_callback

                        and stub_callback were using it. [RT #26732]

diff --git a/CHANGES b/CHANGES
index ea0429bc92ac74e273695aa3407177a73e73d449..012fc01dee84d8f2d95e55434bf82c37f24977f8 100644 (file)
--- a/CHANGES
+++ b/CHANGES
@@ -1,3 +1,6 @@
+3314.  [bug]           The masters list could be updated while refesh_callback
+                       and stub_callback were using it. [RT #26732]
+
 3313.  [protocol]      Add TLSA record type. [RT #28989]
 
 3312.  [bug]           named-checkconf didn't detect a bad dns64 clients acl.

diff --git a/lib/dns/zone.c b/lib/dns/zone.c
index b3ffada5b053e06c05156c916c438494d211d6b0..9b0b1c86b7ef13c3aa3bc08c6c3a39d6259f8d80 100644 (file)
--- a/lib/dns/zone.c
+++ b/lib/dns/zone.c
@@ -455,6 +455,7 @@ struct dns_zonemgr {
        isc_ratelimiter_t *     rl;
        isc_rwlock_t            rwlock;
        isc_mutex_t             iolock;
+       isc_rwlock_t            urlock;
 
        /* Locked by rwlock. */
        dns_zonelist_t          zones;
@@ -472,7 +473,7 @@ struct dns_zonemgr {
        dns_iolist_t            high;
        dns_iolist_t            low;
 
-       /* Locked by rwlock. */
+       /* Locked by urlock. */
        /* LRU cache */
        struct dns_unreachable  unreachable[UNREACH_CHACHE_SIZE];
 };
@@ -2965,7 +2966,8 @@ check_nsec3param(dns_zone_t *zone, dns_db_t *db) {
  */
 static void
 set_refreshkeytimer(dns_zone_t *zone, dns_rdata_keydata_t *key,
-                   isc_stdtime_t now) {
+                   isc_stdtime_t now)
+{
        const char me[] = "set_refreshkeytimer";
        isc_stdtime_t then;
        isc_time_t timenow, timethen;
@@ -9840,6 +9842,8 @@ stub_callback(isc_task_t *task, isc_event_t *event) {
 
        TIME_NOW(&now);
 
+       LOCK_ZONE(zone);
+
        if (DNS_ZONE_FLAG(zone, DNS_ZONEFLG_EXITING)) {
                zone_debuglog(zone, me, 1, "exiting");
                exiting = ISC_TRUE;
@@ -9852,9 +9856,7 @@ stub_callback(isc_task_t *task, isc_event_t *event) {
        if (revent->result != ISC_R_SUCCESS) {
                if (revent->result == ISC_R_TIMEDOUT &&
                    !DNS_ZONE_FLAG(zone, DNS_ZONEFLG_NOEDNS)) {
-                       LOCK_ZONE(zone);
                        DNS_ZONE_SETFLAG(zone, DNS_ZONEFLG_NOEDNS);
-                       UNLOCK_ZONE(zone);
                        dns_zone_log(zone, ISC_LOG_DEBUG(1),
                                     "refreshing stub: timeout retrying "
                                     " without EDNS master %s (source %s)",
@@ -9896,9 +9898,7 @@ stub_callback(isc_task_t *task, isc_event_t *event) {
                                     "refreshing stub: rcode (%.*s) retrying "
                                     "without EDNS master %s (source %s)",
                                     (int)rb.used, rcode, master, source);
-                       LOCK_ZONE(zone);
                        DNS_ZONE_SETFLAG(zone, DNS_ZONEFLG_NOEDNS);
-                       UNLOCK_ZONE(zone);
                        goto same_master;
                }
 
@@ -9920,9 +9920,7 @@ stub_callback(isc_task_t *task, isc_event_t *event) {
                                     master, source);
                        goto next_master;
                }
-               LOCK_ZONE(zone);
                DNS_ZONE_SETFLAG(zone, DNS_ZONEFLG_USEVC);
-               UNLOCK_ZONE(zone);
                goto same_master;
        }
 
@@ -9977,21 +9975,17 @@ stub_callback(isc_task_t *task, isc_event_t *event) {
        ZONEDB_UNLOCK(&zone->dblock, isc_rwlocktype_write);
        dns_db_detach(&stub->db);
 
-       if (zone->masterfile != NULL) {
-               dns_zone_dump(zone);
-               TIME_NOW(&zone->loadtime);
-       }
+       if (zone->masterfile != NULL)
+               zone_needdump(zone, 0);
 
        dns_message_destroy(&msg);
        isc_event_free(&event);
-       LOCK_ZONE(zone);
        dns_request_destroy(&zone->request);
        DNS_ZONE_CLRFLAG(zone, DNS_ZONEFLG_REFRESH);
        DNS_ZONE_JITTER_ADD(&now, zone->refresh, &zone->refreshtime);
        isc_interval_set(&i, zone->expire, 0);
        DNS_ZONE_TIME_ADD(&now, zone->expire, &zone->expiretime);
        zone_settimer(zone, &now);
-       UNLOCK_ZONE(zone);
        goto free_stub;
 
  next_master:
@@ -10002,7 +9996,6 @@ stub_callback(isc_task_t *task, isc_event_t *event) {
        if (msg != NULL)
                dns_message_destroy(&msg);
        isc_event_free(&event);
-       LOCK_ZONE(zone);
        dns_request_destroy(&zone->request);
        /*
         * Skip to next failed / untried master.
@@ -10040,25 +10033,23 @@ stub_callback(isc_task_t *task, isc_event_t *event) {
                        DNS_ZONE_CLRFLAG(zone, DNS_ZONEFLG_REFRESH);
 
                        zone_settimer(zone, &now);
-                       UNLOCK_ZONE(zone);
                        goto free_stub;
                }
        }
        queue_soa_query(zone);
-       UNLOCK_ZONE(zone);
        goto free_stub;
 
  same_master:
        if (msg != NULL)
                dns_message_destroy(&msg);
        isc_event_free(&event);
-       LOCK_ZONE(zone);
        dns_request_destroy(&zone->request);
-       UNLOCK_ZONE(zone);
        ns_query(zone, NULL, stub);
+       UNLOCK_ZONE(zone);
        goto done;
 
  free_stub:
+       UNLOCK_ZONE(zone);
        stub->magic = 0;
        dns_zone_idetach(&stub->zone);
        INSIST(stub->db == NULL);
@@ -10089,6 +10080,7 @@ refresh_callback(isc_task_t *task, isc_event_t *event) {
        isc_result_t result;
        isc_uint32_t serial, oldserial = 0;
        unsigned int j;
+       isc_boolean_t do_queue_xfrin = ISC_FALSE;
 
        zone = revent->ev_arg;
        INSIST(DNS_ZONE_VALID(zone));
@@ -10097,6 +10089,10 @@ refresh_callback(isc_task_t *task, isc_event_t *event) {
 
        ENTER;
 
+       TIME_NOW(&now);
+
+       LOCK_ZONE(zone);
+
        /*
         * if timeout log and next master;
         */
@@ -10104,14 +10100,10 @@ refresh_callback(isc_task_t *task, isc_event_t *event) {
        isc_sockaddr_format(&zone->masteraddr, master, sizeof(master));
        isc_sockaddr_format(&zone->sourceaddr, source, sizeof(source));
 
-       TIME_NOW(&now);
-
        if (revent->result != ISC_R_SUCCESS) {
                if (revent->result == ISC_R_TIMEDOUT &&
                    !DNS_ZONE_FLAG(zone, DNS_ZONEFLG_NOEDNS)) {
-                       LOCK_ZONE(zone);
                        DNS_ZONE_SETFLAG(zone, DNS_ZONEFLG_NOEDNS);
-                       UNLOCK_ZONE(zone);
                        dns_zone_log(zone, ISC_LOG_DEBUG(1),
                                     "refresh: timeout retrying without EDNS "
                                     "master %s (source %s)", master, source);
@@ -10132,10 +10124,8 @@ refresh_callback(isc_task_t *task, isc_event_t *event) {
                                                             &zone->sourceaddr,
                                                             &now))
                                {
-                                       LOCK_ZONE(zone);
                                        DNS_ZONE_SETFLAG(zone,
                                                     DNS_ZONEFLG_SOABEFOREAXFR);
-                                       UNLOCK_ZONE(zone);
                                        goto tcp_transfer;
                                }
                                dns_zone_log(zone, ISC_LOG_DEBUG(1),
@@ -10182,9 +10172,7 @@ refresh_callback(isc_task_t *task, isc_event_t *event) {
                                     "refresh: rcode (%.*s) retrying without "
                                     "EDNS master %s (source %s)",
                                     (int)rb.used, rcode, master, source);
-                       LOCK_ZONE(zone);
                        DNS_ZONE_SETFLAG(zone, DNS_ZONEFLG_NOEDNS);
-                       UNLOCK_ZONE(zone);
                        goto same_master;
                }
                dns_zone_log(zone, ISC_LOG_INFO,
@@ -10212,9 +10200,7 @@ refresh_callback(isc_task_t *task, isc_event_t *event) {
                                     "initiating TCP zone xfer "
                                     "for master %s (source %s)",
                                     master, source);
-                       LOCK_ZONE(zone);
                        DNS_ZONE_SETFLAG(zone, DNS_ZONEFLG_SOABEFOREAXFR);
-                       UNLOCK_ZONE(zone);
                        goto tcp_transfer;
                } else {
                        INSIST(zone->type == dns_zone_stub);
@@ -10225,9 +10211,7 @@ refresh_callback(isc_task_t *task, isc_event_t *event) {
                                             master, source);
                                goto next_master;
                        }
-                       LOCK_ZONE(zone);
                        DNS_ZONE_SETFLAG(zone, DNS_ZONEFLG_USEVC);
-                       UNLOCK_ZONE(zone);
                        goto same_master;
                }
        }
@@ -10288,6 +10272,7 @@ refresh_callback(isc_task_t *task, isc_event_t *event) {
                             soacnt, master, source);
                goto next_master;
        }
+
        /*
         * Extract serial
         */
@@ -10315,7 +10300,9 @@ refresh_callback(isc_task_t *task, isc_event_t *event) {
 
        serial = soa.serial;
        if (DNS_ZONE_FLAG(zone, DNS_ZONEFLG_LOADED)) {
-               result = dns_zone_getserial2(zone, &oldserial);
+               result = zone_get_from_db(zone, zone->db, NULL, NULL,
+                                         &oldserial, NULL, NULL, NULL, NULL,
+                                         NULL);
                RUNTIME_CHECK(result == ISC_R_SUCCESS);
                zone_debuglog(zone, me, 1, "serial: new %u, old %u",
                              serial, oldserial);
@@ -10340,12 +10327,10 @@ refresh_callback(isc_task_t *task, isc_event_t *event) {
                }
  tcp_transfer:
                isc_event_free(&event);
-               LOCK_ZONE(zone);
                dns_request_destroy(&zone->request);
-               UNLOCK_ZONE(zone);
                if (zone->type == dns_zone_slave ||
                    zone->type == dns_zone_redirect) {
-                       queue_xfrin(zone);
+                       do_queue_xfrin = TRUE;
                } else {
                        INSIST(zone->type == dns_zone_stub);
                        ns_query(zone, rdataset, NULL);
@@ -10367,9 +10352,7 @@ refresh_callback(isc_task_t *task, isc_event_t *event) {
                                                          &now);
                        /* Someone removed the file from underneath us! */
                        if (result == ISC_R_FILENOTFOUND) {
-                               LOCK_ZONE(zone);
                                zone_needdump(zone, DNS_DUMP_DELAY);
-                               UNLOCK_ZONE(zone);
                        } else if (result != ISC_R_SUCCESS)
                                dns_zone_log(zone, ISC_LOG_ERROR,
                                             "refresh: could not set file "
@@ -10399,7 +10382,6 @@ refresh_callback(isc_task_t *task, isc_event_t *event) {
        if (msg != NULL)
                dns_message_destroy(&msg);
        isc_event_free(&event);
-       LOCK_ZONE(zone);
        dns_request_destroy(&zone->request);
        /*
         * Skip to next failed / untried master.
@@ -10441,25 +10423,24 @@ refresh_callback(isc_task_t *task, isc_event_t *event) {
                }
                DNS_ZONE_CLRFLAG(zone, DNS_ZONEFLG_USEALTXFRSRC);
                zone_settimer(zone, &now);
-               UNLOCK_ZONE(zone);
                goto detach;
        }
 
  requeue:
        queue_soa_query(zone);
-       UNLOCK_ZONE(zone);
        goto detach;
 
  same_master:
        if (msg != NULL)
                dns_message_destroy(&msg);
        isc_event_free(&event);
-       LOCK_ZONE(zone);
        dns_request_destroy(&zone->request);
        queue_soa_query(zone);
-       UNLOCK_ZONE(zone);
 
  detach:
+       UNLOCK_ZONE(zone);
+       if (do_queue_xfrin)
+               queue_xfrin(zone);
        dns_zone_idetach(&zone);
        return;
 }
@@ -10817,13 +10798,13 @@ ns_query(dns_zone_t *zone, dns_rdataset_t *soardataset, dns_stub_t *stub) {
        isc_uint16_t udpsize = SEND_BUFFER_SIZE;
 
        REQUIRE(DNS_ZONE_VALID(zone));
+       REQUIRE(LOCKED_ZONE(zone));
        REQUIRE((soardataset != NULL && stub == NULL) ||
                (soardataset == NULL && stub != NULL));
        REQUIRE(stub == NULL || DNS_STUB_VALID(stub));
 
        ENTER;
 
-       LOCK_ZONE(zone);
        if (stub == NULL) {
                stub = isc_mem_get(zone->mctx, sizeof(*stub));
                if (stub == NULL)
@@ -11014,10 +10995,9 @@ ns_query(dns_zone_t *zone, dns_rdataset_t *soardataset, dns_stub_t *stub) {
        }
        if (message != NULL)
                dns_message_destroy(&message);
-  unlock:
+ unlock:
        if (key != NULL)
                dns_tsigkey_detach(&key);
-       UNLOCK_ZONE(zone);
        return;
 }
 
@@ -13758,15 +13738,22 @@ dns_zonemgr_create(isc_mem_t *mctx, isc_taskmgr_t *taskmgr,
        zmgr->transfersin = 10;
        zmgr->transfersperns = 2;
 
+       /* Unreachable lock. */
+       result = isc_rwlock_init(&zmgr->urlock, 0, 0);
+       if (result != ISC_R_SUCCESS)
+               goto free_rwlock;
+
        /* Create a single task for queueing of SOA queries. */
        result = isc_task_create(taskmgr, 1, &zmgr->task);
        if (result != ISC_R_SUCCESS)
-               goto free_rwlock;
+               goto free_urlock;
+
        isc_task_setname(zmgr->task, "zmgr", zmgr);
        result = isc_ratelimiter_create(mctx, timermgr, zmgr->task,
                                        &zmgr->rl);
        if (result != ISC_R_SUCCESS)
                goto free_task;
+
        /* default to 20 refresh queries / notifies per second. */
        isc_interval_set(&interval, 0, 1000000000/2);
        result = isc_ratelimiter_setinterval(zmgr->rl, &interval);
@@ -13795,6 +13782,8 @@ dns_zonemgr_create(isc_mem_t *mctx, isc_taskmgr_t *taskmgr,
        isc_ratelimiter_detach(&zmgr->rl);
  free_task:
        isc_task_detach(&zmgr->task);
+ free_urlock:
+       isc_rwlock_destroy(&zmgr->urlock);
  free_rwlock:
        isc_rwlock_destroy(&zmgr->rwlock);
  free_mem:
@@ -13980,7 +13969,6 @@ dns_zonemgr_shutdown(dns_zonemgr_t *zmgr) {
                UNLOCK_ZONE(zone);
        }
        RWUNLOCK(&zmgr->rwlock, isc_rwlocktype_read);
-
 }
 
 isc_result_t
@@ -14051,6 +14039,7 @@ zonemgr_free(dns_zonemgr_t *zmgr) {
        DESTROYLOCK(&zmgr->iolock);
        isc_ratelimiter_detach(&zmgr->rl);
 
+       isc_rwlock_destroy(&zmgr->urlock);
        isc_rwlock_destroy(&zmgr->rwlock);
        mctx = zmgr->mctx;
        isc_mem_put(zmgr->mctx, zmgr, sizeof(*zmgr));
@@ -14464,12 +14453,12 @@ dns_zonemgr_unreachable(dns_zonemgr_t *zmgr, isc_sockaddr_t *remote,
        REQUIRE(DNS_ZONEMGR_VALID(zmgr));
 
        locktype = isc_rwlocktype_read;
-       RWLOCK(&zmgr->rwlock, locktype);
+       RWLOCK(&zmgr->urlock, locktype);
        for (i = 0; i < UNREACH_CHACHE_SIZE; i++) {
                if (zmgr->unreachable[i].expire >= seconds &&
                    isc_sockaddr_equal(&zmgr->unreachable[i].remote, remote) &&
                    isc_sockaddr_equal(&zmgr->unreachable[i].local, local)) {
-                       result = isc_rwlock_tryupgrade(&zmgr->rwlock);
+                       result = isc_rwlock_tryupgrade(&zmgr->urlock);
                        if (result == ISC_R_SUCCESS) {
                                locktype = isc_rwlocktype_write;
                                zmgr->unreachable[i].last = seconds;
@@ -14477,7 +14466,7 @@ dns_zonemgr_unreachable(dns_zonemgr_t *zmgr, isc_sockaddr_t *remote,
                        break;
                }
        }
-       RWUNLOCK(&zmgr->rwlock, locktype);
+       RWUNLOCK(&zmgr->urlock, locktype);
        return (ISC_TF(i < UNREACH_CHACHE_SIZE));
 }
 
@@ -14498,11 +14487,11 @@ dns_zonemgr_unreachabledel(dns_zonemgr_t *zmgr, isc_sockaddr_t *remote,
        REQUIRE(DNS_ZONEMGR_VALID(zmgr));
 
        locktype = isc_rwlocktype_read;
-       RWLOCK(&zmgr->rwlock, locktype);
+       RWLOCK(&zmgr->urlock, locktype);
        for (i = 0; i < UNREACH_CHACHE_SIZE; i++) {
                if (isc_sockaddr_equal(&zmgr->unreachable[i].remote, remote) &&
                    isc_sockaddr_equal(&zmgr->unreachable[i].local, local)) {
-                       result = isc_rwlock_tryupgrade(&zmgr->rwlock);
+                       result = isc_rwlock_tryupgrade(&zmgr->urlock);
                        if (result == ISC_R_SUCCESS) {
                                locktype = isc_rwlocktype_write;
                                zmgr->unreachable[i].expire = 0;
@@ -14515,7 +14504,7 @@ dns_zonemgr_unreachabledel(dns_zonemgr_t *zmgr, isc_sockaddr_t *remote,
                        break;
                }
        }
-       RWUNLOCK(&zmgr->rwlock, locktype);
+       RWUNLOCK(&zmgr->urlock, locktype);
 }
 
 void
@@ -14528,7 +14517,7 @@ dns_zonemgr_unreachableadd(dns_zonemgr_t *zmgr, isc_sockaddr_t *remote,
 
        REQUIRE(DNS_ZONEMGR_VALID(zmgr));
 
-       RWLOCK(&zmgr->rwlock, isc_rwlocktype_write);
+       RWLOCK(&zmgr->urlock, isc_rwlocktype_write);
        for (i = 0; i < UNREACH_CHACHE_SIZE; i++) {
                /* Existing entry? */
                if (isc_sockaddr_equal(&zmgr->unreachable[i].remote, remote) &&
@@ -14567,7 +14556,7 @@ dns_zonemgr_unreachableadd(dns_zonemgr_t *zmgr, isc_sockaddr_t *remote,
                zmgr->unreachable[oldest].remote = *remote;
                zmgr->unreachable[oldest].local = *local;
        }
-       RWUNLOCK(&zmgr->rwlock, isc_rwlocktype_write);
+       RWUNLOCK(&zmgr->urlock, isc_rwlocktype_write);
 }
 
 void