diff --git a/CHANGES b/CHANGES index a442fb314b..24f85d70ed 100644 --- a/CHANGES +++ b/CHANGES @@ -1,3 +1,6 @@ +4253. [bug] Address fetch context reference count handling error + on socket error. [RT#40945] + 4248. [func] Add an isc_atomic_storeq() function, use it in stats counters to improve performance. [RT #39972] [RT #39979] diff --git a/lib/dns/resolver.c b/lib/dns/resolver.c index 5ecd487e80..749264bc75 100644 --- a/lib/dns/resolver.c +++ b/lib/dns/resolver.c @@ -1725,8 +1725,7 @@ fctx_query(fetchctx_t *fctx, dns_adbaddrinfo_t *addrinfo, } query->dispentry = NULL; - fctx_increference(fctx); - query->fctx = fctx; + query->fctx = fctx; /* reference added by caller */ query->tsig = NULL; query->tsigkey = NULL; ISC_LINK_INIT(query, link); @@ -1780,11 +1779,6 @@ fctx_query(fetchctx_t *fctx, dns_adbaddrinfo_t *addrinfo, if (query->dispatch != NULL) dns_dispatch_detach(&query->dispatch); - LOCK(&res->buckets[fctx->bucketnum].lock); - INSIST(fctx->references > 1); - fctx->references--; - UNLOCK(&res->buckets[fctx->bucketnum].lock); - cleanup_query: if (query->connects == 0) { query->magic = 0; @@ -3353,6 +3347,8 @@ fctx_try(fetchctx_t *fctx, isc_boolean_t retrying, isc_boolean_t badcache) { isc_result_t result; dns_adbaddrinfo_t *addrinfo = NULL; dns_resolver_t *res; + unsigned int bucketnum; + isc_boolean_t bucket_empty; FCTXTRACE("try"); @@ -3431,10 +3427,17 @@ fctx_try(fetchctx_t *fctx, isc_boolean_t retrying, isc_boolean_t badcache) { } } + bucketnum = fctx->bucketnum; + fctx_increference(fctx); result = fctx_query(fctx, addrinfo, fctx->options); - if (result != ISC_R_SUCCESS) + if (result != ISC_R_SUCCESS) { fctx_done(fctx, result, __LINE__); - else if (retrying) + LOCK(&res->buckets[bucketnum].lock); + bucket_empty = fctx_decreference(fctx); + UNLOCK(&res->buckets[bucketnum].lock); + if (bucket_empty) + empty_bucket(res); + } else if (retrying) inc_stats(res, dns_resstatscounter_retry); } @@ -7293,6 +7296,9 @@ resquery_response(isc_task_t *task, isc_event_t *event) { isc_result_t broken_server; badnstype_t broken_type = badns_response; isc_boolean_t no_response; + unsigned int bucketnum; + dns_resolver_t *res; + isc_boolean_t bucket_empty; REQUIRE(VALID_QUERY(query)); fctx = query->fctx; @@ -7302,10 +7308,11 @@ resquery_response(isc_task_t *task, isc_event_t *event) { QTRACE("response"); + res = fctx->res; if (isc_sockaddr_pf(&query->addrinfo->sockaddr) == PF_INET) - inc_stats(fctx->res, dns_resstatscounter_responsev4); + inc_stats(res, dns_resstatscounter_responsev4); else - inc_stats(fctx->res, dns_resstatscounter_responsev6); + inc_stats(res, dns_resstatscounter_responsev6); (void)isc_timer_touch(fctx->timer); @@ -7317,7 +7324,7 @@ resquery_response(isc_task_t *task, isc_event_t *event) { finish = NULL; no_response = ISC_FALSE; - if (fctx->res->exiting) { + if (res->exiting) { result = ISC_R_SHUTTINGDOWN; FCTXTRACE("resolver shutting down"); goto done; @@ -7426,7 +7433,7 @@ resquery_response(isc_task_t *task, isc_event_t *event) { resend = ISC_TRUE; add_bad_edns(fctx, &query->addrinfo->sockaddr); - inc_stats(fctx->res, + inc_stats(res, dns_resstatscounter_edns0fail); } else { broken_server = result; @@ -7450,8 +7457,7 @@ resquery_response(isc_task_t *task, isc_event_t *event) { options |= DNS_FETCHOPT_NOEDNS0; resend = ISC_TRUE; add_bad_edns(fctx, &query->addrinfo->sockaddr); - inc_stats(fctx->res, - dns_resstatscounter_edns0fail); + inc_stats(res, dns_resstatscounter_edns0fail); } else { broken_server = DNS_R_UNEXPECTEDRCODE; keep_trying = ISC_TRUE; @@ -7469,7 +7475,7 @@ resquery_response(isc_task_t *task, isc_event_t *event) { /* * Log the incoming packet. */ - log_packet(message, ISC_LOG_DEBUG(10), fctx->res->mctx); + log_packet(message, ISC_LOG_DEBUG(10), res->mctx); /* * Process receive opt record. @@ -7482,7 +7488,7 @@ resquery_response(isc_task_t *task, isc_event_t *event) { * If the message is signed, check the signature. If not, this * returns success anyway. */ - result = dns_message_checksig(message, fctx->res->view); + result = dns_message_checksig(message, res->view); if (result != ISC_R_SUCCESS) { FCTXTRACE3("signature check failed", result); goto done; @@ -7539,7 +7545,7 @@ resquery_response(isc_task_t *task, isc_event_t *event) { truncated = ISC_TRUE; if (truncated) { - inc_stats(fctx->res, dns_resstatscounter_truncated); + inc_stats(res, dns_resstatscounter_truncated); if ((options & DNS_FETCHOPT_TCP) != 0) { broken_server = DNS_R_TRUNCATEDTCP; keep_trying = ISC_TRUE; @@ -7568,16 +7574,16 @@ resquery_response(isc_task_t *task, isc_event_t *event) { if (message->rcode != dns_rcode_noerror) { switch (message->rcode) { case dns_rcode_nxdomain: - inc_stats(fctx->res, dns_resstatscounter_nxdomain); + inc_stats(res, dns_resstatscounter_nxdomain); break; case dns_rcode_servfail: - inc_stats(fctx->res, dns_resstatscounter_servfail); + inc_stats(res, dns_resstatscounter_servfail); break; case dns_rcode_formerr: - inc_stats(fctx->res, dns_resstatscounter_formerr); + inc_stats(res, dns_resstatscounter_formerr); break; default: - inc_stats(fctx->res, dns_resstatscounter_othererror); + inc_stats(res, dns_resstatscounter_othererror); break; } } @@ -7611,7 +7617,7 @@ resquery_response(isc_task_t *task, isc_event_t *event) { * Remember that they may not like EDNS0. */ add_bad_edns(fctx, &query->addrinfo->sockaddr); - inc_stats(fctx->res, dns_resstatscounter_edns0fail); + inc_stats(res, dns_resstatscounter_edns0fail); } else if (message->rcode == dns_rcode_formerr) { if (ISFORWARDER(query->addrinfo)) { /* @@ -7678,13 +7684,13 @@ resquery_response(isc_task_t *task, isc_event_t *event) { /* * Is the server lame? */ - if (fctx->res->lame_ttl != 0 && !ISFORWARDER(query->addrinfo) && + if (res->lame_ttl != 0 && !ISFORWARDER(query->addrinfo) && is_lame(fctx)) { - inc_stats(fctx->res, dns_resstatscounter_lame); + inc_stats(res, dns_resstatscounter_lame); log_lame(fctx, query->addrinfo); result = dns_adb_marklame(fctx->adb, query->addrinfo, &fctx->name, fctx->type, - now + fctx->res->lame_ttl); + now + res->lame_ttl); if (result != ISC_R_SUCCESS) isc_log_write(dns_lctx, DNS_LOGCATEGORY_RESOLVER, DNS_LOGMODULE_RESOLVER, ISC_LOG_ERROR, @@ -7700,7 +7706,7 @@ resquery_response(isc_task_t *task, isc_event_t *event) { * Enforce delegations only zones like NET and COM. */ if (!ISFORWARDER(query->addrinfo) && - dns_view_isdelegationonly(fctx->res->view, &fctx->domain) && + dns_view_isdelegationonly(res->view, &fctx->domain) && !dns_name_equal(&fctx->domain, &fctx->name) && fix_mustbedelegationornxdomain(message, fctx)) { char namebuf[DNS_NAME_FORMATSIZE]; @@ -7712,7 +7718,7 @@ resquery_response(isc_task_t *task, isc_event_t *event) { dns_name_format(&fctx->name, namebuf, sizeof(namebuf)); dns_name_format(&fctx->domain, domainbuf, sizeof(domainbuf)); dns_rdatatype_format(fctx->type, typebuf, sizeof(typebuf)); - dns_rdataclass_format(fctx->res->rdclass, classbuf, + dns_rdataclass_format(res->rdclass, classbuf, sizeof(classbuf)); isc_sockaddr_format(&query->addrinfo->sockaddr, addrbuf, sizeof(addrbuf)); @@ -7724,7 +7730,7 @@ resquery_response(isc_task_t *task, isc_event_t *event) { domainbuf, namebuf, typebuf, classbuf, addrbuf); } - if ((fctx->res->options & DNS_RESOLVER_CHECKNAMES) != 0) + if ((res->options & DNS_RESOLVER_CHECKNAMES) != 0) checknames(message); /* @@ -7966,7 +7972,7 @@ resquery_response(isc_task_t *task, isc_event_t *event) { name = &fctx->name; else name = &fctx->domain; - result = dns_view_findzonecut(fctx->res->view, + result = dns_view_findzonecut(res->view, name, fname, now, findoptions, ISC_TRUE, @@ -8024,10 +8030,18 @@ resquery_response(isc_task_t *task, isc_event_t *event) { * Resend (probably with changed options). */ FCTXTRACE("resend"); - inc_stats(fctx->res, dns_resstatscounter_retry); + inc_stats(res, dns_resstatscounter_retry); + bucketnum = fctx->bucketnum; + fctx_increference(fctx); result = fctx_query(fctx, addrinfo, options); - if (result != ISC_R_SUCCESS) + if (result != ISC_R_SUCCESS) { fctx_done(fctx, result, __LINE__); + LOCK(&res->buckets[bucketnum].lock); + bucket_empty = fctx_decreference(fctx); + UNLOCK(&res->buckets[bucketnum].lock); + if (bucket_empty) + empty_bucket(res); + } } else if (result == ISC_R_SUCCESS && !HAVE_ANSWER(fctx)) { /* * All has gone well so far, but we are waiting for the @@ -8054,7 +8068,7 @@ resquery_response(isc_task_t *task, isc_event_t *event) { FCTXTRACE("suspending DS lookup to find parent's NS records"); - result = dns_resolver_createfetch(fctx->res, &fctx->nsname, + result = dns_resolver_createfetch(res, &fctx->nsname, dns_rdatatype_ns, NULL, NULL, NULL, 0, task, resume_dslookup, fctx,