]> git.ipfire.org Git - thirdparty/bind9.git/commitdiff
TLS: Avoid accessing non-atomic listener socket flags during HS
authorArtem Boldariev <artem@boldariev.com>
Thu, 1 Dec 2022 20:42:54 +0000 (22:42 +0200)
committerArtem Boldariev <artem@boldariev.com>
Fri, 2 Dec 2022 10:16:12 +0000 (12:16 +0200)
This commit ensures that the non-atomic flags inside a TLS listener
socket object (and associated worker) are accessed when doing
handshake for a connection only from within the context of the
dedicated thread, but not other worker threads.

The purpose of this commit is to avoid TSAN errors during
isc__nmsocket_closing() calls. It is a continuation of
4b5559cd8f41e67bf03e92d9d0338dee9d549b48.

lib/isc/netmgr/tlsstream.c

index 4a2497d52617030b5d481b61328d6b60d7172e57..1ac83f4c7ee61ce4d4addf98fd7a73a3813cf395 100644 (file)
@@ -351,7 +351,30 @@ tls_try_handshake(isc_nmsocket_t *sock, isc_result_t *presult) {
                tlshandle = isc__nmhandle_get(sock, &sock->peer, &sock->iface);
                tls_read_stop(sock);
                if (sock->tlsstream.server) {
-                       if (isc__nmsocket_closing(sock->listener)) {
+                       /*
+                        * We need to check for 'sock->listener->closing' to
+                        * make sure that we are not breaking the contract by
+                        * calling an accept callback after the listener socket
+                        * was shot down. Also, in this case the accept callback
+                        * can be 'NULL'. That can happen as calling the accept
+                        * callback in TLS is deferred until handshake is done.
+                        * There is a possibility for that to happen *after* the
+                        * underlying TCP connection was accepted. That is, a
+                        * situation possible when the underlying TCP connection
+                        * was accepted, handshake related data transmission
+                        * took place, but in the middle of that the socket is
+                        * being shot down before the TLS accept callback could
+                        * have been called.
+                        *
+                        * Also see 'isc__nmsocket_stop()' - the function used
+                        * to shut down the listening TLS socket - for more
+                        * details.
+                        */
+                       if (isc__nm_closing(sock->worker)) {
+                               result = ISC_R_SHUTTINGDOWN;
+                       } else if (isc__nmsocket_closing(sock) ||
+                                  atomic_load(&sock->listener->closing))
+                       {
                                result = ISC_R_CANCELED;
                        } else {
                                result = sock->listener->accept_cb(