netmgr: have a single source of truth for tcpdns callback

We pass interface as an opaque argument to tcpdns listening socket.
If we stop listening on an interface but still have in-flight connections
the opaque 'interface' is not properly reference counted, and we might
hit a dead memory. We put just a single source of truth in a listening
socket and make the child sockets use that instead of copying the
value from listening socket. We clean the callback when we stop listening.

diff --git a/lib/isc/netmgr/netmgr-int.h b/lib/isc/netmgr/netmgr-int.h
index cf9478f171f71952cec9eb7af2ea9f0bcd2f0a14..177adec3c6330ad79424e23a8c15c50bff87dcf5 100644 (file)
--- a/lib/isc/netmgr/netmgr-int.h
+++ b/lib/isc/netmgr/netmgr-int.h
@@ -347,7 +347,10 @@ struct isc_nmsocket {
        int                             tid;
        isc_nmsocket_type               type;
        isc_nm_t                        *mgr;
+       /*% Parent socket for multithreaded listeners */
        isc_nmsocket_t                  *parent;
+       /*% Listener socket this connection was accepted on */
+       isc_nmsocket_t                  *listener;
 
        /*%
         * quota is the TCP client, attached when a TCP connection

diff --git a/lib/isc/netmgr/tcpdns.c b/lib/isc/netmgr/tcpdns.c
index 7aa90fefcdd07f8527bb2029ccd11e42b3733555..a15cb07b9645d99abf78537c64f41d180346a255 100644 (file)
--- a/lib/isc/netmgr/tcpdns.c
+++ b/lib/isc/netmgr/tcpdns.c
@@ -121,10 +121,8 @@ dnslisten_acceptcb(isc_nmhandle_t *handle, isc_result_t result, void *cbarg) {
        isc__nmsocket_init(dnssock, handle->sock->mgr,
                           isc_nm_tcpdnssocket, handle->sock->iface);
 
-       /* We need to copy read callbacks from outer socket */
-       dnssock->rcb.recv = dnslistensock->rcb.recv;
-       dnssock->rcbarg = dnslistensock->rcbarg;
        dnssock->extrahandlesize = dnslistensock->extrahandlesize;
+       isc_nmsocket_attach(dnslistensock, &dnssock->listener);
        isc_nmsocket_attach(handle->sock, &dnssock->outer);
        dnssock->peer = handle->sock->peer;
        dnssock->read_timeout = handle->sock->mgr->init;
@@ -171,14 +169,17 @@ processbuffer(isc_nmsocket_t *dnssock, isc_nmhandle_t **handlep) {
         */
        len = dnslen(dnssock->buf);
        if (len <= dnssock->buf_len - 2) {
-               isc_nmhandle_t *dnshandle = NULL;
-               isc_region_t r2 = {
-                       .base = dnssock->buf + 2,
-                       .length = len
-               };
-
-               dnshandle = isc__nmhandle_get(dnssock, NULL, NULL);
-               dnssock->rcb.recv(dnshandle, &r2, dnssock->rcbarg);
+               isc_nmhandle_t *dnshandle = isc__nmhandle_get(dnssock,
+                                                             NULL, NULL);
+               isc_nmsocket_t *listener = dnssock->listener;
+
+               if (listener != NULL && listener->rcb.recv != NULL) {
+                       listener->rcb.recv(dnshandle,
+                                          &(isc_region_t){
+                                               .base = dnssock->buf + 2,
+                                               .length = len
+                                          }, listener->rcbarg);
+               }
 
                len += 2;
                dnssock->buf_len -= len;
@@ -322,6 +323,8 @@ 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;
 
        if (sock->outer != NULL) {
                isc_nm_tcp_stoplistening(sock->outer);
@@ -502,6 +505,9 @@ tcpdns_close_direct(isc_nmsocket_t *sock) {
                sock->outer->rcb.recv = NULL;
                isc_nmsocket_detach(&sock->outer);
        }
+       if (sock->listener != NULL) {
+               isc_nmsocket_detach(&sock->listener);
+       }
        /* We don't need atomics here, it's all in single network thread */
        if (sock->timer_initialized) {
                sock->timer_initialized = false;

diff --git a/lib/ns/client.c b/lib/ns/client.c
index a7878aa8a0cbec39275b487f3a01cd04ccc0dc0e..ad99883a23e50841807c0419cdbf79ce2c2f992d 100644 (file)
--- a/lib/ns/client.c
+++ b/lib/ns/client.c
@@ -1625,6 +1625,11 @@ ns__client_request(isc_nmhandle_t *handle, isc_region_t *region, void *arg) {
        ifp = (ns_interface_t *) arg;
 
        mgr = ifp->clientmgr;
+       if (mgr == NULL) {
+               /* The interface was shut down in the meantime, just bail */
+               return;
+       }
+
        REQUIRE(VALID_MANAGER(mgr));
 
        client = isc_nmhandle_getdata(handle);