From: Tony Finch <fanf@isc.org>
Date: Wed, 3 May 2023 14:29:43 +0000 (+0100)
Subject: Avoid using the zone timer after its loop has gone
X-Git-Tag: v9.19.14~52^2~4
X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=2bce998b2b85eb42312610171ab013a1490a9b3e;p=thirdparty%2Fbind9.git

Avoid using the zone timer after its loop has gone

Shutdown and cleanup of zones is more asynchronous with the qp-trie
zone table. As a result it's possible that some activity is delayed
until after a zone has been released from its zonemanager.

Previously, the dns_zone code was not very strict in the way it
refers to the loop it is running on: The loop pointer was stashed when
dns_zonemgr_managezone() was called and never cleared. Now, zones
properly attach to and detach from their loops.

The zone timer depends on its loop. The shutdown crashes occurred
when asynchronous calls tried to modify the zone timer after
dns_zonemgr_releasezone() has been called and the loop was
invalidated. In these cases the attempt to set the timer is now
ignored, with a debug log message.
---

diff --git a/lib/dns/zone.c b/lib/dns/zone.c
index 7481d3b12f1..be7eb70c90c 100644
--- a/lib/dns/zone.c
+++ b/lib/dns/zone.c
@@ -10619,8 +10619,6 @@ failure:
 cleanup:
 	dns_db_detach(&kfetch->db);
 
-	/* The zone must be managed */
-	INSIST(kfetch->zone->loop != NULL);
 	isc_refcount_decrement(&zone->irefs);
 
 	if (dns_rdataset_isassociated(keydataset)) {
@@ -14658,12 +14656,15 @@ zone_timer_set(dns_zone_t *zone, isc_time_t *next, isc_time_t *now) {
 		isc_time_subtract(next, now, &interval);
 	}
 
-	if (zone->timer == NULL) {
+	if (zone->loop == NULL) {
+		zone_debuglog(zone, __func__, 10, "zone is not managed");
+	} else if (zone->timer == NULL) {
 		isc_refcount_increment0(&zone->irefs);
 		isc_timer_create(zone->loop, zone_timer, zone, &zone->timer);
 	}
-
-	isc_timer_start(zone->timer, isc_timertype_once, &interval);
+	if (zone->timer != NULL) {
+		isc_timer_start(zone->timer, isc_timertype_once, &interval);
+	}
 }
 
 static void
@@ -18169,7 +18170,8 @@ dns_zonemgr_managezone(dns_zonemgr_t *zmgr, dns_zone_t *zone) {
 	REQUIRE(zone->timer == NULL);
 	REQUIRE(zone->zmgr == NULL);
 
-	zone->loop = isc_loop_get(zmgr->loopmgr, zone->tid);
+	isc_loop_t *loop = isc_loop_get(zmgr->loopmgr, zone->tid);
+	isc_loop_attach(loop, &zone->loop);
 
 	zonemgr_keymgmt_add(zmgr, zone, &zone->kfio);
 	INSIST(zone->kfio != NULL);
@@ -18200,6 +18202,13 @@ dns_zonemgr_releasezone(dns_zonemgr_t *zmgr, dns_zone_t *zone) {
 		ENSURE(zone->kfio == NULL);
 	}
 
+	if (zone->timer != NULL) {
+		isc_refcount_decrement(&zone->irefs);
+		isc_timer_destroy(&zone->timer);
+	}
+
+	isc_loop_detach(&zone->loop);
+
 	/* Detach below, outside of the write lock. */
 	zone->zmgr = NULL;
 
@@ -20658,8 +20667,6 @@ done:
 	}
 
 cleanup:
-	/* The zone must be managed */
-	INSIST(nsfetch->zone->loop != NULL);
 	isc_refcount_decrement(&zone->irefs);
 
 	if (dns_rdataset_isassociated(nsrrset)) {
@@ -21922,7 +21929,7 @@ dns_zone_link(dns_zone_t *zone, dns_zone_t *raw) {
 	LOCK_ZONE(zone);
 	LOCK_ZONE(raw);
 
-	raw->loop = zone->loop;
+	isc_loop_attach(zone->loop, &raw->loop);
 
 	/* dns_zone_attach(raw, &zone->raw); */
 	isc_refcount_increment(&raw->references);