From 718e92c31a6148c586be1e48a320ec369ff26f0a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Sur=C3=BD?= Date: Wed, 24 Aug 2022 14:59:50 +0200 Subject: [PATCH] Clear the callbacks when isc_nm_stoplistening() is called When we are closing the listening sockets, there's a time window in which the TCP connection could be accepted although the respective stoplistening function has already returned to control to the caller. Clear the accept callback function early, so it doesn't get called when we are not interested in the incoming connections anymore. --- lib/isc/netmgr/http.c | 19 ++++--------------- lib/isc/netmgr/netmgr-int.h | 6 ------ lib/isc/netmgr/netmgr.c | 2 -- lib/isc/netmgr/tcp.c | 3 +++ lib/isc/netmgr/tcpdns.c | 1 + lib/isc/netmgr/tlsdns.c | 3 ++- lib/isc/netmgr/tlsstream.c | 15 +++++++++------ lib/isc/netmgr/udp.c | 4 ++++ 8 files changed, 23 insertions(+), 30 deletions(-) diff --git a/lib/isc/netmgr/http.c b/lib/isc/netmgr/http.c index 2ad1cf3142..319e7e7005 100644 --- a/lib/isc/netmgr/http.c +++ b/lib/isc/netmgr/http.c @@ -2708,29 +2708,18 @@ void isc__nm_http_stoplistening(isc_nmsocket_t *sock) { REQUIRE(VALID_NMSOCK(sock)); REQUIRE(sock->type == isc_nm_httplistener); + REQUIRE(isc_tid() == sock->tid); if (!atomic_compare_exchange_strong(&sock->closing, &(bool){ false }, true)) { UNREACHABLE(); } - REQUIRE(isc_tid() == sock->tid); - isc__netievent_httpstop_t ievent = { .sock = sock }; - isc__nm_async_httpstop(NULL, (isc__netievent_t *)&ievent); -} - -void -isc__nm_async_httpstop(isc__networker_t *worker, isc__netievent_t *ev0) { - isc__netievent_httpstop_t *ievent = (isc__netievent_httpstop_t *)ev0; - isc_nmsocket_t *sock = ievent->sock; - - UNUSED(worker); - - REQUIRE(VALID_NMSOCK(sock)); - atomic_store(&sock->listening, false); - atomic_store(&sock->closing, false); atomic_store(&sock->closed, true); + sock->recv_cb = NULL; + sock->recv_cbarg = NULL; + if (sock->outer != NULL) { isc_nm_stoplistening(sock->outer); isc_nmsocket_close(&sock->outer); diff --git a/lib/isc/netmgr/netmgr-int.h b/lib/isc/netmgr/netmgr-int.h index 1479bfe1b5..9f2d76ded6 100644 --- a/lib/isc/netmgr/netmgr-int.h +++ b/lib/isc/netmgr/netmgr-int.h @@ -305,7 +305,6 @@ typedef enum isc__netievent_type { netievent_tcpdnsstop, netievent_tlsdnslisten, netievent_tlsdnsstop, - netievent_httpstop, netievent_detach, } isc__netievent_type; @@ -1697,9 +1696,6 @@ isc__nm_http_verify_tls_peer_result_string(const isc_nmhandle_t *handle); void isc__nm_async_httpsend(isc__networker_t *worker, isc__netievent_t *ev0); -void -isc__nm_async_httpstop(isc__networker_t *worker, isc__netievent_t *ev0); - void isc__nm_async_httpclose(isc__networker_t *worker, isc__netievent_t *ev0); @@ -1861,7 +1857,6 @@ NETIEVENT_SOCKET_QUOTA_TYPE(tlsdnsaccept); NETIEVENT_SOCKET_TYPE(tlsdnscycle); #ifdef HAVE_LIBNGHTTP2 -NETIEVENT_SOCKET_TYPE(httpstop); NETIEVENT_SOCKET_REQ_TYPE(httpsend); NETIEVENT_SOCKET_TYPE(httpclose); NETIEVENT_SOCKET_HTTP_EPS_TYPE(httpendpoints); @@ -1921,7 +1916,6 @@ NETIEVENT_SOCKET_QUOTA_DECL(tlsdnsaccept); NETIEVENT_SOCKET_DECL(tlsdnscycle); #ifdef HAVE_LIBNGHTTP2 -NETIEVENT_SOCKET_DECL(httpstop); NETIEVENT_SOCKET_REQ_DECL(httpsend); NETIEVENT_SOCKET_DECL(httpclose); NETIEVENT_SOCKET_HTTP_EPS_DECL(httpendpoints); diff --git a/lib/isc/netmgr/netmgr.c b/lib/isc/netmgr/netmgr.c index 974f882914..be44e59f6f 100644 --- a/lib/isc/netmgr/netmgr.c +++ b/lib/isc/netmgr/netmgr.c @@ -488,7 +488,6 @@ process_netievent(void *arg) { NETIEVENT_CASE(tlsdobio); NETIEVENT_CASE(tlscancel); - NETIEVENT_CASE(httpstop); NETIEVENT_CASE(httpsend); NETIEVENT_CASE(httpclose); NETIEVENT_CASE(httpendpoints); @@ -560,7 +559,6 @@ NETIEVENT_SOCKET_DEF(tlsdnscycle); NETIEVENT_SOCKET_DEF(tlsdnsshutdown); #ifdef HAVE_LIBNGHTTP2 -NETIEVENT_SOCKET_DEF(httpstop); NETIEVENT_SOCKET_REQ_DEF(httpsend); NETIEVENT_SOCKET_DEF(httpclose); NETIEVENT_SOCKET_HTTP_EPS_DEF(httpendpoints); diff --git a/lib/isc/netmgr/tcp.c b/lib/isc/netmgr/tcp.c index 60b7a46eb0..82953cb0d9 100644 --- a/lib/isc/netmgr/tcp.c +++ b/lib/isc/netmgr/tcp.c @@ -967,6 +967,8 @@ accept_connection(isc_nmsocket_t *ssock, isc_quota_t *quota) { return (ISC_R_CANCELED); } + REQUIRE(ssock->accept_cb != NULL); + csock = isc_mem_get(ssock->worker->mctx, sizeof(isc_nmsocket_t)); isc__nmsocket_init(csock, ssock->worker, isc_nm_tcpsocket, &ssock->iface); @@ -1249,6 +1251,7 @@ tcp_close_direct(isc_nmsocket_t *sock) { isc_quota_detach(&sock->quota); } + isc__nmsocket_clearcb(sock); isc__nmsocket_timer_stop(sock); isc__nm_stop_reading(sock); diff --git a/lib/isc/netmgr/tcpdns.c b/lib/isc/netmgr/tcpdns.c index 203d18f8c4..172d5b0740 100644 --- a/lib/isc/netmgr/tcpdns.c +++ b/lib/isc/netmgr/tcpdns.c @@ -1300,6 +1300,7 @@ tcpdns_close_direct(isc_nmsocket_t *sock) { isc_nmhandle_detach(&sock->recv_handle); } + isc__nmsocket_clearcb(sock); isc__nmsocket_timer_stop(sock); isc__nm_stop_reading(sock); diff --git a/lib/isc/netmgr/tlsdns.c b/lib/isc/netmgr/tlsdns.c index 9bd88e0fe7..a4ae9755e6 100644 --- a/lib/isc/netmgr/tlsdns.c +++ b/lib/isc/netmgr/tlsdns.c @@ -1172,10 +1172,10 @@ tls_cycle_input(isc_nmsocket_t *sock) { if (SSL_is_server(sock->tls.tls)) { REQUIRE(sock->recv_handle != NULL); + REQUIRE(sock->accept_cb != NULL); result = sock->accept_cb(sock->recv_handle, ISC_R_SUCCESS, sock->accept_cbarg); - if (result != ISC_R_SUCCESS) { isc_nmhandle_detach(&sock->recv_handle); goto failure; @@ -1940,6 +1940,7 @@ tlsdns_close_direct(isc_nmsocket_t *sock) { isc_nmhandle_detach(&sock->recv_handle); } + isc__nmsocket_clearcb(sock); isc__nmsocket_timer_stop(sock); isc__nm_stop_reading(sock); diff --git a/lib/isc/netmgr/tlsstream.c b/lib/isc/netmgr/tlsstream.c index 8a61d327ec..f63bd479fe 100644 --- a/lib/isc/netmgr/tlsstream.c +++ b/lib/isc/netmgr/tlsstream.c @@ -340,9 +340,13 @@ tls_try_handshake(isc_nmsocket_t *sock, isc_result_t *presult) { isc__nmsocket_log_tls_session_reuse(sock, sock->tlsstream.tls); tlshandle = isc__nmhandle_get(sock, &sock->peer, &sock->iface); if (sock->tlsstream.server) { - result = sock->listener->accept_cb( - tlshandle, result, - sock->listener->accept_cbarg); + if (sock->listener->accept_cb == NULL) { + result = ISC_R_CANCELED; + } else { + result = sock->listener->accept_cb( + tlshandle, result, + sock->listener->accept_cbarg); + } } else { tls_call_connect_cb(sock, tlshandle, result); } @@ -931,6 +935,8 @@ void isc__nm_tls_stoplistening(isc_nmsocket_t *sock) { REQUIRE(VALID_NMSOCK(sock)); REQUIRE(sock->type == isc_nm_tlslistener); + REQUIRE(sock->tlsstream.tls == NULL); + REQUIRE(sock->tlsstream.ctx == NULL); if (!atomic_compare_exchange_strong(&sock->closing, &(bool){ false }, true)) { @@ -942,9 +948,6 @@ isc__nm_tls_stoplistening(isc_nmsocket_t *sock) { sock->recv_cb = NULL; sock->recv_cbarg = NULL; - INSIST(sock->tlsstream.tls == NULL); - INSIST(sock->tlsstream.ctx == NULL); - if (sock->outer != NULL) { isc_nm_stoplistening(sock->outer); isc__nmsocket_detach(&sock->outer); diff --git a/lib/isc/netmgr/udp.c b/lib/isc/netmgr/udp.c index ee8e4caf29..31a638bb15 100644 --- a/lib/isc/netmgr/udp.c +++ b/lib/isc/netmgr/udp.c @@ -1060,6 +1060,10 @@ isc__nm_udp_close(isc_nmsocket_t *sock) { return; } + isc__nmsocket_clearcb(sock); + isc__nmsocket_timer_stop(sock); + isc__nm_stop_reading(sock); + uv_close((uv_handle_t *)&sock->read_timer, read_timer_close_cb); }