mir/bind
2
0
Fork 0
mirror of https://gitlab.isc.org/isc-projects/bind9 synced 2025-08-31 14:35:26 +00:00
Code Issues Releases Wiki Activity

3241. [bug] Address race conditions in the resolver code.

Browse Source 
[RT #26889]
This commit is contained in:
Mark Andrews
2011-12-07 23:08:42 +00:00
parent 71e4c3ee74
commit e4aac0596c
2 changed files with 66 additions and 50 deletions
Download Patch File Download Diff File Expand all files Collapse all files

3
CHANGES
View File

@@ -1,3 +1,6 @@
3241. [bug] Address race conditions in the resolver code.
[RT #26889]
3240. [bug] DNSKEY state change events could be missed. [RT #26874] 3240. [bug] DNSKEY state change events could be missed. [RT #26874]
3239. [bug] dns_dnssec_findmatchingkeys needs to use a consistent 3239. [bug] dns_dnssec_findmatchingkeys needs to use a consistent

113
lib/dns/resolver.c
View File

@@ -15,7 +15,7 @@
* PERFORMANCE OF THIS SOFTWARE. * PERFORMANCE OF THIS SOFTWARE.
*/ */
/* $Id: resolver.c,v 1.445 2011/12/05 17:27:16 each Exp $ */ /* $Id: resolver.c,v 1.446 2011/12/07 23:08:42 marka Exp $ */
/*! \file */ /*! \file */
@@ -453,7 +453,7 @@ static isc_result_t ncache_adderesult(dns_message_t *message,
dns_rdataset_t *ardataset, dns_rdataset_t *ardataset,
isc_result_t *eresultp); isc_result_t *eresultp);
static void validated(isc_task_t *task, isc_event_t *event); static void validated(isc_task_t *task, isc_event_t *event);
static void maybe_destroy(fetchctx_t *fctx, isc_boolean_t locked); static isc_boolean_t maybe_destroy(fetchctx_t *fctx, isc_boolean_t locked);
static void add_bad(fetchctx_t *fctx, dns_adbaddrinfo_t *addrinfo, static void add_bad(fetchctx_t *fctx, dns_adbaddrinfo_t *addrinfo,
isc_result_t reason, badnstype_t badtype); isc_result_t reason, badnstype_t badtype);
@@ -746,8 +746,11 @@ resquery_destroy(resquery_t **queryp) {
INSIST(query->tcpsocket == NULL); INSIST(query->tcpsocket == NULL);
query->fctx->nqueries--; query->fctx->nqueries--;
if (SHUTTINGDOWN(query->fctx)) if (SHUTTINGDOWN(query->fctx)) {
maybe_destroy(query->fctx, ISC_FALSE); /* Locks bucket. */ dns_resolver_t *res = query->fctx->res;
if (maybe_destroy(query->fctx, ISC_FALSE))
empty_bucket(res);
}
query->magic = 0; query->magic = 0;
isc_mem_put(query->mctx, query, sizeof(*query)); isc_mem_put(query->mctx, query, sizeof(*query));
*queryp = NULL; *queryp = NULL;
@@ -2161,6 +2164,7 @@ fctx_finddone(isc_task_t *task, isc_event_t *event) {
isc_boolean_t want_try = ISC_FALSE; isc_boolean_t want_try = ISC_FALSE;
isc_boolean_t want_done = ISC_FALSE; isc_boolean_t want_done = ISC_FALSE;
isc_boolean_t bucket_empty = ISC_FALSE; isc_boolean_t bucket_empty = ISC_FALSE;
isc_boolean_t destroy = ISC_FALSE;
unsigned int bucketnum; unsigned int bucketnum;
find = event->ev_sender; find = event->ev_sender;
@@ -2172,6 +2176,9 @@ fctx_finddone(isc_task_t *task, isc_event_t *event) {
FCTXTRACE("finddone"); FCTXTRACE("finddone");
bucketnum = fctx->bucketnum;
LOCK(&res->buckets[bucketnum].lock);
INSIST(fctx->pending > 0); INSIST(fctx->pending > 0);
fctx->pending--; fctx->pending--;
@@ -2196,17 +2203,17 @@ fctx_finddone(isc_task_t *task, isc_event_t *event) {
} }
} else if (SHUTTINGDOWN(fctx) && fctx->pending == 0 && } else if (SHUTTINGDOWN(fctx) && fctx->pending == 0 &&
fctx->nqueries == 0 && ISC_LIST_EMPTY(fctx->validators)) { fctx->nqueries == 0 && ISC_LIST_EMPTY(fctx->validators)) {
bucketnum = fctx->bucketnum;
LOCK(&res->buckets[bucketnum].lock);
/* /*
* Note that we had to wait until we had the lock before * Note that we had to wait until we had the lock before
* looking at fctx->references. * looking at fctx->references.
*/ */
if (fctx->references == 0) if (fctx->references == 0)
bucket_empty = fctx_destroy(fctx); destroy = ISC_TRUE;
UNLOCK(&res->buckets[bucketnum].lock);
} }
UNLOCK(&res->buckets[bucketnum].lock);
if (destroy)
bucket_empty = fctx_destroy(fctx);
isc_event_free(&event); isc_event_free(&event);
dns_adb_destroyfind(&find); dns_adb_destroyfind(&find);
@@ -3889,13 +3896,15 @@ clone_results(fetchctx_t *fctx) {
/* /*
* Destroy '*fctx' if it is ready to be destroyed (i.e., if it has * Destroy '*fctx' if it is ready to be destroyed (i.e., if it has
* no references and is no longer waiting for any events). If this * no references and is no longer waiting for any events).
* was the last fctx in the resolver, destroy the resolver.
* *
* Requires: * Requires:
* '*fctx' is shutting down. * '*fctx' is shutting down.
*
* Returns:
* true if the resolver is exiting and this is the last fctx in the bucket.
*/ */
static void static isc_boolean_t
maybe_destroy(fetchctx_t *fctx, isc_boolean_t locked) { maybe_destroy(fetchctx_t *fctx, isc_boolean_t locked) {
unsigned int bucketnum; unsigned int bucketnum;
isc_boolean_t bucket_empty = ISC_FALSE; isc_boolean_t bucket_empty = ISC_FALSE;
@@ -3904,8 +3913,11 @@ maybe_destroy(fetchctx_t *fctx, isc_boolean_t locked) {
REQUIRE(SHUTTINGDOWN(fctx)); REQUIRE(SHUTTINGDOWN(fctx));
bucketnum = fctx->bucketnum;
if (!locked)
LOCK(&res->buckets[bucketnum].lock);
if (fctx->pending != 0 || fctx->nqueries != 0) if (fctx->pending != 0 || fctx->nqueries != 0)
return; goto unlock;
for (validator = ISC_LIST_HEAD(fctx->validators); for (validator = ISC_LIST_HEAD(fctx->validators);
validator != NULL; validator = next_validator) { validator != NULL; validator = next_validator) {
@@ -3913,16 +3925,12 @@ maybe_destroy(fetchctx_t *fctx, isc_boolean_t locked) {
dns_validator_cancel(validator); dns_validator_cancel(validator);
} }
bucketnum = fctx->bucketnum;
if (!locked)
LOCK(&res->buckets[bucketnum].lock);
if (fctx->references == 0 && ISC_LIST_EMPTY(fctx->validators)) if (fctx->references == 0 && ISC_LIST_EMPTY(fctx->validators))
bucket_empty = fctx_destroy(fctx); bucket_empty = fctx_destroy(fctx);
unlock:
if (!locked) if (!locked)
UNLOCK(&res->buckets[bucketnum].lock); UNLOCK(&res->buckets[bucketnum].lock);
return (bucket_empty);
if (bucket_empty)
empty_bucket(res);
} }
/* /*
@@ -3930,31 +3938,33 @@ maybe_destroy(fetchctx_t *fctx, isc_boolean_t locked) {
*/ */
static void static void
validated(isc_task_t *task, isc_event_t *event) { validated(isc_task_t *task, isc_event_t *event) {
isc_result_t result = ISC_R_SUCCESS; dns_adbaddrinfo_t *addrinfo;
isc_result_t eresult = ISC_R_SUCCESS; dns_dbnode_t *node = NULL;
isc_stdtime_t now; dns_dbnode_t *nsnode = NULL;
fetchctx_t *fctx;
dns_validatorevent_t *vevent;
dns_fetchevent_t *hevent; dns_fetchevent_t *hevent;
dns_name_t *name;
dns_rdataset_t *ardataset = NULL; dns_rdataset_t *ardataset = NULL;
dns_rdataset_t *asigrdataset = NULL; dns_rdataset_t *asigrdataset = NULL;
dns_dbnode_t *node = NULL;
isc_boolean_t negative;
isc_boolean_t chaining;
isc_boolean_t sentresponse;
isc_uint32_t ttl;
dns_dbnode_t *nsnode = NULL;
dns_name_t *name;
dns_rdataset_t *rdataset; dns_rdataset_t *rdataset;
dns_rdataset_t *sigrdataset; dns_rdataset_t *sigrdataset;
dns_resolver_t *res;
dns_valarg_t *valarg; dns_valarg_t *valarg;
dns_adbaddrinfo_t *addrinfo; dns_validatorevent_t *vevent;
fetchctx_t *fctx;
isc_boolean_t chaining;
isc_boolean_t negative;
isc_boolean_t sentresponse;
isc_result_t eresult = ISC_R_SUCCESS;
isc_result_t result = ISC_R_SUCCESS;
isc_stdtime_t now;
isc_uint32_t ttl;
UNUSED(task); /* for now */ UNUSED(task); /* for now */
REQUIRE(event->ev_type == DNS_EVENT_VALIDATORDONE); REQUIRE(event->ev_type == DNS_EVENT_VALIDATORDONE);
valarg = event->ev_arg; valarg = event->ev_arg;
fctx = valarg->fctx; fctx = valarg->fctx;
res = fctx->res;
addrinfo = valarg->addrinfo; addrinfo = valarg->addrinfo;
REQUIRE(VALID_FCTX(fctx)); REQUIRE(VALID_FCTX(fctx));
REQUIRE(!ISC_LIST_EMPTY(fctx->validators)); REQUIRE(!ISC_LIST_EMPTY(fctx->validators));
@@ -3964,7 +3974,7 @@ validated(isc_task_t *task, isc_event_t *event) {
FCTXTRACE("received validation completion event"); FCTXTRACE("received validation completion event");
LOCK(&fctx->res->buckets[fctx->bucketnum].lock); LOCK(&res->buckets[fctx->bucketnum].lock);
ISC_LIST_UNLINK(fctx->validators, vevent->validator, link); ISC_LIST_UNLINK(fctx->validators, vevent->validator, link);
fctx->validator = NULL; fctx->validator = NULL;
@@ -3974,7 +3984,7 @@ validated(isc_task_t *task, isc_event_t *event) {
* destroy the fctx if necessary. * destroy the fctx if necessary.
*/ */
dns_validator_destroy(&vevent->validator); dns_validator_destroy(&vevent->validator);
isc_mem_put(fctx->res->buckets[fctx->bucketnum].mctx, isc_mem_put(res->buckets[fctx->bucketnum].mctx,
valarg, sizeof(*valarg)); valarg, sizeof(*valarg));
negative = ISC_TF(vevent->rdataset == NULL); negative = ISC_TF(vevent->rdataset == NULL);
@@ -3987,10 +3997,12 @@ validated(isc_task_t *task, isc_event_t *event) {
* so, destroy the fctx. * so, destroy the fctx.
*/ */
if (SHUTTINGDOWN(fctx) && !sentresponse) { if (SHUTTINGDOWN(fctx) && !sentresponse) {
isc_mutex_t *bucketlock = isc_uint32_t bucketnum = fctx->bucketnum;
&fctx->res->buckets[fctx->bucketnum].lock; isc_boolean_t bucket_empty;
maybe_destroy(fctx, ISC_TRUE); bucket_empty = maybe_destroy(fctx, ISC_TRUE);
UNLOCK(bucketlock); UNLOCK(&res->buckets[bucketnum].lock);
if (bucket_empty)
empty_bucket(res);
goto cleanup_event; goto cleanup_event;
} }
@@ -4039,7 +4051,7 @@ validated(isc_task_t *task, isc_event_t *event) {
if (vevent->result != ISC_R_SUCCESS) { if (vevent->result != ISC_R_SUCCESS) {
FCTXTRACE("validation failed"); FCTXTRACE("validation failed");
inc_stats(fctx->res, dns_resstatscounter_valfail); inc_stats(res, dns_resstatscounter_valfail);
fctx->valfail++; fctx->valfail++;
fctx->vresult = vevent->result; fctx->vresult = vevent->result;
if (fctx->vresult != DNS_R_BROKENCHAIN) { if (fctx->vresult != DNS_R_BROKENCHAIN) {
@@ -4088,7 +4100,7 @@ validated(isc_task_t *task, isc_event_t *event) {
result = fctx->vresult; result = fctx->vresult;
add_bad(fctx, addrinfo, result, badns_validation); add_bad(fctx, addrinfo, result, badns_validation);
isc_event_free(&event); isc_event_free(&event);
UNLOCK(&fctx->res->buckets[fctx->bucketnum].lock); UNLOCK(&res->buckets[fctx->bucketnum].lock);
INSIST(fctx->validator == NULL); INSIST(fctx->validator == NULL);
fctx->validator = ISC_LIST_HEAD(fctx->validators); fctx->validator = ISC_LIST_HEAD(fctx->validators);
if (fctx->validator != NULL) if (fctx->validator != NULL)
@@ -4107,8 +4119,7 @@ validated(isc_task_t *task, isc_event_t *event) {
fctx->type == dns_rdatatype_dlv || fctx->type == dns_rdatatype_dlv ||
fctx->type == dns_rdatatype_ds) && fctx->type == dns_rdatatype_ds) &&
tresult == ISC_R_SUCCESS) tresult == ISC_R_SUCCESS)
dns_resolver_addbadcache(fctx->res, dns_resolver_addbadcache(res, &fctx->name,
&fctx->name,
fctx->type, &expire); fctx->type, &expire);
fctx_done(fctx, result, __LINE__); /* Locks bucket. */ fctx_done(fctx, result, __LINE__); /* Locks bucket. */
} else } else
@@ -4121,7 +4132,7 @@ validated(isc_task_t *task, isc_event_t *event) {
dns_rdatatype_t covers; dns_rdatatype_t covers;
FCTXTRACE("nonexistence validation OK"); FCTXTRACE("nonexistence validation OK");
inc_stats(fctx->res, dns_resstatscounter_valnegsuccess); inc_stats(res, dns_resstatscounter_valnegsuccess);
if (fctx->rmessage->rcode == dns_rcode_nxdomain) if (fctx->rmessage->rcode == dns_rcode_nxdomain)
covers = dns_rdatatype_any; covers = dns_rdatatype_any;
@@ -4138,10 +4149,9 @@ validated(isc_task_t *task, isc_event_t *event) {
* to zero to facilitate locating the containing zone of * to zero to facilitate locating the containing zone of
* a arbitrary zone. * a arbitrary zone.
*/ */
ttl = fctx->res->view->maxncachettl; ttl = res->view->maxncachettl;
if (fctx->type == dns_rdatatype_soa && if (fctx->type == dns_rdatatype_soa &&
covers == dns_rdatatype_any && covers == dns_rdatatype_any && res->zero_no_soa_ttl)
fctx->res->zero_no_soa_ttl)
ttl = 0; ttl = 0;
result = ncache_adderesult(fctx->rmessage, fctx->cache, node, result = ncache_adderesult(fctx->rmessage, fctx->cache, node,
@@ -4151,7 +4161,7 @@ validated(isc_task_t *task, isc_event_t *event) {
goto noanswer_response; goto noanswer_response;
goto answer_response; goto answer_response;
} else } else
inc_stats(fctx->res, dns_resstatscounter_valsuccess); inc_stats(res, dns_resstatscounter_valsuccess);
FCTXTRACE("validation OK"); FCTXTRACE("validation OK");
@@ -4199,14 +4209,17 @@ validated(isc_task_t *task, isc_event_t *event) {
} }
if (sentresponse) { if (sentresponse) {
isc_boolean_t bucket_empty = ISC_FALSE;
/* /*
* If we only deferred the destroy because we wanted to cache * If we only deferred the destroy because we wanted to cache
* the data, destroy now. * the data, destroy now.
*/ */
dns_db_detachnode(fctx->cache, &node); dns_db_detachnode(fctx->cache, &node);
UNLOCK(&fctx->res->buckets[fctx->bucketnum].lock);
if (SHUTTINGDOWN(fctx)) if (SHUTTINGDOWN(fctx))
maybe_destroy(fctx, ISC_FALSE); /* Locks bucket. */ bucket_empty = maybe_destroy(fctx, ISC_TRUE);
UNLOCK(&res->buckets[fctx->bucketnum].lock);
if (bucket_empty)
empty_bucket(res);
goto cleanup_event; goto cleanup_event;
} }
@@ -4221,7 +4234,7 @@ validated(isc_task_t *task, isc_event_t *event) {
* be validated. * be validated.
*/ */
dns_db_detachnode(fctx->cache, &node); dns_db_detachnode(fctx->cache, &node);
UNLOCK(&fctx->res->buckets[fctx->bucketnum].lock); UNLOCK(&res->buckets[fctx->bucketnum].lock);
dns_validator_send(ISC_LIST_HEAD(fctx->validators)); dns_validator_send(ISC_LIST_HEAD(fctx->validators));
goto cleanup_event; goto cleanup_event;
} }
@@ -4296,7 +4309,7 @@ validated(isc_task_t *task, isc_event_t *event) {
if (node != NULL) if (node != NULL)
dns_db_detachnode(fctx->cache, &node); dns_db_detachnode(fctx->cache, &node);
UNLOCK(&fctx->res->buckets[fctx->bucketnum].lock); UNLOCK(&res->buckets[fctx->bucketnum].lock);
fctx_done(fctx, result, __LINE__); /* Locks bucket. */ fctx_done(fctx, result, __LINE__); /* Locks bucket. */
cleanup_event: cleanup_event: