ComputerSurge/bind9
3
0
Fork 0
Code Pull Requests 1 Activity

Merge branch '3617-keyfetch-race-v9_18' into 'v9_18'

Browse Source 
Call dns_resolver_createfetch() asynchronously in zone_refreshkeys()

See merge request isc-projects/bind9!7006
This commit is contained in:
Evan Hunt
2022-11-01 08:44:07 +00:00
parent c8ff8108e7 67f60e5ae4
commit dfcbbdcb6f
3 changed files with 99 additions and 76 deletions
Download Patch File Download Diff File Expand all files Collapse all files
CHANGES
+3
View File
@@ -1,3 +1,6 @@
6008. [bug] Fixed a race condition that could cause a crash
in dns_zone_synckeyzone(). [GL #3617]
6007. [cleanup] Don't enforce the jemalloc use on NetBSD. [GL #3634]
6003. [bug] Fix an inheritance bug when setting the port on
lib/dns/zone.c
+92 -73
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);
}
tests/dns/zonemgr_test.c
+4 -3
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);