From e2c3f8059e77a8e11c4378d22e5d8e78b423a28f Mon Sep 17 00:00:00 2001 From: Mark Andrews Date: Fri, 14 Sep 2007 05:43:05 +0000 Subject: [PATCH] 2238. [bug] It was possible to trigger a REQUIRE when a validation was cancelled. [RT #17106] --- CHANGES | 3 +++ lib/dns/resolver.c | 37 ++++++++++++++++++++++++++----------- lib/dns/validator.c | 29 ++++++++++++++++++++++------- 3 files changed, 51 insertions(+), 18 deletions(-) diff --git a/CHANGES b/CHANGES index fe08744cd2..fa921b9f28 100644 --- 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 727b406d73..1cebd5bdb5 100644 --- a/lib/dns/resolver.c +++ b/lib/dns/resolver.c @@ -15,7 +15,7 @@ * PERFORMANCE OF THIS SOFTWARE. */ -/* $Id: resolver.c,v 1.351 2007/09/06 10:00:19 marka Exp $ */ +/* $Id: resolver.c,v 1.352 2007/09/14 05:43:05 marka Exp $ */ /*! \file */ @@ -195,6 +195,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; @@ -426,9 +427,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); @@ -3040,6 +3045,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; @@ -3378,7 +3384,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)); @@ -3386,16 +3392,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); @@ -3442,6 +3454,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 @@ -3527,9 +3540,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 5f11616582..1630576ab6 100644 --- a/lib/dns/validator.c +++ b/lib/dns/validator.c @@ -15,7 +15,7 @@ * PERFORMANCE OF THIS SOFTWARE. */ -/* $Id: validator.c,v 1.153 2007/08/27 04:36:54 marka Exp $ */ +/* $Id: validator.c,v 1.154 2007/09/14 05:43:05 marka Exp $ */ #include @@ -87,6 +87,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. */ @@ -112,6 +113,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); @@ -278,7 +280,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); /* @@ -342,7 +346,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; @@ -415,7 +421,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. */ @@ -490,7 +498,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); /* @@ -540,7 +550,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) @@ -749,7 +761,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)); @@ -2932,6 +2946,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); }