]> git.ipfire.org Git - thirdparty/bind9.git/commitdiff
clean up outerhandle when a tcpdns socket is disconnected
authorEvan Hunt <each@isc.org>
Sat, 20 Jun 2020 22:03:05 +0000 (15:03 -0700)
committerEvan Hunt <each@isc.org>
Fri, 26 Jun 2020 07:19:42 +0000 (00:19 -0700)
this prevents a crash when some non-netmgr thread, such as a
recursive lookup, times out after the TCP socket is already
disconnected.

lib/isc/netmgr/netmgr-int.h
lib/isc/netmgr/netmgr.c
lib/isc/netmgr/tcp.c
lib/isc/netmgr/tcpdns.c

index b0b4833e4bd3a9f0adf9b3c5cce93d76eddeb8a6..71b2d20c2c60f931a2085ebfd0f5775d18524abb 100644 (file)
@@ -98,14 +98,6 @@ struct isc_nmhandle {
        isc_nmsocket_t *sock;
        size_t ah_pos; /* Position in the socket's 'active handles' array */
 
-       /*
-        * The handle is 'inflight' if netmgr is not currently processing
-        * it in any way - it might mean that e.g. a recursive resolution
-        * is happening. For an inflight handle we must wait for the
-        * calling code to finish before we can free it.
-        */
-       atomic_bool inflight;
-
        isc_sockaddr_t peer;
        isc_sockaddr_t local;
        isc_nm_opaquecb_t doreset; /* reset extra callback, external */
@@ -650,6 +642,12 @@ isc__nmsocket_active(isc_nmsocket_t *sock);
  * or, for child sockets, 'sock->parent->active'.
  */
 
+void
+isc__nmsocket_clearcb(isc_nmsocket_t *sock);
+/*%<
+ * Clear the recv and accept callbacks in 'sock'.
+ */
+
 void
 isc__nm_async_closecb(isc__networker_t *worker, isc__netievent_t *ev0);
 /*%<
index 7fd029178c1906a4dae035c89342ef46db95384d..dafe203601aaa7c91e2e511610e779f562f84c8c 100644 (file)
@@ -987,6 +987,16 @@ isc__nmsocket_init(isc_nmsocket_t *sock, isc_nm_t *mgr, isc_nmsocket_type type,
        sock->magic = NMSOCK_MAGIC;
 }
 
+void
+isc__nmsocket_clearcb(isc_nmsocket_t *sock) {
+       REQUIRE(VALID_NMSOCK(sock));
+
+       sock->rcb.recv = NULL;
+       sock->rcbarg = NULL;
+       sock->accept_cb.accept = NULL;
+       sock->accept_cbarg = NULL;
+}
+
 void
 isc__nm_alloc_cb(uv_handle_t *handle, size_t size, uv_buf_t *buf) {
        isc_nmsocket_t *sock = uv_handle_get_data(handle);
index f38f31070bf874aa6d71e082461898b9cbccbb7e..e3e222dead4ed572ae3735a1e2e8bcf12ef99464 100644 (file)
@@ -584,6 +584,7 @@ readtimeout_cb(uv_timer_t *handle) {
        if (sock->rcb.recv != NULL) {
                sock->rcb.recv(sock->tcphandle, ISC_R_TIMEDOUT, NULL,
                               sock->rcbarg);
+               isc__nmsocket_clearcb(sock);
        }
 }
 
@@ -682,7 +683,9 @@ isc__nm_tcp_resumeread(isc_nmsocket_t *sock) {
        isc__netievent_startread_t *ievent = NULL;
 
        REQUIRE(VALID_NMSOCK(sock));
-       REQUIRE(sock->rcb.recv != NULL);
+       if (sock->rcb.recv == NULL) {
+               return (ISC_R_CANCELED);
+       }
 
        if (!atomic_load(&sock->readpaused)) {
                return (ISC_R_SUCCESS);
@@ -744,6 +747,7 @@ read_cb(uv_stream_t *stream, ssize_t nread, const uv_buf_t *buf) {
        if (sock->rcb.recv != NULL) {
                isc__nm_incstats(sock->mgr, sock->statsindex[STATID_RECVFAIL]);
                sock->rcb.recv(sock->tcphandle, ISC_R_EOF, NULL, sock->rcbarg);
+               isc__nmsocket_clearcb(sock);
        }
 
        /*
@@ -1080,6 +1084,7 @@ isc__nm_tcp_shutdown(isc_nmsocket_t *sock) {
        {
                sock->rcb.recv(sock->tcphandle, ISC_R_CANCELED, NULL,
                               sock->rcbarg);
+               isc__nmsocket_clearcb(sock);
        }
 }
 
@@ -1092,5 +1097,6 @@ isc__nm_tcp_cancelread(isc_nmsocket_t *sock) {
        {
                sock->rcb.recv(sock->tcphandle, ISC_R_CANCELED, NULL,
                               sock->rcbarg);
+               isc__nmsocket_clearcb(sock);
        }
 }
index 19a31870f2de64d35ebe4672828778934f2963ee..a4d3f37dce85dd0ae59a9e39805e07fc5c3e9330 100644 (file)
@@ -231,6 +231,11 @@ dnslisten_readcb(isc_nmhandle_t *handle, isc_result_t eresult,
                if (dnssock->self != NULL) {
                        isc__nmsocket_detach(&dnssock->self);
                }
+               isc__nmsocket_clearcb(dnssock);
+               if (dnssock->outerhandle != NULL) {
+                       isc_nmhandle_unref(dnssock->outerhandle);
+                       dnssock->outerhandle = NULL;
+               }
                return;
        }
 
@@ -340,8 +345,7 @@ isc__nm_tcpdns_stoplistening(isc_nmsocket_t *sock) {
 
        atomic_store(&sock->listening, false);
        atomic_store(&sock->closed, true);
-       sock->rcb.recv = NULL;
-       sock->rcbarg = NULL;
+       isc__nmsocket_clearcb(sock);
 
        if (sock->outer != NULL) {
                isc__nm_tcp_stoplistening(sock->outer);
@@ -431,7 +435,11 @@ resume_processing(void *arg) {
                        }
                        isc_nmhandle_unref(handle);
                } else if (sock->outerhandle != NULL) {
-                       isc_nm_resumeread(sock->outerhandle->sock);
+                       result = isc_nm_resumeread(sock->outerhandle->sock);
+                       if (result != ISC_R_SUCCESS) {
+                               isc_nmhandle_unref(sock->outerhandle);
+                               sock->outerhandle = NULL;
+                       }
                }
 
                return;
@@ -524,7 +532,9 @@ tcpdns_close_direct(isc_nmsocket_t *sock) {
        REQUIRE(sock->tid == isc_nm_tid());
 
        /* We don't need atomics here, it's all in single network thread */
-       if (sock->timer_initialized) {
+       if (sock->self != NULL) {
+               isc__nmsocket_detach(&sock->self);
+       } else if (sock->timer_initialized) {
                /*
                 * We need to fire the timer callback to clean it up,
                 * it will then call us again (via detach) so that we
@@ -533,15 +543,13 @@ tcpdns_close_direct(isc_nmsocket_t *sock) {
                sock->timer_initialized = false;
                uv_timer_stop(&sock->timer);
                uv_close((uv_handle_t *)&sock->timer, timer_close_cb);
-       } else if (sock->self != NULL) {
-               isc__nmsocket_detach(&sock->self);
        } else {
                /*
                 * At this point we're certain that there are no external
                 * references, we can close everything.
                 */
                if (sock->outerhandle != NULL) {
-                       sock->outerhandle->sock->rcb.recv = NULL;
+                       isc__nmsocket_clearcb(sock->outerhandle->sock);
                        isc_nmhandle_unref(sock->outerhandle);
                        sock->outerhandle = NULL;
                }