From: Evan Hunt <each@isc.org>
Date: Thu, 28 Apr 2022 05:12:01 +0000 (-0700)
Subject: prevent a deadlock in the shutdown system test
X-Git-Tag: v9.19.1~38^2~1
X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=5c4cf3fcc43a481d77c22cf3c70c736467651cd3;p=thirdparty%2Fbind9.git

prevent a deadlock in the shutdown system test

The shutdown test sends 'rdnc status' commands in parallel with
'rndc stop' A new rndc connection arriving will reference the ACL
environment to see whether the client is allowed to connect.
Commit c0995bc380 added a mutex lock to ns_interfacemgr_getaclenv(),
but if the new connection arrives while the interfaces are being
purged during shutdown, that lock is already being held. If the
the connection event slips in ahead of one of the netmgr's "stop
listening" events on a worker thread, a deadlock can occur.

The fix is not to hold the interfacemgr lock while shutting down
interfaces; only while actually traversing the interface list to
identify interfaces needing shutdown.
---

diff --git a/lib/ns/interfacemgr.c b/lib/ns/interfacemgr.c
index d62b8d92d4b..7b5093e13bb 100644
--- a/lib/ns/interfacemgr.c
+++ b/lib/ns/interfacemgr.c
@@ -742,10 +742,6 @@ interface_destroy(ns_interface_t **interfacep) {
 
 	ns_interface_shutdown(ifp);
 
-	if (ISC_LINK_LINKED(ifp, link)) {
-		ISC_LIST_UNLINK(mgr->interfaces, ifp, link);
-	}
-
 	ifp->magic = 0;
 	isc_mutex_destroy(&ifp->lock);
 	ns_interfacemgr_detach(&ifp->mgr);
@@ -779,26 +775,34 @@ find_matching_interface(ns_interfacemgr_t *mgr, isc_sockaddr_t *addr) {
  */
 static void
 purge_old_interfaces(ns_interfacemgr_t *mgr) {
-	ns_interface_t *ifp, *next;
+	ns_interface_t *ifp = NULL, *next = NULL;
+	ISC_LIST(ns_interface_t) interfaces;
+
+	ISC_LIST_INIT(interfaces);
+
 	LOCK(&mgr->lock);
 	for (ifp = ISC_LIST_HEAD(mgr->interfaces); ifp != NULL; ifp = next) {
 		INSIST(NS_INTERFACE_VALID(ifp));
 		next = ISC_LIST_NEXT(ifp, link);
 		if (ifp->generation != mgr->generation) {
 			ISC_LIST_UNLINK(ifp->mgr->interfaces, ifp, link);
-			if (LISTENING(ifp)) {
-				char sabuf[256];
-				isc_sockaddr_format(&ifp->addr, sabuf,
-						    sizeof(sabuf));
-				isc_log_write(
-					IFMGR_COMMON_LOGARGS, ISC_LOG_INFO,
-					"no longer listening on %s", sabuf);
-				ns_interface_shutdown(ifp);
-			}
-			interface_destroy(&ifp);
+			ISC_LIST_APPEND(interfaces, ifp, link);
 		}
 	}
 	UNLOCK(&mgr->lock);
+
+	for (ifp = ISC_LIST_HEAD(interfaces); ifp != NULL; ifp = next) {
+		next = ISC_LIST_NEXT(ifp, link);
+		if (LISTENING(ifp)) {
+			char sabuf[256];
+			isc_sockaddr_format(&ifp->addr, sabuf, sizeof(sabuf));
+			isc_log_write(IFMGR_COMMON_LOGARGS, ISC_LOG_INFO,
+				      "no longer listening on %s", sabuf);
+			ns_interface_shutdown(ifp);
+		}
+		ISC_LIST_UNLINK(interfaces, ifp, link);
+		interface_destroy(&ifp);
+	}
 }
 
 static bool