Add a lock for dns_catz_zone_t

Use a lock for the catalog zones during dns__catz_zones_merge() to
avoid races between 'catz' and 'parentcatz'.

(cherry picked from commit 2ae3bc6e1d8c3c38ceeec276f2c5438a11474f2e)

diff --git a/lib/dns/catz.c b/lib/dns/catz.c
index fa5c45f70f18767241dd009bf5bad9d7f6467993..5f642494b198e5c3eb6520ac3f04912d849f515b 100644 (file)
--- a/lib/dns/catz.c
+++ b/lib/dns/catz.c
@@ -101,6 +101,7 @@ struct dns_catz_zone {
        bool broken;
 
        isc_refcount_t references;
+       isc_mutex_t lock;
 };
 
 static void
@@ -460,8 +461,17 @@ dns_catz_zone_resetdefoptions(dns_catz_zone_t *catz) {
        dns_catz_options_init(&catz->defoptions);
 }
 
-isc_result_t
-dns_catz_zones_merge(dns_catz_zone_t *catz, dns_catz_zone_t *newcatz) {
+/*%<
+ * Merge 'newcatz' into 'catz', calling addzone/delzone/modzone
+ * (from catz->catzs->zmm) for appropriate member zones.
+ *
+ * Requires:
+ * \li 'catz' is a valid dns_catz_zone_t.
+ * \li 'newcatz' is a valid dns_catz_zone_t.
+ *
+ */
+static isc_result_t
+dns__catz_zones_merge(dns_catz_zone_t *catz, dns_catz_zone_t *newcatz) {
        isc_result_t result;
        isc_ht_iter_t *iter1 = NULL, *iter2 = NULL;
        isc_ht_iter_t *iteradd = NULL, *itermod = NULL;
@@ -474,6 +484,8 @@ dns_catz_zones_merge(dns_catz_zone_t *catz, dns_catz_zone_t *newcatz) {
        REQUIRE(DNS_CATZ_ZONE_VALID(catz));
        REQUIRE(DNS_CATZ_ZONE_VALID(newcatz));
 
+       LOCK(&catz->lock);
+
        /* TODO verify the new zone first! */
 
        addzone = catz->catzs->zmm->addzone;
@@ -549,12 +561,18 @@ dns_catz_zones_merge(dns_catz_zone_t *catz, dns_catz_zone_t *newcatz) {
                if (zt_find_result == ISC_R_SUCCESS) {
                        dns_catz_coo_t *coo = NULL;
                        char pczname[DNS_NAME_FORMATSIZE];
+                       bool parentcatz_locked = false;
 
                        /*
                         * Change of ownership (coo) processing, if required
                         */
                        parentcatz = dns_zone_get_parentcatz(zone);
-                       if (parentcatz != NULL && parentcatz != catz &&
+                       if (parentcatz != NULL && parentcatz != catz) {
+                               UNLOCK(&catz->lock);
+                               LOCK(&parentcatz->lock);
+                               parentcatz_locked = true;
+                       }
+                       if (parentcatz_locked &&
                            isc_ht_find(parentcatz->coos, nentry->name.ndata,
                                        nentry->name.length,
                                        (void **)&coo) == ISC_R_SUCCESS &&
@@ -581,6 +599,10 @@ dns_catz_zones_merge(dns_catz_zone_t *catz, dns_catz_zone_t *newcatz) {
                                              zname, pczname,
                                              isc_result_totext(result));
                        }
+                       if (parentcatz_locked) {
+                               UNLOCK(&parentcatz->lock);
+                               LOCK(&catz->lock);
+                       }
                }
                if (zt_find_result == ISC_R_SUCCESS ||
                    zt_find_result == DNS_R_PARTIALMATCH)
@@ -742,6 +764,8 @@ dns_catz_zones_merge(dns_catz_zone_t *catz, dns_catz_zone_t *newcatz) {
        isc_ht_destroy(&toadd);
        isc_ht_destroy(&tomod);
 
+       UNLOCK(&catz->lock);
+
        return (result);
 }
 
@@ -817,6 +841,7 @@ dns_catz_new_zone(dns_catz_zones_t *catzs, dns_catz_zone_t **catzp,
                goto cleanup_timer;
        }
 
+       isc_mutex_init(&catz->lock);
        isc_refcount_init(&catz->references, 1);
        isc_ht_init(&catz->entries, catzs->mctx, 4, ISC_HT_CASE_SENSITIVE);
        isc_ht_init(&catz->coos, catzs->mctx, 4, ISC_HT_CASE_INSENSITIVE);
@@ -960,6 +985,8 @@ dns__catz_zone_destroy(dns_catz_zone_t *catz) {
                isc_ht_destroy(&catz->coos);
        }
        catz->magic = 0;
+
+       isc_mutex_destroy(&catz->lock);
        isc_timer_destroy(&catz->updatetimer);
        if (catz->db_registered) {
                dns_db_updatenotify_unregister(
@@ -1752,9 +1779,18 @@ catz_process_value(dns_catz_zone_t *catz, dns_name_t *name,
        }
 }
 
-isc_result_t
-dns_catz_update_process(dns_catz_zone_t *catz, const dns_name_t *src_name,
-                       dns_rdataset_t *rdataset) {
+/*%<
+ * Process a single rdataset from a catalog zone 'catz' update, src_name is the
+ * record name.
+ *
+ * Requires:
+ * \li 'catz' is a valid dns_catz_zone_t.
+ * \li 'src_name' is a valid dns_name_t.
+ * \li 'rdataset' is valid rdataset.
+ */
+static isc_result_t
+dns__catz_update_process(dns_catz_zone_t *catz, const dns_name_t *src_name,
+                        dns_rdataset_t *rdataset) {
        isc_result_t result;
        int order;
        unsigned int nlabels;
@@ -2349,8 +2385,16 @@ dns__catz_update_cb(void *data) {
                                goto next;
                        }
 
-                       result = dns_catz_update_process(newcatz, name,
-                                                        &rdataset);
+                       /*
+                        * Although newcatz->coos is accessed in
+                        * catz_process_coo() in the call-chain below, we don't
+                        * need to hold the newcatz->lock, because the newcatz
+                        * is still local to this thread and function and
+                        * newcatz->coos can't be accessed from the outside
+                        * until dns__catz_zones_merge() has been called.
+                        */
+                       result = dns__catz_update_process(newcatz, name,
+                                                         &rdataset);
                        if (result != ISC_R_SUCCESS) {
                                char typebuf[DNS_RDATATYPE_FORMATSIZE];
                                char classbuf[DNS_RDATACLASS_FORMATSIZE];
@@ -2432,7 +2476,7 @@ final:
        /*
         * Finally merge new zone into old zone.
         */
-       result = dns_catz_zones_merge(oldcatz, newcatz);
+       result = dns__catz_zones_merge(oldcatz, newcatz);
        dns_catz_detach_catz(&newcatz);
        if (result != ISC_R_SUCCESS) {
                isc_log_write(dns_lctx, DNS_LOGCATEGORY_GENERAL,
@@ -2568,7 +2612,7 @@ dns_catz_postreconfig(dns_catz_zones_t *catzs) {
                        result = dns_catz_new_zone(catzs, &newcatz,
                                                   &catz->name);
                        INSIST(result == ISC_R_SUCCESS);
-                       dns_catz_zones_merge(catz, newcatz);
+                       dns__catz_zones_merge(catz, newcatz);
                        dns_catz_detach_catz(&newcatz);
 
                        /* Make sure that we have an empty catalog zone. */

diff --git a/lib/dns/include/dns/catz.h b/lib/dns/include/dns/catz.h
index f9b3b1f722b33b0c2e2300efd5e8a1b8e25e6f8b..2454ed4f663dde9415d30cf91b269f0687dd0fed 100644 (file)
--- a/lib/dns/include/dns/catz.h
+++ b/lib/dns/include/dns/catz.h
@@ -249,31 +249,6 @@ dns_catz_zone_resetdefoptions(dns_catz_zone_t *catz);
  * \li 'catz' is a valid dns_catz_zone_t.
  */
 
-isc_result_t
-dns_catz_zones_merge(dns_catz_zone_t *catz, dns_catz_zone_t *newcatz);
-/*%<
- * Merge 'newcatz' into 'catz', calling addzone/delzone/modzone
- * (from catz->catzs->zmm) for appropriate member zones.
- *
- * Requires:
- * \li 'catz' is a valid dns_catz_zone_t.
- * \li 'newcatz' is a valid dns_catz_zone_t.
- *
- */
-
-isc_result_t
-dns_catz_update_process(dns_catz_zone_t *catz, const dns_name_t *src_name,
-                       dns_rdataset_t *rdataset);
-/*%<
- * Process a single rdataset from a catalog zone 'catz' update, src_name is the
- * record name.
- *
- * Requires:
- * \li 'catz' is a valid dns_catz_zone_t.
- * \li 'src_name' is a valid dns_name_t.
- * \li 'rdataset' is valid rdataset.
- */
-
 isc_result_t
 dns_catz_generate_masterfilename(dns_catz_zone_t *catz, dns_catz_entry_t *entry,
                                 isc_buffer_t **buffer);