Commit a4195c3c48 for bind
commit a4195c3c4841d15aaef9044d59db75b324a82cea
Author: Evan Hunt <each@isc.org>
Date: Thu Oct 29 19:11:17 2026 -0700
Handle dns_resolver_createfetch() failures in zonefetch
When dns_resolver_createfetch() is called by dns_zonefetch_run() and
immediately fails - for example, when it gets DNS_R_NXDOMAIN from
dns_view_bestzonecut() - the completion handler for the zone fetch
must be called with the result code.
Previously, a cancel handler was called instead, but that didn't take a
result code and wasn't always able to handle failures correctly. We now
call the "done_fetch" callback and the "cancel_fetch" callback has been
removed.
Additionally, the "start_fetch" callbacks have been cleaned up to ensure
that fetch->qname and fetch->qtype are always initialized even in the
event of failure.
diff --git a/lib/dns/include/dns/zonefetch.h b/lib/dns/include/dns/zonefetch.h
index f4068ede38..52f91e32c4 100644
--- a/lib/dns/include/dns/zonefetch.h
+++ b/lib/dns/include/dns/zonefetch.h
@@ -48,7 +48,6 @@ typedef struct dns_zonefetch dns_zonefetch_t;
typedef struct dns_zonefetch_methods {
isc_result_t (*start_fetch)(dns_zonefetch_t *fetch);
void (*continue_fetch)(dns_zonefetch_t *fetch);
- void (*cancel_fetch)(dns_zonefetch_t *fetch);
void (*cleanup_fetch)(dns_zonefetch_t *fetch);
isc_result_t (*done_fetch)(dns_zonefetch_t *fetch,
isc_result_t eresult);
@@ -84,6 +83,9 @@ struct dns_zonefetch {
dns_zone_t *zone;
dns_fixedname_t name;
+ /* Status */
+ bool canceled;
+
/* Query */
dns_name_t *qname;
dns_rdatatype_t qtype;
diff --git a/lib/dns/zone.c b/lib/dns/zone.c
index 75b94de265..6f96b34380 100644
--- a/lib/dns/zone.c
+++ b/lib/dns/zone.c
@@ -9083,42 +9083,6 @@ keyfetch_continue(dns_zonefetch_t *fetch) {
REQUIRE(0);
}
-static void
-keyfetch_cancel(dns_zonefetch_t *fetch) {
- dns_keyfetch_t *kfetch;
- dns_zone_t *zone;
-
- REQUIRE(fetch->fetchtype == ZONEFETCHTYPE_KEY);
- REQUIRE(DNS_ZONE_VALID(fetch->zone));
- REQUIRE(LOCKED_ZONE(fetch->zone));
-
- kfetch = &fetch->fetchdata.keyfetch;
- zone = fetch->zone;
-
- /*
- * Error during a key fetch; cancel and retry in an hour.
- */
- zone->fetchcount[ZONEFETCHTYPE_KEY]--;
-
- dns_db_detach(&kfetch->db);
- dns_rdataset_disassociate(&kfetch->keydataset);
-
- if (!DNS_ZONE_FLAG(zone, DNS_ZONEFLG_EXITING)) {
- /* Don't really retry if we are exiting */
- isc_time_t timenow, timethen;
- char timebuf[80];
-
- timenow = isc_time_now();
- DNS_ZONE_TIME_ADD(&timenow, dns_zone_mkey_hour, &timethen);
- zone->refreshkeytime = timethen;
- dns__zone_settimer(zone, timenow);
-
- isc_time_formattimestamp(&zone->refreshkeytime, timebuf, 80);
- dnssec_log(zone, ISC_LOG_DEBUG(1), "retry key refresh: %s",
- timebuf);
- }
-}
-
static void
keyfetch_cleanup(dns_zonefetch_t *fetch) {
dns_keyfetch_t *kfetch = NULL;
@@ -9173,6 +9137,27 @@ keyfetch_done(dns_zonefetch_t *fetch, isc_result_t eresult) {
kfetch = &fetch->fetchdata.keyfetch;
zone = fetch->zone;
+
+ zone->fetchcount[ZONEFETCHTYPE_KEY]--;
+
+ if (fetch->canceled) {
+ if (!DNS_ZONE_FLAG(zone, DNS_ZONEFLG_EXITING)) {
+ /* Don't really retry if we are exiting */
+ isc_time_t timenow, timethen;
+ char timebuf[80];
+ timenow = isc_time_now();
+ DNS_ZONE_TIME_ADD(&timenow, dns_zone_mkey_hour,
+ &timethen);
+ zone->refreshkeytime = timethen;
+ dns__zone_settimer(zone, timenow);
+ isc_time_formattimestamp(&zone->refreshkeytime, timebuf,
+ 80);
+ dnssec_log(zone, ISC_LOG_DEBUG(1),
+ "retry key refresh: %s", timebuf);
+ }
+ return eresult;
+ }
+
mctx = fetch->mctx;
keyname = dns_name(&fetch->name);
dnskeys = &fetch->rrset;
@@ -9190,7 +9175,6 @@ keyfetch_done(dns_zonefetch_t *fetch, isc_result_t eresult) {
CHECK(dns_db_newversion(kfetch->db, &ver));
- zone->fetchcount[ZONEFETCHTYPE_KEY]--;
alldone = (zone->fetchcount[ZONEFETCHTYPE_KEY] == 0);
if (alldone) {
@@ -9889,7 +9873,6 @@ zone_refreshkeys(dns_zone_t *zone) {
.start_fetch = keyfetch_start,
.continue_fetch =
keyfetch_continue,
- .cancel_fetch = keyfetch_cancel,
.cleanup_fetch =
keyfetch_cleanup,
.done_fetch = keyfetch_done,
@@ -17918,19 +17901,18 @@ nsfetch_start(dns_zonefetch_t *fetch) {
nsfetch = &fetch->fetchdata.nsfetch;
- /* Derive parent domain. Check for root domain. */
- if (!dns_name_belowroot(&nsfetch->pname)) {
- return ISC_R_NOTFOUND;
- }
-
- dns_name_split(&nsfetch->pname,
- dns_name_countlabels(&nsfetch->pname) - 1U, NULL,
- &nsfetch->pname);
-
fetch->qtype = dns_rdatatype_ns;
fetch->qname = &nsfetch->pname;
- return ISC_R_SUCCESS;
+ /* Derive parent domain, unless we're at the root. */
+ if (dns_name_belowroot(&nsfetch->pname)) {
+ dns_name_split(&nsfetch->pname,
+ dns_name_countlabels(&nsfetch->pname) - 1U, NULL,
+ &nsfetch->pname);
+ return ISC_R_SUCCESS;
+ } else {
+ return ISC_R_NOTFOUND;
+ }
}
/*
@@ -17964,19 +17946,6 @@ nsfetch_continue(dns_zonefetch_t *fetch) {
#endif /* ifdef ENABLE_AFL */
}
-static void
-nsfetch_cancel(dns_zonefetch_t *fetch) {
- dns_zone_t *zone;
-
- REQUIRE(fetch->fetchtype == ZONEFETCHTYPE_NS);
- REQUIRE(DNS_ZONE_VALID(fetch->zone));
- REQUIRE(LOCKED_ZONE(fetch->zone));
-
- zone = fetch->zone;
-
- zone->fetchcount[ZONEFETCHTYPE_NS]--;
-}
-
static void
nsfetch_cleanup(dns_zonefetch_t *fetch) {
REQUIRE(fetch->fetchtype == ZONEFETCHTYPE_NS);
@@ -18008,6 +17977,10 @@ nsfetch_checkds(dns_zonefetch_t *fetch, isc_result_t eresult) {
zone->fetchcount[ZONEFETCHTYPE_NS]--;
+ if (fetch->canceled) {
+ return eresult;
+ }
+
dns_name_format(pname, pnamebuf, sizeof(pnamebuf));
dnssec_log(zone, ISC_LOG_DEBUG(3),
"Returned from '%s' NS fetch in nsfetch_checkds(): %s",
@@ -18141,7 +18114,6 @@ zone_checkds(dns_zone_t *zone) {
(dns_zonefetch_methods_t){
.start_fetch = nsfetch_start,
.continue_fetch = nsfetch_continue,
- .cancel_fetch = nsfetch_cancel,
.cleanup_fetch = nsfetch_cleanup,
.done_fetch = nsfetch_checkds,
},
@@ -18174,16 +18146,18 @@ static dns_name_t _dsync = DNS_NAME_INITNONABSOLUTE(_dsync_data);
static isc_result_t
dsyncfetch_start(dns_zonefetch_t *fetch) {
dns_dsyncfetch_t *dsyncfetch;
- dns_zone_t *zone;
- dns_name_t *dsyncname, prefix;
+ dns_zone_t *zone = fetch->zone;
+ dns_name_t *dsyncname = NULL, prefix = DNS_NAME_INITEMPTY;
unsigned int nlabels;
isc_result_t result;
REQUIRE(fetch->fetchtype == ZONEFETCHTYPE_DSYNC);
- REQUIRE(DNS_ZONE_VALID(fetch->zone));
+ REQUIRE(DNS_ZONE_VALID(zone));
dsyncfetch = &fetch->fetchdata.dsyncfetch;
- zone = fetch->zone;
+
+ fetch->qtype = dns_rdatatype_dsync;
+ fetch->qname = dns_name(&fetch->name); /* placeholder value */
/*
* The dsync owner name is build up of <prefix>._dsync.<parent-name>.
@@ -18193,7 +18167,6 @@ dsyncfetch_start(dns_zonefetch_t *fetch) {
dsyncname = dns_fixedname_initname(&dsyncfetch->dsyncname);
nlabels = dns_name_countlabels(&dsyncfetch->pname);
- dns_name_init(&prefix);
dns_name_split(dns_name(&fetch->name), nlabels, &prefix, NULL);
result = dns_name_concatenate(&prefix, &_dsync, dsyncname);
@@ -18214,7 +18187,6 @@ dsyncfetch_start(dns_zonefetch_t *fetch) {
return result;
}
- fetch->qtype = dns_rdatatype_dsync;
fetch->qname = dsyncname;
return ISC_R_SUCCESS;
@@ -18251,19 +18223,6 @@ dsyncfetch_continue(dns_zonefetch_t *fetch) {
#endif /* ifdef ENABLE_AFL */
}
-static void
-dsyncfetch_cancel(dns_zonefetch_t *fetch) {
- dns_zone_t *zone;
-
- REQUIRE(fetch->fetchtype == ZONEFETCHTYPE_DSYNC);
- REQUIRE(DNS_ZONE_VALID(fetch->zone));
- REQUIRE(LOCKED_ZONE(fetch->zone));
-
- zone = fetch->zone;
-
- zone->fetchcount[ZONEFETCHTYPE_DSYNC]--;
-}
-
static void
dsyncfetch_cleanup(dns_zonefetch_t *fetch) {
REQUIRE(fetch->fetchtype == ZONEFETCHTYPE_DSYNC);
@@ -18298,6 +18257,10 @@ dsyncfetch_done(dns_zonefetch_t *fetch, isc_result_t eresult) {
zone->fetchcount[ZONEFETCHTYPE_DSYNC]--;
+ if (fetch->canceled) {
+ return eresult;
+ }
+
dns_name_format(dsyncname, dsyncnamebuf, sizeof(dsyncnamebuf));
dns_zone_log(zone, ISC_LOG_DEBUG(3),
"dsyncfetch: Returned from '%s' DSYNC fetch in "
@@ -18443,6 +18406,10 @@ nsfetch_dsync(dns_zonefetch_t *fetch, isc_result_t eresult) {
zone->fetchcount[ZONEFETCHTYPE_NS]--;
+ if (fetch->canceled) {
+ return eresult;
+ }
+
dns_name_format(pname, pnamebuf, sizeof(pnamebuf));
dnssec_log(zone, ISC_LOG_DEBUG(3),
"Returned from '%s' NS fetch in nsfetch_dsync(): %s",
@@ -18473,7 +18440,6 @@ nsfetch_dsync(dns_zonefetch_t *fetch, isc_result_t eresult) {
(dns_zonefetch_methods_t){
.start_fetch = dsyncfetch_start,
.continue_fetch = dsyncfetch_continue,
- .cancel_fetch = dsyncfetch_cancel,
.cleanup_fetch = dsyncfetch_cleanup,
.done_fetch = dsyncfetch_done,
},
@@ -18525,7 +18491,6 @@ zone_notifycds(dns_zone_t *zone) {
(dns_zonefetch_methods_t){
.start_fetch = nsfetch_start,
.continue_fetch = nsfetch_continue,
- .cancel_fetch = nsfetch_cancel,
.cleanup_fetch = nsfetch_cleanup,
.done_fetch = nsfetch_dsync,
},
diff --git a/lib/dns/zonefetch.c b/lib/dns/zonefetch.c
index c522ad1aaa..c68666bcaf 100644
--- a/lib/dns/zonefetch.c
+++ b/lib/dns/zonefetch.c
@@ -32,28 +32,21 @@ dns_zonefetch_run(void *arg) {
isc_loop_t *loop;
isc_result_t result;
dns_resolver_t *resolver = NULL;
+ dns_fetchresponse_t *resp = NULL;
+ isc_mem_t *mctx = NULL;
zone = fetch->zone;
- if (dns__zone_exiting(zone)) {
- result = ISC_R_SHUTTINGDOWN;
- goto cancel;
- }
view = dns_zone_getview(zone);
loop = dns_zone_getloop(zone);
INSIST(view != NULL);
INSIST(loop != NULL);
- result = fetch->fetchmethods.start_fetch(fetch);
- if (result != ISC_R_SUCCESS) {
- goto cancel;
- }
-
- result = dns_view_getresolver(view, &resolver);
- if (result != ISC_R_SUCCESS) {
- goto cancel;
+ if (dns__zone_exiting(zone)) {
+ CLEANUP(ISC_R_SHUTTINGDOWN);
}
+ CHECK(fetch->fetchmethods.start_fetch(fetch));
if (isc_log_wouldlog(ISC_LOG_DEBUG(3))) {
char namebuf[DNS_NAME_FORMATSIZE];
char typebuf[DNS_RDATATYPE_FORMATSIZE];
@@ -63,17 +56,19 @@ dns_zonefetch_run(void *arg) {
"Do fetch for %s/%s request", namebuf, typebuf);
}
+ fetch->canceled = false;
+ CHECK(dns_view_getresolver(view, &resolver));
result = dns_resolver_createfetch(
resolver, fetch->qname, fetch->qtype, NULL, NULL, NULL, NULL, 0,
fetch->options, 0, NULL, NULL, NULL, loop, dns_zonefetch_done,
fetch, NULL, &fetch->rrset, &fetch->sigset, &fetch->fetch);
-
dns_resolver_detach(&resolver);
-
-cancel:
if (result == ISC_R_SUCCESS) {
return;
- } else if (result != ISC_R_SHUTTINGDOWN) {
+ }
+
+cleanup:
+ if (result != ISC_R_SHUTTINGDOWN) {
char namebuf[DNS_NAME_FORMATSIZE];
if (DNS_NAME_VALID(fetch->qname)) {
@@ -82,37 +77,29 @@ cancel:
dns_rdatatype_format(fetch->qtype, typebuf,
sizeof(typebuf));
dns_zone_log(zone, ISC_LOG_WARNING,
- "Failed fetch for %s/%s request", namebuf,
- typebuf);
+ "Failed fetch for %s/%s (%s)", namebuf,
+ typebuf, isc_result_totext(result));
} else {
dns_zone_nameonly(zone, namebuf, sizeof(namebuf));
dns_zone_log(zone, ISC_LOG_WARNING,
- "Failed fetch for zone %s", namebuf);
+ "Failed fetch for zone %s (%s)", namebuf,
+ isc_result_totext(result));
}
}
/*
- * Fetch failed, cancel.
+ * Fetch failed: call the completion handler.
*/
- dns__zone_lock(zone);
-
- dns_name_t *zname = dns_name(&fetch->name);
- isc_mem_t *mctx = dns_zone_getmctx(zone);
- bool free_needed;
-
- isc_refcount_decrement(dns__zone_irefs(zone));
- dns_name_free(zname, mctx);
-
- fetch->fetchmethods.cancel_fetch(fetch);
-
- isc_mem_putanddetach(&fetch->mctx, fetch, sizeof(*fetch));
- free_needed = dns__zone_free_check(zone);
-
- dns__zone_unlock(zone);
-
- if (free_needed) {
- dns__zone_free(zone);
- }
+ fetch->canceled = true;
+ mctx = dns_zone_getmctx(zone);
+ resp = isc_mem_get(mctx, sizeof(*resp));
+ *resp = (dns_fetchresponse_t){
+ .result = result,
+ .arg = fetch,
+ .link = ISC_LINK_INITIALIZER,
+ };
+ isc_mem_attach(mctx, &resp->mctx);
+ isc_async_run(loop, dns_zonefetch_done, resp);
}
void
@@ -149,16 +136,17 @@ dns_zonefetch_done(void *arg) {
if (resp->cache != NULL) {
dns_db_detach(&resp->cache);
}
- dns_resolver_destroyfetch(&fetch->fetch);
+ if (fetch->fetch != NULL) {
+ dns_resolver_destroyfetch(&fetch->fetch);
+ }
+ /* Call the done handler if we're not shutting down */
dns__zone_lock(zone);
- if (dns__zone_exiting(zone) || view == NULL) {
- goto cleanup;
+ if (view != NULL && !dns__zone_exiting(zone)) {
+ result = fetch->fetchmethods.done_fetch(fetch, eresult);
}
- result = fetch->fetchmethods.done_fetch(fetch, eresult);
-
-cleanup:
+ /* Clean up */
isc_refcount_decrement(dns__zone_irefs(zone));
dns_rdataset_cleanup(rrset);