From: Witold Kręcicki <wpk@isc.org>
Date: Wed, 22 May 2019 10:37:03 +0000 (+0200)
Subject: Fix a possible deadlock in TCP accepting
X-Git-Tag: v9.15.1~35^2
X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=75815c15813cc2e9bedf9b34a19d3899d7605e4a;p=thirdparty%2Fbind9.git

Fix a possible deadlock in TCP accepting

Each network thread holds an array of locks, indexed by a hash
of fd. When we accept a connection we hold a lock in accepting thread.
We then generate the thread number and lock bucket for the new
connection socket - if we hit the same thread and lock bucket as
accepting socket we get a deadlock. Avoid this by checking if we're
in the same thread/lock bucket and not locking in this case.
---

diff --git a/CHANGES b/CHANGES
index 53bf0b65f40..335b9b2b02b 100644
--- a/CHANGES
+++ b/CHANGES
@@ -1,3 +1,5 @@
+5238.	[bug]		Fix a possible deadlock in TCP code. [GL #1046]
+
 5237.	[bug]		Recurse to find the root server list with 'dig +trace'.
 			[GL #1028]
 
diff --git a/lib/isc/unix/socket.c b/lib/isc/unix/socket.c
index d62af359d51..fe187028c23 100644
--- a/lib/isc/unix/socket.c
+++ b/lib/isc/unix/socket.c
@@ -2977,6 +2977,13 @@ internal_accept(isc__socket_t *sock) {
 		NEWCONNSOCK(dev)->connected = 1;
 		nthread = &manager->threads[NEWCONNSOCK(dev)->threadid];
 
+		/*
+		 * We already hold a lock on one fdlock in accepting thread,
+		 * we need to make sure that we don't double lock.
+		 */
+		bool same_bucket = (sock->threadid == NEWCONNSOCK(dev)->threadid) &&
+				   (FDLOCK_ID(sock->fd) == lockid);
+
 		/*
 		 * Use minimum mtu if possible.
 		 */
@@ -2999,13 +3006,17 @@ internal_accept(isc__socket_t *sock) {
 			NEWCONNSOCK(dev)->active = 1;
 		}
 
-		LOCK(&nthread->fdlock[lockid]);
+		if (!same_bucket) {
+			LOCK(&nthread->fdlock[lockid]);
+		}
 		nthread->fds[fd] = NEWCONNSOCK(dev);
 		nthread->fdstate[fd] = MANAGED;
 #if defined(USE_EPOLL)
 		nthread->epoll_events[fd] = 0;
 #endif
-		UNLOCK(&nthread->fdlock[lockid]);
+		if (!same_bucket) {
+			UNLOCK(&nthread->fdlock[lockid]);
+		}
 
 		LOCK(&manager->lock);