2238. [bug] It was possible to trigger a REQUIRE when a

                        validation was cancelled. [RT #17106]

diff --git a/CHANGES b/CHANGES
index 71fb6ccf25c5504e033ceb773ec5ca0a90bd090a..28a327df1553aee0674b07349c4bb96af12fd6d7 100644 (file)
--- a/CHANGES
+++ b/CHANGES
@@ -1,3 +1,6 @@
+2238.  [bug]           It was possible to trigger a REQUIRE when a
+                       validation was cancelled. [RT #17106]
+
 2237.  [bug]           libbind: res_init() was not thread aware. [RT #17123]
 
 2236.  [bug]           dnssec-signzone failed to preserve the case of

diff --git a/lib/dns/resolver.c b/lib/dns/resolver.c
index e980cf4fe4dc3ae03bf73ffb5237737c3787f851..2f614c76648c686d7df8ff04e4bcd127047dae2a 100644 (file)
--- a/lib/dns/resolver.c
+++ b/lib/dns/resolver.c
@@ -15,7 +15,7 @@
  * PERFORMANCE OF THIS SOFTWARE.
  */
 
-/* $Id: resolver.c,v 1.284.18.62 2007/08/28 07:20:05 tbox Exp $ */
+/* $Id: resolver.c,v 1.284.18.63 2007/09/14 05:52:49 marka Exp $ */
 
 /*! \file */
 
@@ -194,6 +194,7 @@ struct fetchctx {
        isc_sockaddrlist_t              bad;
        isc_sockaddrlist_t              edns;
        isc_sockaddrlist_t              edns512;
+       dns_validator_t                 *validator;
        ISC_LIST(dns_validator_t)       validators;
        dns_db_t *                      cache;
        dns_adb_t *                     adb;
@@ -398,9 +399,13 @@ valcreate(fetchctx_t *fctx, dns_adbaddrinfo_t *addrinfo, dns_name_t *name,
                                      sigrdataset, fctx->rmessage,
                                      valoptions, task, validated, valarg,
                                      &validator);
-       if (result == ISC_R_SUCCESS)
+       if (result == ISC_R_SUCCESS) {
+               if ((valoptions & DNS_VALIDATOR_DEFER) == 0) {
+                       INSIST(fctx->validator == NULL);
+                       fctx->validator  = validator;
+               }
                ISC_LIST_APPEND(fctx->validators, validator, link);
-       else
+       } else
                isc_mem_put(fctx->res->buckets[fctx->bucketnum].mctx,
                            valarg, sizeof(*valarg));
        return (result);
@@ -2976,6 +2981,7 @@ fctx_create(dns_resolver_t *res, dns_name_t *name, dns_rdatatype_t type,
        ISC_LIST_INIT(fctx->edns);
        ISC_LIST_INIT(fctx->edns512);
        ISC_LIST_INIT(fctx->validators);
+       fctx->validator = NULL;
        fctx->find = NULL;
        fctx->altfind = NULL;
        fctx->pending = 0;
@@ -3314,7 +3320,7 @@ maybe_destroy(fetchctx_t *fctx) {
        unsigned int bucketnum;
        isc_boolean_t bucket_empty = ISC_FALSE;
        dns_resolver_t *res = fctx->res;
-       dns_validator_t *validator;
+       dns_validator_t *validator, *next_validator;
 
        REQUIRE(SHUTTINGDOWN(fctx));
 
@@ -3322,16 +3328,22 @@ maybe_destroy(fetchctx_t *fctx) {
                return;
 
        for (validator = ISC_LIST_HEAD(fctx->validators);
-            validator != NULL;
-            validator = ISC_LIST_HEAD(fctx->validators)) {
-               ISC_LIST_UNLINK(fctx->validators, validator, link);
+            validator != NULL; validator = next_validator) {
+               next_validator = ISC_LIST_NEXT(validator, link);
                dns_validator_cancel(validator);
+               /*
+                * If this is a active validator wait for the cancel
+                * to complete before calling dns_validator_destroy().
+                */
+               if (validator == fctx->validator)
+                       continue;
+               ISC_LIST_UNLINK(fctx->validators, validator, link);
                dns_validator_destroy(&validator);
        }
 
        bucketnum = fctx->bucketnum;
        LOCK(&res->buckets[bucketnum].lock);
-       if (fctx->references == 0)
+       if (fctx->references == 0 && ISC_LIST_EMPTY(fctx->validators))
                bucket_empty = fctx_destroy(fctx);
        UNLOCK(&res->buckets[bucketnum].lock);
 
@@ -3378,6 +3390,7 @@ validated(isc_task_t *task, isc_event_t *event) {
        FCTXTRACE("received validation completion event");
 
        ISC_LIST_UNLINK(fctx->validators, vevent->validator, link);
+       fctx->validator = NULL;
 
        /*
         * Destroy the validator early so that we can
@@ -3463,9 +3476,11 @@ validated(isc_task_t *task, isc_event_t *event) {
                add_bad(fctx, addrinfo, result);
                isc_event_free(&event);
                UNLOCK(&fctx->res->buckets[fctx->bucketnum].lock);
-               if (!ISC_LIST_EMPTY(fctx->validators))
-                       dns_validator_send(ISC_LIST_HEAD(fctx->validators));
-               else if (sentresponse)
+               INSIST(fctx->validator == NULL);
+               fctx->validator = ISC_LIST_HEAD(fctx->validators);
+               if (fctx->validator != NULL) {
+                       dns_validator_send(fctx->validator);
+               } else if (sentresponse)
                        fctx_done(fctx, result);        /* Locks bucket. */
                else
                        fctx_try(fctx);                 /* Locks bucket. */

diff --git a/lib/dns/validator.c b/lib/dns/validator.c
index e9a21f9f2ac95670bdfac4212c41b3d50c0ec4a4..87c183c47a50d45917a96a8c8a343fd9813d7861 100644 (file)
--- a/lib/dns/validator.c
+++ b/lib/dns/validator.c
@@ -15,7 +15,7 @@
  * PERFORMANCE OF THIS SOFTWARE.
  */
 
-/* $Id: validator.c,v 1.119.18.33 2007/08/28 07:20:05 tbox Exp $ */
+/* $Id: validator.c,v 1.119.18.34 2007/09/14 05:52:50 marka Exp $ */
 
 /*! \file */
 
@@ -89,6 +89,7 @@
 #define VALID_VALIDATOR(v)             ISC_MAGIC_VALID(v, VALIDATOR_MAGIC)
 
 #define VALATTR_SHUTDOWN               0x0001  /*%< Shutting down. */
+#define VALATTR_CANCELED               0x0002  /*%< Cancelled. */
 #define VALATTR_TRIEDVERIFY            0x0004  /*%< We have found a key and
                                                 * have attempted a verify. */
 #define VALATTR_INSECURITY             0x0010  /*%< Attempting proveunsecure. */
@@ -114,6 +115,7 @@
 #define DLVTRIED(val) ((val->attributes & VALATTR_DLVTRIED) != 0)
 
 #define SHUTDOWN(v)            (((v)->attributes & VALATTR_SHUTDOWN) != 0)
+#define CANCELED(v)            (((v)->attributes & VALATTR_CANCELED) != 0)
 
 static void
 destroy(dns_validator_t *val);
@@ -280,7 +282,9 @@ fetch_callback_validator(isc_task_t *task, isc_event_t *event) {
 
        validator_log(val, ISC_LOG_DEBUG(3), "in fetch_callback_validator");
        LOCK(&val->lock);
-       if (eresult == ISC_R_SUCCESS) {
+       if (CANCELED(val)) {
+               validator_done(val, ISC_R_CANCELED);
+       } else if (eresult == ISC_R_SUCCESS) {
                validator_log(val, ISC_LOG_DEBUG(3),
                              "keyset with trust %d", rdataset->trust);
                /*
@@ -344,7 +348,9 @@ dsfetched(isc_task_t *task, isc_event_t *event) {
 
        validator_log(val, ISC_LOG_DEBUG(3), "in dsfetched");
        LOCK(&val->lock);
-       if (eresult == ISC_R_SUCCESS) {
+       if (CANCELED(val)) {
+               validator_done(val, ISC_R_CANCELED);
+       } else if (eresult == ISC_R_SUCCESS) {
                validator_log(val, ISC_LOG_DEBUG(3),
                              "dsset with trust %d", rdataset->trust);
                val->dsset = &val->frdataset;
@@ -417,7 +423,9 @@ dsfetched2(isc_task_t *task, isc_event_t *event) {
        validator_log(val, ISC_LOG_DEBUG(3), "in dsfetched2: %s",
                      dns_result_totext(eresult));
        LOCK(&val->lock);
-       if (eresult == DNS_R_NXRRSET || eresult == DNS_R_NCACHENXRRSET) {
+       if (CANCELED(val)) {
+               validator_done(val, ISC_R_CANCELED);
+       } else if (eresult == DNS_R_NXRRSET || eresult == DNS_R_NCACHENXRRSET) {
                /*
                 * There is no DS.  If this is a delegation, we're done.
                 */
@@ -492,7 +500,9 @@ keyvalidated(isc_task_t *task, isc_event_t *event) {
 
        validator_log(val, ISC_LOG_DEBUG(3), "in keyvalidated");
        LOCK(&val->lock);
-       if (eresult == ISC_R_SUCCESS) {
+       if (CANCELED(val)) {
+               validator_done(val, ISC_R_CANCELED);
+       } else if (eresult == ISC_R_SUCCESS) {
                validator_log(val, ISC_LOG_DEBUG(3),
                              "keyset with trust %d", val->frdataset.trust);
                /*
@@ -542,7 +552,9 @@ dsvalidated(isc_task_t *task, isc_event_t *event) {
 
        validator_log(val, ISC_LOG_DEBUG(3), "in dsvalidated");
        LOCK(&val->lock);
-       if (eresult == ISC_R_SUCCESS) {
+       if (CANCELED(val)) {
+               validator_done(val, ISC_R_CANCELED);
+       } else if (eresult == ISC_R_SUCCESS) {
                validator_log(val, ISC_LOG_DEBUG(3),
                              "dsset with trust %d", val->frdataset.trust);
                if ((val->attributes & VALATTR_INSECURITY) != 0)
@@ -751,7 +763,9 @@ authvalidated(isc_task_t *task, isc_event_t *event) {
 
        validator_log(val, ISC_LOG_DEBUG(3), "in authvalidated");
        LOCK(&val->lock);
-       if (result != ISC_R_SUCCESS) {
+       if (CANCELED(val)) {
+               validator_done(val, ISC_R_CANCELED);
+       } else if (result != ISC_R_SUCCESS) {
                validator_log(val, ISC_LOG_DEBUG(3),
                              "authvalidated: got %s",
                              isc_result_totext(result));
@@ -2934,6 +2948,7 @@ dns_validator_cancel(dns_validator_t *validator) {
                        isc_event_free((isc_event_t **)&validator->event);
                        isc_task_detach(&task);
                }
+               validator->attributes |= VALATTR_CANCELED;
        }
        UNLOCK(&validator->lock);
 }