mir/bind
2
0
Fork 0
mirror of https://gitlab.isc.org/isc-projects/bind9 synced 2025-08-29 05:28:00 +00:00
Code Issues Releases Wiki Activity

Fix and clean up handling of connect callbacks

Browse Source 
Serveral problems were discovered and fixed after the change in
the connection timeout in the previous commits:

  * In TLSDNS, the connection callback was not called at all under some
    circumstances when the TCP connection had been established, but the
    TLS handshake hadn't been completed yet.  Additional checks have
    been put in place so that tls_cycle() will end early when the
    nmsocket is invalidated by the isc__nm_tlsdns_shutdown() call.

  * In TCP, TCPDNS and TLSDNS, new connections would be established
    even when the network manager was shutting down.  The new
    call isc__nm_closing() has been added and is used to bail out
    early even before uv_tcp_connect() is attempted.
This commit is contained in:
Ondřej Surý 2021-03-31 11:48:41 +02:00
parent 5a87c7372c
commit 7df8c7061c
6 changed files with 118 additions and 63 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

11
lib/isc/netmgr/netmgr-int.h
View File

@ -1134,6 +1134,13 @@ isc___nmsocket_prep_destroy(isc_nmsocket_t *sock FLARG);
* if there are no remaining references or active handles.
*/
void
isc__nmsocket_shutdown(isc_nmsocket_t *sock);
/*%<
* Initiate the socket shutdown which actively calls the active
* callbacks.
*/
bool
isc__nmsocket_active(isc_nmsocket_t *sock);
/*%<
@ -1876,7 +1883,9 @@ isc__nm_process_sock_buffer(isc_nmsocket_t *sock);
void
isc__nm_resume_processing(void *arg);
bool
isc__nm_inactive(isc_nmsocket_t *sock);
isc__nmsocket_closing(isc_nmsocket_t *sock);
bool
isc__nm_closing(isc_nmsocket_t *sock);
void
isc__nm_alloc_dnsbuf(isc_nmsocket_t *sock, size_t len);

14
lib/isc/netmgr/netmgr.c
View File

@ -1521,7 +1521,7 @@ isc__nmhandle_detach(isc_nmhandle_t **handlep FLARG) {
}
}
static void
void
isc__nmsocket_shutdown(isc_nmsocket_t *sock);
static void
@ -1670,6 +1670,7 @@ isc__nm_failed_connect_cb(isc_nmsocket_t *sock, isc__nm_uvreq_t *req,
REQUIRE(req->cb.connect != NULL);
isc__nmsocket_timer_stop(sock);
uv_handle_set_data((uv_handle_t *)&sock->timer, sock);
atomic_store(&sock->connecting, false);
@ -1930,7 +1931,12 @@ isc__nm_stop_reading(isc_nmsocket_t *sock) {
}
bool
isc__nm_inactive(isc_nmsocket_t *sock) {
isc__nm_closing(isc_nmsocket_t *sock) {
return (atomic_load(&sock->mgr->closing));
}
bool
isc__nmsocket_closing(isc_nmsocket_t *sock) {
return (!isc__nmsocket_active(sock) || atomic_load(&sock->closing) ||
atomic_load(&sock->mgr->closing) ||
(sock->server != NULL && !isc__nmsocket_active(sock->server)));
@ -2017,7 +2023,7 @@ isc__nm_resume_processing(void *arg) {
REQUIRE(sock->tid == isc_nm_tid());
REQUIRE(!atomic_load(&sock->client));
if (isc__nm_inactive(sock)) {
if (isc__nmsocket_closing(sock)) {
return;
}
@ -2461,7 +2467,7 @@ isc__nm_async_detach(isc__networker_t *worker, isc__netievent_t *ev0) {
nmhandle_detach_cb(&ievent->handle FLARG_PASS);
}
static void
void
isc__nmsocket_shutdown(isc_nmsocket_t *sock) {
REQUIRE(VALID_NMSOCK(sock));
switch (sock->type) {

29
lib/isc/netmgr/tcp.c
View File

@ -95,13 +95,6 @@ stop_reading(isc_nmsocket_t *sock);
static void
tcp_alloc_cb(uv_handle_t *handle, size_t size, uv_buf_t *buf);
static bool
inactive(isc_nmsocket_t *sock) {
return (!isc__nmsocket_active(sock) || atomic_load(&sock->closing) ||
atomic_load(&sock->mgr->closing) ||
(sock->server != NULL && !isc__nmsocket_active(sock->server)));
}
static void
failed_accept_cb(isc_nmsocket_t *sock, isc_result_t eresult) {
REQUIRE(sock->accepting);
@ -158,6 +151,12 @@ tcp_connect_direct(isc_nmsocket_t *sock, isc__nm_uvreq_t *req) {
r = uv_timer_init(&worker->loop, &sock->timer);
RUNTIME_CHECK(r == 0);
uv_handle_set_data((uv_handle_t *)&sock->timer, sock);
if (isc__nm_closing(sock)) {
result = ISC_R_CANCELED;
goto error;
}
r = uv_tcp_open(&sock->uv_handle.tcp, sock->fd);
if (r != 0) {
@ -193,7 +192,7 @@ tcp_connect_direct(isc_nmsocket_t *sock, isc__nm_uvreq_t *req) {
done:
result = isc__nm_uverr2result(r);
error:
LOCK(&sock->lock);
sock->result = result;
SIGNAL(&sock->cond);
@ -263,7 +262,7 @@ tcp_connect_cb(uv_connect_t *uvreq, int status) {
*/
isc__nm_uvreq_put(&req, sock);
return;
} else if (!isc__nmsocket_active(sock)) {
} else if (isc__nmsocket_closing(sock)) {
/* Socket was closed midflight by isc__nm_tcp_shutdown() */
result = ISC_R_CANCELED;
goto error;
@ -608,7 +607,7 @@ tcp_connection_cb(uv_stream_t *server, int status) {
REQUIRE(VALID_NMSOCK(ssock));
REQUIRE(ssock->tid == isc_nm_tid());
if (inactive(ssock)) {
if (isc__nmsocket_closing(ssock)) {
result = ISC_R_CANCELED;
goto done;
}
@ -826,7 +825,7 @@ isc__nm_async_tcpstartread(isc__networker_t *worker, isc__netievent_t *ev0) {
REQUIRE(sock->tid == isc_nm_tid());
UNUSED(worker);
if (inactive(sock)) {
if (isc__nmsocket_closing(sock)) {
sock->reading = true;
failed_read_cb(sock, ISC_R_CANCELED);
return;
@ -914,7 +913,7 @@ read_cb(uv_stream_t *stream, ssize_t nread, const uv_buf_t *buf) {
REQUIRE(sock->reading);
REQUIRE(buf != NULL);
if (inactive(sock)) {
if (isc__nmsocket_closing(sock)) {
failed_read_cb(sock, ISC_R_CANCELED);
goto free;
}
@ -1013,7 +1012,7 @@ accept_connection(isc_nmsocket_t *ssock, isc_quota_t *quota) {
REQUIRE(VALID_NMSOCK(ssock));
REQUIRE(ssock->tid == isc_nm_tid());
if (inactive(ssock)) {
if (isc__nmsocket_closing(ssock)) {
if (quota != NULL) {
isc_quota_detach(&quota);
}
@ -1187,7 +1186,7 @@ tcp_send_direct(isc_nmsocket_t *sock, isc__nm_uvreq_t *req) {
int r;
if (inactive(sock)) {
if (isc__nmsocket_closing(sock)) {
return (ISC_R_CANCELED);
}
@ -1417,7 +1416,7 @@ isc__nm_tcp_shutdown(isc_nmsocket_t *sock) {
return;
}
if (sock->statichandle) {
if (sock->statichandle != NULL) {
failed_read_cb(sock, ISC_R_CANCELED);
return;
}

24
lib/isc/netmgr/tcpdns.c
View File

@ -106,6 +106,11 @@ tcpdns_connect_direct(isc_nmsocket_t *sock, isc__nm_uvreq_t *req) {
RUNTIME_CHECK(r == 0);
uv_handle_set_data((uv_handle_t *)&sock->timer, sock);
if (isc__nm_closing(sock)) {
result = ISC_R_CANCELED;
goto error;
}
r = uv_tcp_open(&sock->uv_handle.tcp, sock->fd);
if (r != 0) {
isc__nm_closesocket(sock->fd);
@ -144,7 +149,7 @@ tcpdns_connect_direct(isc_nmsocket_t *sock, isc__nm_uvreq_t *req) {
done:
result = isc__nm_uverr2result(r);
error:
LOCK(&sock->lock);
sock->result = result;
SIGNAL(&sock->cond);
@ -212,7 +217,7 @@ tcpdns_connect_cb(uv_connect_t *uvreq, int status) {
*/
isc__nm_uvreq_put(&req, sock);
return;
} else if (!isc__nmsocket_active(sock)) {
} else if (isc__nmsocket_closing(sock)) {
/* Socket was closed midflight by isc__nm_tcpdns_shutdown() */
result = ISC_R_CANCELED;
goto error;
@ -560,7 +565,7 @@ tcpdns_connection_cb(uv_stream_t *server, int status) {
REQUIRE(VALID_NMSOCK(ssock));
REQUIRE(ssock->tid == isc_nm_tid());
if (isc__nm_inactive(ssock)) {
if (isc__nmsocket_closing(ssock)) {
result = ISC_R_CANCELED;
goto done;
}
@ -714,7 +719,7 @@ isc__nm_async_tcpdnsread(isc__networker_t *worker, isc__netievent_t *ev0) {
REQUIRE(VALID_NMSOCK(sock));
REQUIRE(sock->tid == isc_nm_tid());
if (isc__nm_inactive(sock)) {
if (isc__nmsocket_closing(sock)) {
sock->reading = true;
isc__nm_failed_read_cb(sock, ISC_R_CANCELED);
return;
@ -741,7 +746,7 @@ isc__nm_tcpdns_processbuffer(isc_nmsocket_t *sock) {
REQUIRE(VALID_NMSOCK(sock));
REQUIRE(sock->tid == isc_nm_tid());
if (isc__nm_inactive(sock)) {
if (isc__nmsocket_closing(sock)) {
return (ISC_R_CANCELED);
}
@ -818,7 +823,7 @@ isc__nm_tcpdns_read_cb(uv_stream_t *stream, ssize_t nread,
REQUIRE(sock->reading);
REQUIRE(buf != NULL);
if (isc__nm_inactive(sock)) {
if (isc__nmsocket_closing(sock)) {
isc__nm_failed_read_cb(sock, ISC_R_CANCELED);
goto free;
}
@ -917,7 +922,7 @@ accept_connection(isc_nmsocket_t *ssock, isc_quota_t *quota) {
REQUIRE(VALID_NMSOCK(ssock));
REQUIRE(ssock->tid == isc_nm_tid());
if (isc__nm_inactive(ssock)) {
if (isc__nmsocket_closing(ssock)) {
if (quota != NULL) {
isc_quota_detach(&quota);
}
@ -1111,7 +1116,7 @@ isc__nm_async_tcpdnssend(isc__networker_t *worker, isc__netievent_t *ev0) {
UNUSED(worker);
if (isc__nm_inactive(sock)) {
if (isc__nmsocket_closing(sock)) {
result = ISC_R_CANCELED;
goto fail;
}
@ -1295,6 +1300,7 @@ tcpdns_close_direct(isc_nmsocket_t *sock) {
isc__nmsocket_timer_stop(sock);
isc__nm_stop_reading(sock);
uv_handle_set_data((uv_handle_t *)&sock->timer, sock);
uv_close((uv_handle_t *)&sock->timer, timer_close_cb);
}
@ -1375,7 +1381,7 @@ isc__nm_tcpdns_shutdown(isc_nmsocket_t *sock) {
return;
}
if (sock->statichandle) {
if (sock->statichandle != NULL) {
isc__nm_failed_read_cb(sock, ISC_R_CANCELED);
return;
}

95
lib/isc/netmgr/tlsdns.c
View File

@ -110,6 +110,10 @@ tlsdns_connect_direct(isc_nmsocket_t *sock, isc__nm_uvreq_t *req) {
atomic_store(&sock->connecting, true);
/* 2 minute timeout */
result = isc__nm_socket_connectiontimeout(sock->fd, 120 * 1000);
RUNTIME_CHECK(result == ISC_R_SUCCESS);
r = uv_tcp_init(&worker->loop, &sock->uv_handle.tcp);
RUNTIME_CHECK(r == 0);
uv_handle_set_data(&sock->uv_handle.handle, sock);
@ -118,6 +122,11 @@ tlsdns_connect_direct(isc_nmsocket_t *sock, isc__nm_uvreq_t *req) {
RUNTIME_CHECK(r == 0);
uv_handle_set_data((uv_handle_t *)&sock->timer, sock);
if (isc__nm_closing(sock)) {
result = ISC_R_CANCELED;
goto error;
}
r = uv_tcp_open(&sock->uv_handle.tcp, sock->fd);
if (r != 0) {
isc__nm_closesocket(sock->fd);
@ -156,7 +165,7 @@ tlsdns_connect_direct(isc_nmsocket_t *sock, isc__nm_uvreq_t *req) {
done:
result = isc__nm_uverr2result(r);
error:
LOCK(&sock->lock);
sock->result = result;
SIGNAL(&sock->cond);
@ -209,9 +218,6 @@ tlsdns_connect_cb(uv_connect_t *uvreq, int status) {
REQUIRE(VALID_NMSOCK(sock));
REQUIRE(sock->tid == isc_nm_tid());
isc__nmsocket_timer_stop(sock);
uv_handle_set_data((uv_handle_t *)&sock->timer, sock);
req = uv_handle_get_data((uv_handle_t *)uvreq);
REQUIRE(VALID_UVREQ(req));
@ -224,8 +230,8 @@ tlsdns_connect_cb(uv_connect_t *uvreq, int status) {
*/
isc__nm_uvreq_put(&req, sock);
return;
} else if (!isc__nmsocket_active(sock)) {
/* Socket was closed midflight by isc__nm_tcpdns_shutdown() */
} else if (isc__nmsocket_closing(sock)) {
/* Socket was closed midflight by isc__nm_tlsdns_shutdown() */
result = ISC_R_CANCELED;
goto error;
} else if (status == UV_ETIMEDOUT) {
@ -279,19 +285,20 @@ tlsdns_connect_cb(uv_connect_t *uvreq, int status) {
result = isc_sockaddr_fromsockaddr(&sock->peer, (struct sockaddr *)&ss);
RUNTIME_CHECK(result == ISC_R_SUCCESS);
/* Setting pending req */
sock->tls.pending_req = req;
isc__nm_process_sock_buffer(sock);
result = tls_cycle(sock);
if (result != ISC_R_SUCCESS) {
sock->tls.pending_req = NULL;
goto error;
}
return;
error:
sock->tls.pending_req = NULL;
isc__nm_failed_connect_cb(sock, req, result);
}
@ -616,7 +623,7 @@ tlsdns_connection_cb(uv_stream_t *server, int status) {
REQUIRE(VALID_NMSOCK(ssock));
REQUIRE(ssock->tid == isc_nm_tid());
if (isc__nm_inactive(ssock)) {
if (isc__nmsocket_closing(ssock)) {
result = ISC_R_CANCELED;
goto done;
}
@ -854,7 +861,7 @@ isc__nm_async_tlsdnsread(isc__networker_t *worker, isc__netievent_t *ev0) {
REQUIRE(VALID_NMSOCK(sock));
REQUIRE(sock->tid == isc_nm_tid());
if (isc__nm_inactive(sock)) {
if (isc__nmsocket_closing(sock)) {
sock->reading = true;
isc__nm_failed_read_cb(sock, ISC_R_CANCELED);
return;
@ -884,7 +891,7 @@ isc__nm_tlsdns_processbuffer(isc_nmsocket_t *sock) {
REQUIRE(VALID_NMSOCK(sock));
REQUIRE(sock->tid == isc_nm_tid());
if (isc__nm_inactive(sock)) {
if (isc__nmsocket_closing(sock)) {
return (ISC_R_CANCELED);
}
@ -978,16 +985,19 @@ tls_cycle_input(isc_nmsocket_t *sock) {
sock->buf + sock->buf_len,
sock->buf_size - sock->buf_len, &len);
if (rv != 1) {
/* Process what's in the buffer so far
/*
* Process what's in the buffer so far
*/
isc__nm_process_sock_buffer(sock);
/* FIXME: Should we call
* failed_read_cb()? */
/*
* FIXME: Should we call
* isc__nm_failed_read_cb()?
*/
break;
}
REQUIRE((size_t)pending == len);
INSIST((size_t)pending == len);
sock->buf_len += len;
@ -1052,6 +1062,9 @@ tls_cycle_input(isc_nmsocket_t *sock) {
isc__nm_uvreq_t *req = sock->tls.pending_req;
sock->tls.pending_req = NULL;
isc__nmsocket_timer_stop(sock);
uv_handle_set_data((uv_handle_t *)&sock->timer, sock);
atomic_store(&sock->connecting, false);
isc__nm_connectcb(sock, req, ISC_R_SUCCESS, true);
@ -1067,7 +1080,13 @@ tls_error(isc_nmsocket_t *sock, isc_result_t result) {
switch (sock->tls.state) {
case TLS_STATE_HANDSHAKE:
case TLS_STATE_IO:
isc__nm_tlsdns_failed_read_cb(sock, result);
if (atomic_load(&sock->connecting)) {
isc__nm_uvreq_t *req = sock->tls.pending_req;
sock->tls.pending_req = NULL;
isc__nm_failed_connect_cb(sock, req, result);
} else {
isc__nm_tlsdns_failed_read_cb(sock, result);
}
break;
case TLS_STATE_ERROR:
return;
@ -1078,7 +1097,7 @@ tls_error(isc_nmsocket_t *sock, isc_result_t result) {
sock->tls.state = TLS_STATE_ERROR;
sock->tls.pending_error = result;
/* tlsdns_close_direct(sock); */
isc__nmsocket_shutdown(sock);
}
static void
@ -1205,6 +1224,10 @@ static isc_result_t
tls_cycle(isc_nmsocket_t *sock) {
isc_result_t result;
if (isc__nmsocket_closing(sock)) {
return (ISC_R_CANCELED);
}
result = tls_pop_error(sock);
if (result != ISC_R_SUCCESS) {
goto done;
@ -1235,7 +1258,7 @@ async_tlsdns_cycle(isc_nmsocket_t *sock) {
REQUIRE(VALID_NMSOCK(sock));
/* Socket was closed midflight by isc__nm_tlsdns_shutdown() */
if (!isc__nmsocket_active(sock)) {
if (isc__nmsocket_closing(sock)) {
return;
}
@ -1278,7 +1301,7 @@ isc__nm_tlsdns_read_cb(uv_stream_t *stream, ssize_t nread,
REQUIRE(sock->reading);
REQUIRE(buf != NULL);
if (isc__nm_inactive(sock)) {
if (isc__nmsocket_closing(sock)) {
isc__nm_failed_read_cb(sock, ISC_R_CANCELED);
goto free;
}
@ -1375,7 +1398,7 @@ accept_connection(isc_nmsocket_t *ssock, isc_quota_t *quota) {
REQUIRE(VALID_NMSOCK(ssock));
REQUIRE(ssock->tid == isc_nm_tid());
if (isc__nm_inactive(ssock)) {
if (isc__nmsocket_closing(ssock)) {
if (quota != NULL) {
isc_quota_detach(&quota);
}
@ -1608,7 +1631,7 @@ tlsdns_send_direct(isc_nmsocket_t *sock, isc__nm_uvreq_t *req) {
return (result);
}
if (isc__nm_inactive(sock)) {
if (isc__nmsocket_closing(sock)) {
return (ISC_R_CANCELED);
}
@ -1813,6 +1836,8 @@ tlsdns_close_direct(isc_nmsocket_t *sock) {
isc__nmsocket_timer_stop(sock);
isc__nm_stop_reading(sock);
uv_handle_set_data((uv_handle_t *)&sock->timer, sock);
uv_close((uv_handle_t *)&sock->timer, timer_close_cb);
}
@ -1883,27 +1908,37 @@ isc__nm_tlsdns_shutdown(isc_nmsocket_t *sock) {
return;
}
if (sock->tls.tls) {
/* Shutdown any active TLS connections */
(void)SSL_shutdown(sock->tls.tls);
}
if (sock->accepting) {
return;
}
if (sock->tls.pending_req != NULL) {
isc__nm_uvreq_t *req = sock->tls.pending_req;
sock->tls.pending_req = NULL;
isc__nm_failed_connect_cb(sock, req, ISC_R_CANCELED);
return;
}
/* TLS handshake hasn't been completed yet */
if (atomic_load(&sock->connecting)) {
/*
* TCP connection has been established, now waiting on
* TLS handshake to complete
*/
if (sock->tls.pending_req != NULL) {
isc__nm_uvreq_t *req = sock->tls.pending_req;
sock->tls.pending_req = NULL;
isc__nm_failed_connect_cb(sock, req, ISC_R_CANCELED);
return;
}
/* The TCP connection hasn't been established yet */
isc_nmsocket_t *tsock = NULL;
isc__nmsocket_attach(sock, &tsock);
uv_close(&sock->uv_handle.handle, tlsdns_close_connect_cb);
return;
}
if (sock->statichandle) {
if (sock->statichandle != NULL) {
isc__nm_failed_read_cb(sock, ISC_R_CANCELED);
return;
}

8
lib/isc/netmgr/udp.c
View File

@ -523,7 +523,7 @@ isc__nm_async_udpsend(isc__networker_t *worker, isc__netievent_t *ev0) {
REQUIRE(sock->tid == isc_nm_tid());
UNUSED(worker);
if (isc__nm_inactive(sock)) {
if (isc__nmsocket_closing(sock)) {
isc__nm_failed_send_cb(sock, uvreq, ISC_R_CANCELED);
return;
}
@ -567,7 +567,7 @@ udp_send_direct(isc_nmsocket_t *sock, isc__nm_uvreq_t *req,
REQUIRE(sock->tid == isc_nm_tid());
REQUIRE(sock->type == isc_nm_udpsocket);
if (isc__nm_inactive(sock)) {
if (isc__nmsocket_closing(sock)) {
return (ISC_R_CANCELED);
}
@ -868,7 +868,7 @@ isc__nm_async_udpread(isc__networker_t *worker, isc__netievent_t *ev0) {
REQUIRE(VALID_NMSOCK(sock));
REQUIRE(sock->tid == isc_nm_tid());
if (isc__nm_inactive(sock)) {
if (isc__nmsocket_closing(sock)) {
sock->reading = true;
isc__nm_failed_read_cb(sock, ISC_R_CANCELED);
return;
@ -1084,7 +1084,7 @@ isc__nm_udp_shutdown(isc_nmsocket_t *sock) {
* sock->statichandle would be NULL, in that case, nobody is
* interested in the callback.
*/
if (sock->statichandle) {
if (sock->statichandle != NULL) {
isc__nm_failed_read_cb(sock, ISC_R_CANCELED);
return;
}