diff --git a/lib/dns/zone.c b/lib/dns/zone.c index 8821d1f862..3b280ab495 100644 --- a/lib/dns/zone.c +++ b/lib/dns/zone.c @@ -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); } diff --git a/tests/dns/zonemgr_test.c b/tests/dns/zonemgr_test.c index c26d3fdebd..7eeaf770ef 100644 --- a/tests/dns/zonemgr_test.c +++ b/tests/dns/zonemgr_test.c @@ -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);