Use reference counted counters for nfail and nvalidations

The fetch context that held these values could be freed while there
were still active pointers to the memory.  Using a reference counted
pointer avoids this.

diff --git a/lib/dns/include/dns/validator.h b/lib/dns/include/dns/validator.h
index 5ee6206b736e620b1aba65411b4d878cb6d210d6..51440b3caba07207737aa56de43f5741168b22c2 100644 (file)
--- a/lib/dns/include/dns/validator.h
+++ b/lib/dns/include/dns/validator.h
@@ -151,8 +151,8 @@ struct dns_validator {
        uint8_t        unsupported_digest;
        dns_rdata_t    rdata;
        bool           resume;
-       uint32_t      *nvalidations;
-       uint32_t      *nfails;
+       isc_counter_t *nvalidations;
+       isc_counter_t *nfails;
        isc_counter_t *qc;
        isc_counter_t *gqc;
 
@@ -172,7 +172,7 @@ dns_validator_create(dns_view_t *view, dns_name_t *name, dns_rdatatype_t type,
                     dns_rdataset_t *rdataset, dns_rdataset_t *sigrdataset,
                     dns_message_t *message, unsigned int options,
                     isc_loop_t *loop, isc_job_cb cb, void *arg,
-                    uint32_t *nvalidations, uint32_t *nfails,
+                    isc_counter_t *nvalidations, isc_counter_t *nfails,
                     isc_counter_t *qc, isc_counter_t *gqc,
                     dns_edectx_t *edectx, dns_validator_t **validatorp);
 /*%<

diff --git a/lib/dns/resolver.c b/lib/dns/resolver.c
index 504fc0254fc34ddb2af8f55d41c9aaa500616086..410141f052657f9d830a71f9d4d41b50e0590aeb 100644 (file)
--- a/lib/dns/resolver.c
+++ b/lib/dns/resolver.c
@@ -471,8 +471,8 @@ struct fetchctx {
        unsigned int depth;
        char clientstr[ISC_SOCKADDR_FORMATSIZE];
 
-       uint32_t nvalidations;
-       uint32_t nfails;
+       isc_counter_t *nvalidations;
+       isc_counter_t *nfails;
 };
 
 #define FCTX_MAGIC      ISC_MAGIC('F', '!', '!', '!')
@@ -973,8 +973,8 @@ valcreate(fetchctx_t *fctx, dns_message_t *message, dns_adbaddrinfo_t *addrinfo,
 
        result = dns_validator_create(
                fctx->res->view, name, type, rdataset, sigrdataset, message,
-               valoptions, fctx->loop, validated, valarg, &fctx->nvalidations,
-               &fctx->nfails, fctx->qc, fctx->gqc, &fctx->edectx, &validator);
+               valoptions, fctx->loop, validated, valarg, fctx->nvalidations,
+               fctx->nfails, fctx->qc, fctx->gqc, &fctx->edectx, &validator);
        RUNTIME_CHECK(result == ISC_R_SUCCESS);
        inc_stats(fctx->res, dns_resstatscounter_val);
        ISC_LIST_APPEND(fctx->validators, validator, link);
@@ -4500,6 +4500,12 @@ fctx_destroy(fetchctx_t *fctx) {
                isc_mem_put(fctx->mctx, sa, sizeof(*sa));
        }
 
+       if (fctx->nfails != NULL) {
+               isc_counter_detach(&fctx->nfails);
+       }
+       if (fctx->nvalidations != NULL) {
+               isc_counter_detach(&fctx->nvalidations);
+       }
        isc_counter_detach(&fctx->qc);
        if (fctx->gqc != NULL) {
                isc_counter_detach(&fctx->gqc);
@@ -4669,6 +4675,8 @@ fctx_create(dns_resolver_t *res, isc_loop_t *loop, const dns_name_t *name,
        char buf[DNS_NAME_FORMATSIZE + DNS_RDATATYPE_FORMATSIZE + 1];
        isc_mem_t *mctx = isc_loop_getmctx(loop);
        size_t p;
+       uint32_t nvalidations = atomic_load_relaxed(&res->maxvalidations);
+       uint32_t nfails = atomic_load_relaxed(&res->maxvalidationfails);
 
        /*
         * Caller must be holding the lock for 'bucket'
@@ -4687,8 +4695,6 @@ fctx_create(dns_resolver_t *res, isc_loop_t *loop, const dns_name_t *name,
                .fwdpolicy = dns_fwdpolicy_none,
                .result = ISC_R_FAILURE,
                .loop = loop,
-               .nvalidations = atomic_load_relaxed(&res->maxvalidations),
-               .nfails = atomic_load_relaxed(&res->maxvalidationfails),
        };
 
        isc_mem_attach(mctx, &fctx->mctx);
@@ -4710,6 +4716,14 @@ fctx_create(dns_resolver_t *res, isc_loop_t *loop, const dns_name_t *name,
 
        FCTXTRACE("create");
 
+       if (nfails > 0) {
+               isc_counter_create(mctx, nfails, &fctx->nfails);
+       }
+
+       if (nvalidations > 0) {
+               isc_counter_create(mctx, nvalidations, &fctx->nvalidations);
+       }
+
        if (qc != NULL) {
                isc_counter_attach(qc, &fctx->qc);
                isc_log_write(DNS_LOGCATEGORY_RESOLVER, DNS_LOGMODULE_RESOLVER,
@@ -4942,6 +4956,12 @@ cleanup_nameservers:
                dns_rdataset_disassociate(&fctx->nameservers);
        }
        isc_mem_free(fctx->mctx, fctx->info);
+       if (fctx->nfails != NULL) {
+               isc_counter_detach(&fctx->nfails);
+       }
+       if (fctx->nvalidations != NULL) {
+               isc_counter_detach(&fctx->nvalidations);
+       }
        isc_counter_detach(&fctx->qc);
        if (fctx->gqc != NULL) {
                isc_counter_detach(&fctx->gqc);

diff --git a/lib/dns/validator.c b/lib/dns/validator.c
index a9ea04546394159e1512c1ad75a4704255951b2c..770a549ad0add46036f53a0c3e4ecb9335667d98 100644 (file)
--- a/lib/dns/validator.c
+++ b/lib/dns/validator.c
@@ -1245,7 +1245,10 @@ compute_keytag(dns_rdata_t *rdata) {
 
 static bool
 over_max_validations(dns_validator_t *val) {
-       if (val->nvalidations == NULL || (*val->nvalidations) > 0) {
+       if (val->nvalidations == NULL ||
+           isc_counter_used(val->nvalidations) <
+                   isc_counter_getlimit(val->nvalidations))
+       {
                return false;
        }
 
@@ -1259,14 +1262,14 @@ consume_validation(dns_validator_t *val) {
        if (val->nvalidations == NULL) {
                return;
        }
-       INSIST((*val->nvalidations) > 0);
-
-       (*val->nvalidations)--;
+       (void)isc_counter_increment(val->nvalidations);
 }
 
 static bool
 over_max_fails(dns_validator_t *val) {
-       if (val->nfails == NULL || (*val->nfails) > 0) {
+       if (val->nfails == NULL ||
+           isc_counter_used(val->nfails) < isc_counter_getlimit(val->nfails))
+       {
                return false;
        }
 
@@ -1280,9 +1283,7 @@ consume_validation_fail(dns_validator_t *val) {
        if (val->nfails == NULL) {
                return;
        }
-       INSIST((*val->nfails) > 0);
-
-       (*val->nfails)--;
+       (void)isc_counter_increment(val->nfails);
 }
 
 /*%
@@ -3392,7 +3393,7 @@ dns_validator_create(dns_view_t *view, dns_name_t *name, dns_rdatatype_t type,
                     dns_rdataset_t *rdataset, dns_rdataset_t *sigrdataset,
                     dns_message_t *message, unsigned int options,
                     isc_loop_t *loop, isc_job_cb cb, void *arg,
-                    uint32_t *nvalidations, uint32_t *nfails,
+                    isc_counter_t *nvalidations, isc_counter_t *nfails,
                     isc_counter_t *qc, isc_counter_t *gqc,
                     dns_edectx_t *edectx, dns_validator_t **validatorp) {
        isc_result_t result = ISC_R_FAILURE;
@@ -3424,8 +3425,6 @@ dns_validator_create(dns_view_t *view, dns_name_t *name, dns_rdatatype_t type,
                .cb = cb,
                .arg = arg,
                .rdata = DNS_RDATA_INIT,
-               .nvalidations = nvalidations,
-               .nfails = nfails,
                .edectx = edectx,
        };
 
@@ -3435,6 +3434,14 @@ dns_validator_create(dns_view_t *view, dns_name_t *name, dns_rdatatype_t type,
                dns_message_attach(message, &val->message);
        }
 
+       if (nfails != NULL) {
+               isc_counter_attach(nfails, &val->nfails);
+       }
+
+       if (nvalidations != NULL) {
+               isc_counter_attach(nvalidations, &val->nvalidations);
+       }
+
        if (qc != NULL) {
                isc_counter_attach(qc, &val->qc);
        }
@@ -3527,6 +3534,12 @@ destroy_validator(dns_validator_t *val) {
        if (val->message != NULL) {
                dns_message_detach(&val->message);
        }
+       if (val->nfails != NULL) {
+               isc_counter_detach(&val->nfails);
+       }
+       if (val->nvalidations != NULL) {
+               isc_counter_detach(&val->nvalidations);
+       }
        if (val->qc != NULL) {
                isc_counter_detach(&val->qc);
        }