upstream/bind9
1
0
Fork 0
mirror of https://github.com/isc-projects/bind9.git synced 2026-05-28 04:34:54 -04:00
Code Issues Projects Releases Packages Wiki Activity Actions 4

Call dns_resolver_createfetch() asynchronously in zone_refreshkeys()

Browse source 
Because dns_resolver_createfetch() locks the view, it was necessary
to unlock the zone in zone_refreshkeys() before calling it in order
to maintain the lock order, and relock afterward. this permitted a race
with dns_zone_synckeyzone().

This commit moves the call to dns_resolver_createfetch() into a separate
function which is called asynchronously after the zone has been
unlocked.

The keyfetch object now attaches to the zone to ensure that
it won't be shut down before the asynchronous call completes.

This necessitated refactoring dns_zone_detach() so it always runs
unlocked. For managed zones it schedules zone_shutdown() to
run asynchronously; for unmanaged zones there is no task.
This commit is contained in:
Evan Hunt 2022-10-23 11:39:44 -07:00
parent c8ff8108e7
commit 28d0c37ef0
2 changed files with 96 additions and 76 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

165
lib/dns/zone.c
View file

@ -5756,56 +5756,43 @@ dns_zone_attach(dns_zone_t *source, dns_zone_t **target) {
void
dns_zone_detach(dns_zone_t **zonep) {
REQUIRE(zonep != NULL && DNS_ZONE_VALID(*zonep));
dns_zone_t *zone = *zonep;
*zonep = NULL;
bool free_now = false;
dns_zone_t *raw = NULL;
dns_zone_t *secure = NULL;
if (isc_refcount_decrement(&zone->erefs) == 1) {
isc_refcount_destroy(&zone->erefs);
LOCK_ZONE(zone);
INSIST(zone != zone->raw);
/*
* We just detached the last external reference.
*/
if (zone->task != NULL) {
/*
* This zone is being managed. Post
* its control event and let it clean
* up synchronously in the context of
* its task.
* This zone has a task; it can clean
* itself up asynchronously.
*/
isc_event_t *ev = &zone->ctlevent;
isc_task_send(zone->task, &ev);
} else {
/*
* This zone is not being managed; it has
* no task and can have no outstanding
* events. Free it immediately.
*/
/*
* Unmanaged zones should not have non-null views;
* we have no way of detaching from the view here
* without causing deadlock because this code is called
* with the view already locked.
*/
INSIST(zone->view == NULL);
free_now = true;
raw = zone->raw;
zone->raw = NULL;
secure = zone->secure;
zone->secure = NULL;
return;
}
UNLOCK_ZONE(zone);
}
if (free_now) {
if (raw != NULL) {
dns_zone_detach(&raw);
/*
* This zone is unmanaged; we're probably running in
* named-checkzone or a unit test. There's no task,
* so we need to free it immediately.
*
* Unmanaged zones must not have null views;
* we have no way of detaching from the view here
* without causing deadlock because this code is
* called with the view already locked.
*/
INSIST(zone->view == NULL);
if (zone->raw != NULL) {
dns_zone_detach(&zone->raw);
}
if (secure != NULL) {
dns_zone_idetach(&secure);
if (zone->secure != NULL) {
dns_zone_idetach(&zone->secure);
}
zone_free(zone);
}
@ -11147,7 +11134,9 @@ cleanup:
isc_refcount_decrement(&zone->irefs);
kfetch->zone = NULL;
/* The zone must be managed */
INSIST(kfetch->zone->task != NULL);
dns_zone_detach(&kfetch->zone);
if (dns_rdataset_isassociated(keydataset)) {
dns_rdataset_disassociate(keydataset);
@ -11168,6 +11157,7 @@ cleanup:
free_needed = exit_check(zone);
UNLOCK_ZONE(zone);
if (free_needed) {
zone_free(zone);
}
@ -11175,6 +11165,62 @@ cleanup:
INSIST(ver == NULL);
}
static void
retry_keyfetch(dns_keyfetch_t *kfetch, dns_name_t *kname) {
isc_time_t timenow, timethen;
char timebuf[80];
dns_zone_t *zone = kfetch->zone;
/*
* Error during a key fetch; cancel and retry in an hour.
*/
zone->refreshkeycount--;
isc_refcount_decrement(&zone->irefs);
dns_db_detach(&kfetch->db);
dns_rdataset_disassociate(&kfetch->keydataset);
dns_name_free(kname, zone->mctx);
isc_mem_put(zone->mctx, kfetch, sizeof(dns_keyfetch_t));
dnssec_log(zone, ISC_LOG_WARNING,
"Failed to create fetch for DNSKEY update");
TIME_NOW(&timenow);
DNS_ZONE_TIME_ADD(&timenow, dns_zone_mkey_hour, &timethen);
LOCK_ZONE(zone);
zone->refreshkeytime = timethen;
zone_settimer(zone, &timenow);
UNLOCK_ZONE(zone);
isc_time_formattimestamp(&zone->refreshkeytime, timebuf, 80);
dnssec_log(zone, ISC_LOG_DEBUG(1), "retry key refresh: %s", timebuf);
dns_zone_detach(&zone);
}
static void
do_keyfetch(isc_task_t *task, isc_event_t *event) {
isc_result_t result;
dns_keyfetch_t *kfetch = (dns_keyfetch_t *)event->ev_arg;
dns_name_t *kname = dns_fixedname_name(&kfetch->name);
dns_zone_t *zone = kfetch->zone;
UNUSED(task);
isc_event_free(&event);
result = dns_resolver_createfetch(
zone->view->resolver, kname, dns_rdatatype_dnskey, NULL, NULL,
NULL, NULL, 0,
DNS_FETCHOPT_NOVALIDATE | DNS_FETCHOPT_UNSHARED |
DNS_FETCHOPT_NOCACHED,
0, NULL, zone->task, keyfetch_done, kfetch, &kfetch->dnskeyset,
&kfetch->dnskeysigset, &kfetch->fetch);
if (result != ISC_R_SUCCESS) {
retry_keyfetch(kfetch, kname);
}
}
/*
* Refresh the data in the key zone. Initiate a fetch to look up
* DNSKEY records at the trust anchor name.
@ -11191,7 +11237,7 @@ zone_refreshkeys(dns_zone_t *zone) {
dns_rdata_keydata_t kd;
isc_stdtime_t now;
bool commit = false;
bool fetching = false, fetch_err = false;
bool fetching = false;
bool timerset = false;
ENTER;
@ -11276,9 +11322,10 @@ zone_refreshkeys(dns_zone_t *zone) {
dns_rriterator_pause(&rrit);
kfetch = isc_mem_get(zone->mctx, sizeof(dns_keyfetch_t));
*kfetch = (dns_keyfetch_t){ .zone = NULL };
zone->refreshkeycount++;
kfetch->zone = zone;
dns_zone_attach(zone, &kfetch->zone);
isc_refcount_increment0(&zone->irefs);
kname = dns_fixedname_initname(&kfetch->name);
dns_name_dup(name, zone->mctx, kname);
@ -11312,35 +11359,22 @@ zone_refreshkeys(dns_zone_t *zone) {
#ifdef ENABLE_AFL
if (!dns_fuzzing_resolver) {
#endif /* ifdef ENABLE_AFL */
UNLOCK_ZONE(zone);
result = dns_resolver_createfetch(
zone->view->resolver, kname,
dns_rdatatype_dnskey, NULL, NULL, NULL, NULL, 0,
DNS_FETCHOPT_NOVALIDATE |
DNS_FETCHOPT_UNSHARED |
DNS_FETCHOPT_NOCACHED,
0, NULL, zone->task, keyfetch_done, kfetch,
&kfetch->dnskeyset, &kfetch->dnskeysigset,
&kfetch->fetch);
LOCK_ZONE(zone);
#ifdef ENABLE_AFL
} else {
result = ISC_R_FAILURE;
}
#endif /* ifdef ENABLE_AFL */
if (result == ISC_R_SUCCESS) {
isc_event_t *e = isc_event_allocate(
zone->mctx, NULL, DNS_EVENT_ZONE, do_keyfetch,
kfetch, sizeof(isc_event_t));
isc_task_send(zone->task, &e);
fetching = true;
#ifdef ENABLE_AFL
} else {
zone->refreshkeycount--;
isc_refcount_decrement(&zone->irefs);
dns_db_detach(&kfetch->db);
dns_rdataset_disassociate(&kfetch->keydataset);
dns_name_free(kname, zone->mctx);
dns_zone_detach(&kfetch->zone);
isc_mem_put(zone->mctx, kfetch, sizeof(dns_keyfetch_t));
dnssec_log(zone, ISC_LOG_WARNING,
"Failed to create fetch for DNSKEY update");
fetch_err = true;
}
#endif /* ifdef ENABLE_AFL */
}
if (!ISC_LIST_EMPTY(diff.tuples)) {
CHECK(update_soa_serial(zone, db, ver, &diff, zone->mctx,
@ -11352,22 +11386,7 @@ zone_refreshkeys(dns_zone_t *zone) {
}
failure:
if (fetch_err) {
/*
* Error during a key fetch; retry in an hour.
*/
isc_time_t timenow, timethen;
char timebuf[80];
TIME_NOW(&timenow);
DNS_ZONE_TIME_ADD(&timenow, dns_zone_mkey_hour, &timethen);
zone->refreshkeytime = timethen;
zone_settimer(zone, &timenow);
isc_time_formattimestamp(&zone->refreshkeytime, timebuf, 80);
dnssec_log(zone, ISC_LOG_DEBUG(1), "retry key refresh: %s",
timebuf);
} else if (!timerset) {
if (!timerset) {
isc_time_settoepoch(&zone->refreshkeytime);
}

7
tests/dns/zonemgr_test.c
View file

@ -113,9 +113,10 @@ ISC_RUN_TEST_IMPL(dns_zonemgr_createzone) {
assert_int_equal(result, ISC_R_SUCCESS);
assert_non_null(zone);
if (zone != NULL) {
dns_zone_detach(&zone);
}
result = dns_zonemgr_managezone(myzonemgr, zone);
assert_int_equal(result, ISC_R_SUCCESS);
dns_zone_detach(&zone);
dns_zonemgr_shutdown(myzonemgr);
dns_zonemgr_detach(&myzonemgr);