]> git.ipfire.org Git - thirdparty/bind9.git/commitdiff
Fix deadlock in isc_nm_tlsconnect()
authorArtem Boldariev <artem@boldariev.com>
Thu, 11 Feb 2021 13:03:44 +0000 (15:03 +0200)
committerArtem Boldariev <artem@boldariev.com>
Fri, 5 Mar 2021 16:09:22 +0000 (18:09 +0200)
when called from within the context of a network thread,
isc_nm_tlsconnect() hangs. it is waiting for the socket's
result code to be updated, but that update is supposed to happen
asynchronously in the network thread, and if we're already blocking
in the network thread, it can never occur.

we can kluge around this by setting the socket result code
early; this works for most clients (including "dig"), but it causes
inconsistent behaviors that manifest as test failures in the DoH unit
test.

so we kluged around it even more by setting the socket result code
early *only when running in the network thread*. we need a better
solution for this problem, but this will do for now.

lib/isc/netmgr/tlsstream.c

index 9a75dc0f2aca470f61c1ab1e626b82d2d837adb6..5505f3e2b46d17b1a51da65aeaf43d878cf99dfd 100644 (file)
@@ -78,18 +78,21 @@ inactive(isc_nmsocket_t *sock) {
 
 static void
 update_result(isc_nmsocket_t *sock, const isc_result_t result) {
-       if (!sock->tlsstream.server) {
-               LOCK(&sock->lock);
-               sock->result = result;
-               SIGNAL(&sock->cond);
-               while (!atomic_load(&sock->active)) {
-                       WAIT(&sock->scond, &sock->lock);
+       if (!atomic_load(&sock->tlsstream.result_updated)) {
+               atomic_store(&sock->tlsstream.result_updated, true);
+               if (!sock->tlsstream.server) {
+                       LOCK(&sock->lock);
+                       sock->result = result;
+                       SIGNAL(&sock->cond);
+                       while (!atomic_load(&sock->active)) {
+                               WAIT(&sock->scond, &sock->lock);
+                       }
+                       UNLOCK(&sock->lock);
+               } else {
+                       LOCK(&sock->lock);
+                       sock->result = result;
+                       UNLOCK(&sock->lock);
                }
-               UNLOCK(&sock->lock);
-       } else {
-               LOCK(&sock->lock);
-               sock->result = result;
-               UNLOCK(&sock->lock);
        }
 }
 
@@ -751,6 +754,10 @@ isc_nm_tlsconnect(isc_nm_t *mgr, isc_nmiface_t *local, isc_nmiface_t *peer,
        isc_nmsocket_t *nsock = NULL, *tsock = NULL;
        isc__netievent_tlsconnect_t *ievent = NULL;
        isc_result_t result = ISC_R_DEFAULT;
+#if defined(NETMGR_TRACE) && defined(NETMGR_TRACE_VERBOSE)
+       fprintf(stderr, "TLS: isc_nm_tlsconnect(): in net thread: %s\n",
+               isc__nm_in_netthread() ? "yes" : "no");
+#endif /* NETMGR_TRACE */
 
        REQUIRE(VALID_NM(mgr));
 
@@ -765,6 +772,7 @@ isc_nm_tlsconnect(isc_nm_t *mgr, isc_nmiface_t *local, isc_nmiface_t *peer,
        nsock->connect_cbarg = cbarg;
        nsock->connect_timeout = timeout;
        nsock->tlsstream.ctx = ctx;
+       nsock->tlsstream.connect_from_networker = isc__nm_in_netthread();
 
        ievent = isc__nm_get_netievent_tlsconnect(mgr, nsock);
        ievent->local = local->addr;
@@ -844,16 +852,19 @@ isc__nm_async_tlsconnect(isc__networker_t *worker, isc__netievent_t *ev0) {
        tlssock->tlsstream.tls = isc_tls_create(tlssock->tlsstream.ctx);
        if (tlssock->tlsstream.tls == NULL) {
                result = ISC_R_TLSERROR;
-               update_result(tlssock, result);
                goto error;
        }
 
        tlssock->tid = isc_nm_tid();
        tlssock->tlsstream.state = TLS_INIT;
 
-       (void)isc_nm_tcpconnect(worker->mgr, (isc_nmiface_t *)&ievent->local,
-                               (isc_nmiface_t *)&ievent->peer, tcp_connected,
-                               tlssock, tlssock->connect_timeout, 0);
+       result = isc_nm_tcpconnect(worker->mgr, (isc_nmiface_t *)&ievent->local,
+                                  (isc_nmiface_t *)&ievent->peer,
+                                  tcp_connected, tlssock,
+                                  tlssock->connect_timeout, 0);
+       if (tlssock->tlsstream.connect_from_networker) {
+               update_result(tlssock, result);
+       }
        return;
 
 error: