Do not use static variables in lib/dns/zoneverify.c

Make dns_zoneverify_dnssec() eligible for multithreaded use by replacing
the static variables it accesses with a stack-allocated structure
containing these variables.  Implement setup and cleanup routines for
that structure, ensuring no error in these routines causes exit() to be
called any more.  Pass a pointer to that structure to functions
requiring access to variables which were previously static.

diff --git a/lib/dns/zoneverify.c b/lib/dns/zoneverify.c
index 6b45bb7b62807589d95478b1b2f14fc7d7624b76..69ef20fd98462c1f9a2808f0b0c0ca86a6d372ae 100644 (file)
--- a/lib/dns/zoneverify.c
+++ b/lib/dns/zoneverify.c
@@ -51,7 +51,10 @@
        check_result((result == DNS_R_NEWORIGIN) ? ISC_R_SUCCESS : result, \
                     "dns_dbiterator_current()")
 
-static isc_heap_t *expected_chains, *found_chains;
+typedef struct vctx {
+       isc_heap_t *            expected_chains;
+       isc_heap_t *            found_chains;
+} vctx_t;
 
 struct nsec3_chain_fixed {
        isc_uint8_t             hash;
@@ -346,7 +349,7 @@ record_nsec3(const unsigned char *rawhash, const dns_rdata_nsec3_t *nsec3,
 }
 
 static isc_result_t
-match_nsec3(dns_name_t *name, isc_mem_t *mctx,
+match_nsec3(const vctx_t *vctx, dns_name_t *name, isc_mem_t *mctx,
            dns_rdata_nsec3param_t *nsec3param, dns_rdataset_t *rdataset,
            unsigned char types[8192], unsigned int maxtype,
            unsigned char *rawhash, size_t rhsize)
@@ -395,7 +398,7 @@ match_nsec3(dns_name_t *name, isc_mem_t *mctx,
        /*
         * Record chain.
         */
-       result = record_nsec3(rawhash, &nsec3, mctx, expected_chains);
+       result = record_nsec3(rawhash, &nsec3, mctx, vctx->expected_chains);
        check_result(result, "record_nsec3()");
 
        /*
@@ -453,8 +456,8 @@ innsec3params(dns_rdata_nsec3_t *nsec3, dns_rdataset_t *nsec3paramset) {
 }
 
 static isc_result_t
-record_found(dns_db_t *db, dns_dbversion_t *ver, isc_mem_t *mctx,
-            dns_name_t *name, dns_dbnode_t *node,
+record_found(const vctx_t *vctx, dns_db_t *db, dns_dbversion_t *ver,
+            isc_mem_t *mctx, dns_name_t *name, dns_dbnode_t *node,
             dns_rdataset_t *nsec3paramset)
 {
        unsigned char owner[NSEC3_MAX_HASH_LENGTH];
@@ -499,7 +502,7 @@ record_found(dns_db_t *db, dns_dbversion_t *ver, isc_mem_t *mctx,
                /*
                 * Record chain.
                 */
-               result = record_nsec3(owner, &nsec3, mctx, found_chains);
+               result = record_nsec3(owner, &nsec3, mctx, vctx->found_chains);
                check_result(result, "record_nsec3()");
        }
 
@@ -562,9 +565,9 @@ isoptout(dns_db_t *db, dns_dbversion_t *ver, dns_name_t *origin,
 }
 
 static isc_result_t
-verifynsec3(dns_db_t *db, dns_dbversion_t *ver, dns_name_t *origin,
-           isc_mem_t *mctx, dns_name_t *name, dns_rdata_t *rdata,
-           isc_boolean_t delegation, isc_boolean_t empty,
+verifynsec3(const vctx_t *vctx, dns_db_t *db, dns_dbversion_t *ver,
+           dns_name_t *origin, isc_mem_t *mctx, dns_name_t *name,
+           dns_rdata_t *rdata, isc_boolean_t delegation, isc_boolean_t empty,
            unsigned char types[8192], unsigned int maxtype)
 {
        char namebuf[DNS_NAME_FORMATSIZE];
@@ -621,7 +624,7 @@ verifynsec3(dns_db_t *db, dns_dbversion_t *ver, dns_name_t *origin,
        {
                result = ISC_R_SUCCESS;
        } else if (result == ISC_R_SUCCESS) {
-               result = match_nsec3(name, mctx, &nsec3param, &rdataset,
+               result = match_nsec3(vctx, name, mctx, &nsec3param, &rdataset,
                                     types, maxtype, rawhash, rhsize);
        }
 
@@ -634,10 +637,11 @@ verifynsec3(dns_db_t *db, dns_dbversion_t *ver, dns_name_t *origin,
 }
 
 static isc_result_t
-verifynsec3s(dns_db_t *db, dns_dbversion_t *ver, dns_name_t *origin,
-            isc_mem_t *mctx, dns_name_t *name, dns_rdataset_t *nsec3paramset,
-            isc_boolean_t delegation, isc_boolean_t empty,
-            unsigned char types[8192], unsigned int maxtype)
+verifynsec3s(const vctx_t *vctx, dns_db_t *db, dns_dbversion_t *ver,
+            dns_name_t *origin, isc_mem_t *mctx, dns_name_t *name,
+            dns_rdataset_t *nsec3paramset, isc_boolean_t delegation,
+            isc_boolean_t empty, unsigned char types[8192],
+            unsigned int maxtype)
 {
        isc_result_t result;
 
@@ -647,7 +651,7 @@ verifynsec3s(dns_db_t *db, dns_dbversion_t *ver, dns_name_t *origin,
                dns_rdata_t rdata = DNS_RDATA_INIT;
 
                dns_rdataset_current(nsec3paramset, &rdata);
-               result = verifynsec3(db, ver, origin, mctx, name, &rdata,
+               result = verifynsec3(vctx, db, ver, origin, mctx, name, &rdata,
                                     delegation, empty, types, maxtype);
                if (result != ISC_R_SUCCESS)
                        break;
@@ -736,12 +740,12 @@ verifyset(dns_db_t *db, dns_dbversion_t *ver, dns_name_t *origin,
 }
 
 static isc_result_t
-verifynode(dns_db_t *db, dns_dbversion_t *ver, dns_name_t *origin,
-          isc_mem_t *mctx, dns_name_t *name, dns_dbnode_t *node,
-          isc_boolean_t delegation, dns_rdataset_t *keyrdataset,
-          unsigned char *act_algorithms, unsigned char *bad_algorithms,
-          dns_rdataset_t *nsecset, dns_rdataset_t *nsec3paramset,
-          dns_name_t *nextname)
+verifynode(const vctx_t *vctx, dns_db_t *db, dns_dbversion_t *ver,
+          dns_name_t *origin, isc_mem_t *mctx, dns_name_t *name,
+          dns_dbnode_t *node, isc_boolean_t delegation,
+          dns_rdataset_t *keyrdataset, unsigned char *act_algorithms,
+          unsigned char *bad_algorithms, dns_rdataset_t *nsecset,
+          dns_rdataset_t *nsec3paramset, dns_name_t *nextname)
 {
        unsigned char types[8192];
        unsigned int maxtype = 0;
@@ -793,7 +797,7 @@ verifynode(dns_db_t *db, dns_dbversion_t *ver, dns_name_t *origin,
                result = verifynsec(db, ver, name, node, nextname);
 
        if (nsec3paramset != NULL && dns_rdataset_isassociated(nsec3paramset)) {
-               tresult = verifynsec3s(db, ver, origin, mctx, name,
+               tresult = verifynsec3s(vctx, db, ver, origin, mctx, name,
                                       nsec3paramset, delegation, ISC_FALSE,
                                       types, maxtype);
                if (result == ISC_R_SUCCESS && tresult != ISC_R_SUCCESS)
@@ -899,17 +903,17 @@ checknext(const struct nsec3_chain_fixed *first,
 #define EXPECTEDANDFOUND "Expected and found NSEC3 chains not equal\n"
 
 static isc_result_t
-verify_nsec3_chains(isc_mem_t *mctx) {
+verify_nsec3_chains(const vctx_t *vctx, isc_mem_t *mctx) {
        isc_result_t result = ISC_R_SUCCESS;
        struct nsec3_chain_fixed *e, *f = NULL;
        struct nsec3_chain_fixed *first = NULL, *prev = NULL;
 
-       while ((e = isc_heap_element(expected_chains, 1)) != NULL) {
-               isc_heap_delete(expected_chains, 1);
+       while ((e = isc_heap_element(vctx->expected_chains, 1)) != NULL) {
+               isc_heap_delete(vctx->expected_chains, 1);
                if (f == NULL)
-                       f = isc_heap_element(found_chains, 1);
+                       f = isc_heap_element(vctx->found_chains, 1);
                if (f != NULL) {
-                       isc_heap_delete(found_chains, 1);
+                       isc_heap_delete(vctx->found_chains, 1);
 
                        /*
                         * Check that they match.
@@ -926,9 +930,9 @@ verify_nsec3_chains(isc_mem_t *mctx) {
                                 */
                                while (f != NULL && !chain_compare(e, f)) {
                                        free_element(mctx, f);
-                                       f = isc_heap_element(found_chains, 1);
+                                       f = isc_heap_element(vctx->found_chains, 1);
                                        if (f != NULL)
-                                               isc_heap_delete(found_chains, 1);
+                                               isc_heap_delete(vctx->found_chains, 1);
                                        if (f != NULL && chain_equal(e, f)) {
                                                free_element(mctx, f);
                                                f = NULL;
@@ -974,18 +978,19 @@ verify_nsec3_chains(isc_mem_t *mctx) {
                        }
                        free_element(mctx, f);
                }
-               f = isc_heap_element(found_chains, 1);
+               f = isc_heap_element(vctx->found_chains, 1);
                if (f != NULL)
-                       isc_heap_delete(found_chains, 1);
+                       isc_heap_delete(vctx->found_chains, 1);
        } while (f != NULL);
 
        return (result);
 }
 
 static isc_result_t
-verifyemptynodes(dns_db_t *db, dns_dbversion_t *ver, dns_name_t *origin,
-                isc_mem_t *mctx, dns_name_t *name, dns_name_t *prevname,
-                isc_boolean_t isdelegation, dns_rdataset_t *nsec3paramset)
+verifyemptynodes(const vctx_t *vctx, dns_db_t *db, dns_dbversion_t *ver,
+                dns_name_t *origin, isc_mem_t *mctx, dns_name_t *name,
+                dns_name_t *prevname, isc_boolean_t isdelegation,
+                dns_rdataset_t *nsec3paramset)
 {
        dns_namereln_t reln;
        int order;
@@ -1007,8 +1012,9 @@ verifyemptynodes(dns_db_t *db, dns_dbversion_t *ver, dns_name_t *origin,
                                                  &suffix);
                        if (nsec3paramset != NULL &&
                             dns_rdataset_isassociated(nsec3paramset)) {
-                               tresult = verifynsec3s(db, ver, origin, mctx,
-                                                      &suffix, nsec3paramset,
+                               tresult = verifynsec3s(vctx, db, ver, origin,
+                                                      mctx, &suffix,
+                                                      nsec3paramset,
                                                       isdelegation, ISC_TRUE,
                                                       NULL, 0);
                                if (result == ISC_R_SUCCESS &&
@@ -1020,6 +1026,34 @@ verifyemptynodes(dns_db_t *db, dns_dbversion_t *ver, dns_name_t *origin,
        return (result);
 }
 
+static isc_result_t
+vctx_init(vctx_t *vctx, isc_mem_t *mctx) {
+       isc_result_t result;
+
+       vctx->expected_chains = NULL;
+       result = isc_heap_create(mctx, chain_compare, NULL, 1024,
+                                &vctx->expected_chains);
+       if (result != ISC_R_SUCCESS) {
+               return (result);
+       }
+
+       vctx->found_chains = NULL;
+       result = isc_heap_create(mctx, chain_compare, NULL, 1024,
+                                &vctx->found_chains);
+       if (result != ISC_R_SUCCESS) {
+               isc_heap_destroy(&vctx->expected_chains);
+               return (result);
+       }
+
+       return (result);
+}
+
+static void
+vctx_destroy(vctx_t *vctx) {
+       isc_heap_destroy(&vctx->expected_chains);
+       isc_heap_destroy(&vctx->found_chains);
+}
+
 void
 dns_zoneverify_dnssec(dns_db_t *db, dns_dbversion_t *ver, dns_name_t *origin,
                      isc_mem_t *mctx, isc_boolean_t ignore_kskflag,
@@ -1050,13 +1084,12 @@ dns_zoneverify_dnssec(dns_db_t *db, dns_dbversion_t *ver, dns_name_t *origin,
        unsigned char zsk_algorithms[256];
        unsigned char bad_algorithms[256];
        unsigned char act_algorithms[256];
+       vctx_t vctx;
 
-       result = isc_heap_create(mctx, chain_compare, NULL, 1024,
-                                &expected_chains);
-       check_result(result, "isc_heap_create()");
-       result = isc_heap_create(mctx, chain_compare, NULL, 1024,
-                                &found_chains);
-       check_result(result, "isc_heap_create()");
+       result = vctx_init(&vctx, mctx);
+       if (result != ISC_R_SUCCESS) {
+               return;
+       }
 
        result = dns_db_findnode(db, origin, ISC_FALSE, &node);
        if (result != ISC_R_SUCCESS)
@@ -1302,7 +1335,7 @@ dns_zoneverify_dnssec(dns_db_t *db, dns_dbversion_t *ver, dns_name_t *origin,
                } else if (result != ISC_R_SUCCESS)
                        fatal("iterating through the database failed: %s",
                              isc_result_totext(result));
-               result = verifynode(db, ver, origin, mctx, name, node,
+               result = verifynode(&vctx, db, ver, origin, mctx, name, node,
                                    isdelegation, &keyset, act_algorithms,
                                    bad_algorithms, &nsecset, &nsec3paramset,
                                    nextname);
@@ -1311,8 +1344,8 @@ dns_zoneverify_dnssec(dns_db_t *db, dns_dbversion_t *ver, dns_name_t *origin,
                if (vresult == ISC_R_SUCCESS && result != ISC_R_SUCCESS)
                        vresult = result;
                if (prevname != NULL) {
-                       result = verifyemptynodes(db, ver, origin, mctx, name,
-                                                 prevname, isdelegation,
+                       result = verifyemptynodes(&vctx, db, ver, origin, mctx,
+                                                 name, prevname, isdelegation,
                                                  &nsec3paramset);
                } else
                        prevname = dns_fixedname_name(&fprevname);
@@ -1332,11 +1365,11 @@ dns_zoneverify_dnssec(dns_db_t *db, dns_dbversion_t *ver, dns_name_t *origin,
             result = dns_dbiterator_next(dbiter) ) {
                result = dns_dbiterator_current(dbiter, &node, name);
                check_dns_dbiterator_current(result);
-               result = verifynode(db, ver, origin, mctx, name, node,
+               result = verifynode(&vctx, db, ver, origin, mctx, name, node,
                                    ISC_FALSE, &keyset, act_algorithms,
                                    bad_algorithms, NULL, NULL, NULL);
                check_result(result, "verifynode");
-               record_found(db, ver, mctx, name, node, &nsec3paramset);
+               record_found(&vctx, db, ver, mctx, name, node, &nsec3paramset);
                dns_db_detachnode(db, &node);
        }
        dns_dbiterator_destroy(&dbiter);
@@ -1347,13 +1380,11 @@ dns_zoneverify_dnssec(dns_db_t *db, dns_dbversion_t *ver, dns_name_t *origin,
        if (dns_rdataset_isassociated(&nsec3paramset))
                dns_rdataset_disassociate(&nsec3paramset);
 
-       result = verify_nsec3_chains(mctx);
+       result = verify_nsec3_chains(&vctx, mctx);
        if (vresult == ISC_R_UNSET)
                vresult = ISC_R_SUCCESS;
        if (result != ISC_R_SUCCESS && vresult == ISC_R_SUCCESS)
                vresult = result;
-       isc_heap_destroy(&expected_chains);
-       isc_heap_destroy(&found_chains);
 
        /*
         * If we made it this far, we have what we consider a properly signed
@@ -1405,4 +1436,6 @@ dns_zoneverify_dnssec(dns_db_t *db, dns_dbversion_t *ver, dns_name_t *origin,
                        }
                }
        }
+
+       vctx_destroy(&vctx);
 }