From af37c6f60caf258c8d71024f3969361d35f57237 Mon Sep 17 00:00:00 2001 From: Evan Hunt Date: Wed, 27 Apr 2022 22:12:01 -0700 Subject: [PATCH 1/2] 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. (cherry picked from commit 5c4cf3fcc43a481d77c22cf3c70c736467651cd3) --- lib/ns/interfacemgr.c | 34 +++++++++++++++++++--------------- 1 file changed, 19 insertions(+), 15 deletions(-) diff --git a/lib/ns/interfacemgr.c b/lib/ns/interfacemgr.c index 1df13b35b0..9cacd3f02b 100644 --- a/lib/ns/interfacemgr.c +++ b/lib/ns/interfacemgr.c @@ -740,10 +740,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); @@ -777,26 +773,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 From d7c2752d3ce76b2877fed9543f9c7d8f96f5cc35 Mon Sep 17 00:00:00 2001 From: Evan Hunt Date: Wed, 27 Apr 2022 22:17:00 -0700 Subject: [PATCH 2/2] CHANGES for [GL #3272] (cherry picked from commit 6bcf3e5c31043b1c188c0f5ee84c9eff12b501ae) --- CHANGES | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGES b/CHANGES index d3f2043dec..1846b63c98 100644 --- a/CHANGES +++ b/CHANGES @@ -1,3 +1,7 @@ +5875. [bug] Fixed a deadlock that could occur if an rndc + connection arrived during the shutdown of network + interfaces. [GL #3272] + 5873. [bug] Refactor the fctx_done() function to set fctx to NULL after detaching, so that reference counting errors will be easier to avoid. [GL #2969]