upstream/bind9
1
0
Fork 0
mirror of https://github.com/isc-projects/bind9.git synced 2026-06-11 15:39:59 -04:00
Code Issues Projects Releases Packages Wiki Activity Actions

Merge branch '2227-tcp-connection-closed-second-fix' into 'main'

Browse source 
Fix improper closed connection handling in tcpdns.

Closes #2227

See merge request isc-projects/bind9!4341
This commit is contained in:
Ondřej Surý 2020-11-02 22:52:01 +00:00
commit 3ec9b5f1d9
4 changed files with 94 additions and 51 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

4
CHANGES
View file

@ -1,3 +1,5 @@
5526. [bug] Fix a race/NULL dereference in TCPDNS read. [GL #2227]
5525. [placeholder]
5524. [func] Added functionality to the network manager to
@ -9,7 +11,7 @@
not found. Subsequent lookups would succeed.
[GL #2236]
5522. [bug] Fix a race/NULL dereference in TCPDNS. [GL #2227]
5522. [bug] Fix a race/NULL dereference in TCPDNS send. [GL #2227]
5521. [func] All use of libltdl was dropped. libuv's shared library
handling interface is now used instead. [GL !4278]

2
lib/isc/netmgr/netmgr-int.h
View file

@ -871,7 +871,7 @@ isc__nm_async_tcpdnsstop(isc__networker_t *worker, isc__netievent_t *ev0);
void
isc__nm_async_tcpdnsread(isc__networker_t *worker, isc__netievent_t *ev0);
isc_result_t
void
isc__nm_tcpdns_read(isc_nmhandle_t *handle, isc_nm_recv_cb_t cb, void *cbarg);
void

3
lib/isc/netmgr/tcp.c
View file

@ -1391,11 +1391,10 @@ isc__nm_async_tcpclose(isc__networker_t *worker, isc__netievent_t *ev0) {
isc_nmsocket_t *sock = ievent->sock;
REQUIRE(VALID_NMSOCK(sock));
REQUIRE(sock->tid == isc_nm_tid());
UNUSED(worker);
REQUIRE(sock->tid == isc_nm_tid());
tcp_close_direct(sock);
}

136
lib/isc/netmgr/tcpdns.c
View file

@ -152,6 +152,7 @@ dnslisten_acceptcb(isc_nmhandle_t *handle, isc_result_t result, void *cbarg) {
dnssock->timer_initialized = true;
uv_timer_start(&dnssock->timer, dnstcp_readtimeout,
dnssock->read_timeout, 0);
dnssock->timer_running = true;
/*
* Add a reference to handle to keep it from being freed by
@ -260,20 +261,38 @@ dnslisten_readcb(isc_nmhandle_t *handle, isc_result_t eresult,
REQUIRE(dnssock->tid == isc_nm_tid());
REQUIRE(VALID_NMHANDLE(handle));
if (!isc__nmsocket_active(dnssock) || atomic_load(&dnssock->closing) ||
dnssock->outerhandle == NULL ||
(dnssock->listener != NULL &&
!isc__nmsocket_active(dnssock->listener)) ||
atomic_load(&dnssock->mgr->closing))
{
if (eresult == ISC_R_SUCCESS) {
eresult = ISC_R_CANCELED;
}
}
if (region == NULL || eresult != ISC_R_SUCCESS) {
isc_nm_recv_cb_t cb = dnssock->recv_cb;
void *cbarg = dnssock->recv_cbarg;
/* Connection closed */
atomic_store(&dnssock->result, eresult);
if (atomic_load(&dnssock->client) && dnssock->recv_cb != NULL) {
dnssock->recv_cb(dnssock->statichandle, eresult, NULL,
dnssock->recv_cbarg);
isc__nmsocket_clearcb(dnssock);
if (atomic_load(&dnssock->client) && cb != NULL) {
cb(dnssock->statichandle, eresult, NULL, cbarg);
}
if (dnssock->self != NULL) {
isc__nmsocket_detach(&dnssock->self);
}
isc__nmsocket_clearcb(dnssock);
if (dnssock->outerhandle != NULL) {
isc__nmsocket_clearcb(dnssock->outerhandle->sock);
isc_nmhandle_detach(&dnssock->outerhandle);
}
if (dnssock->listener != NULL) {
isc__nmsocket_detach(&dnssock->listener);
}
/*
* Server connections will hold two handle references when
@ -482,6 +501,7 @@ resume_processing(void *arg) {
if (sock->timer_initialized) {
uv_timer_start(&sock->timer, dnstcp_readtimeout,
sock->read_timeout, 0);
sock->timer_running = true;
}
}
@ -546,30 +566,46 @@ tcpdnssend_cb(isc_nmhandle_t *handle, isc_result_t result, void *cbarg) {
isc_nmhandle_detach(&handle);
}
/*
* The socket is closing, outerhandle has been detached, listener is
* inactive, or the netmgr is closing: any operation on it should abort
* with ISC_R_CANCELED.
*/
static bool
inactive(isc_nmsocket_t *sock) {
return (!isc__nmsocket_active(sock) || atomic_load(&sock->closing) ||
sock->outerhandle == NULL ||
(sock->listener != NULL &&
!isc__nmsocket_active(sock->listener)) ||
atomic_load(&sock->mgr->closing));
}
void
isc__nm_async_tcpdnssend(isc__networker_t *worker, isc__netievent_t *ev0) {
isc__netievent_tcpdnssend_t *ievent =
(isc__netievent_tcpdnssend_t *)ev0;
isc__nm_uvreq_t *req = ievent->req;
isc_nmsocket_t *sock = ievent->sock;
isc_nmhandle_t *sendhandle = NULL;
isc_region_t r;
REQUIRE(VALID_NMSOCK(sock));
REQUIRE(VALID_UVREQ(req));
REQUIRE(worker->id == sock->tid);
REQUIRE(sock->tid == isc_nm_tid());
REQUIRE(sock->type == isc_nm_tcpdnssocket);
if (isc__nmsocket_active(sock) && sock->outerhandle != NULL) {
isc_nmhandle_t *sendhandle = NULL;
isc_region_t r;
r.base = (unsigned char *)req->uvbuf.base;
r.length = req->uvbuf.len;
isc_nmhandle_attach(sock->outerhandle, &sendhandle);
isc_nm_send(sendhandle, &r, tcpdnssend_cb, req);
} else {
if (inactive(sock)) {
req->cb.send(req->handle, ISC_R_CANCELED, req->cbarg);
isc_mem_put(req->sock->mgr->mctx, req->uvbuf.base,
req->uvbuf.len);
isc_mem_put(sock->mgr->mctx, req->uvbuf.base, req->uvbuf.len);
isc__nm_uvreq_put(&req, req->handle->sock);
return;
}
r.base = (unsigned char *)req->uvbuf.base;
r.length = req->uvbuf.len;
isc_nmhandle_attach(sock->outerhandle, &sendhandle);
isc_nm_send(sendhandle, &r, tcpdnssend_cb, req);
}
/*
@ -587,7 +623,7 @@ isc__nm_tcpdns_send(isc_nmhandle_t *handle, isc_region_t *region,
REQUIRE(VALID_NMSOCK(sock));
REQUIRE(sock->type == isc_nm_tcpdnssocket);
if (!isc__nmsocket_active(sock)) {
if (inactive(sock)) {
cb(handle, ISC_R_CANCELED, cbarg);
return;
}
@ -602,38 +638,25 @@ isc__nm_tcpdns_send(isc_nmhandle_t *handle, isc_region_t *region,
*(uint16_t *)uvreq->uvbuf.base = htons(region->length);
memmove(uvreq->uvbuf.base + 2, region->base, region->length);
if (sock->tid == isc_nm_tid()) {
isc_nmhandle_t *sendhandle = NULL;
isc_region_t r;
isc__netievent_tcpdnssend_t *ievent = NULL;
r.base = (unsigned char *)uvreq->uvbuf.base;
r.length = uvreq->uvbuf.len;
if (sock->outerhandle != NULL) {
isc_nmhandle_attach(sock->outerhandle, &sendhandle);
isc_nm_send(sock->outerhandle, &r, tcpdnssend_cb,
uvreq);
} else {
cb(handle, ISC_R_CANCELED, cbarg);
isc_mem_put(sock->mgr->mctx, uvreq->uvbuf.base,
uvreq->uvbuf.len);
isc__nm_uvreq_put(&uvreq, sock);
}
} else {
isc__netievent_tcpdnssend_t *ievent = NULL;
ievent = isc__nm_get_ievent(sock->mgr, netievent_tcpdnssend);
ievent->req = uvreq;
ievent->sock = sock;
ievent = isc__nm_get_ievent(sock->mgr, netievent_tcpdnssend);
ievent->req = uvreq;
ievent->sock = sock;
isc__nm_enqueue_ievent(&sock->mgr->workers[sock->tid],
(isc__netievent_t *)ievent);
}
isc__nm_enqueue_ievent(&sock->mgr->workers[sock->tid],
(isc__netievent_t *)ievent);
}
static void
tcpdns_close_direct(isc_nmsocket_t *sock) {
REQUIRE(sock->tid == isc_nm_tid());
if (sock->timer_running) {
uv_timer_stop(&sock->timer);
sock->timer_running = false;
}
/* We don't need atomics here, it's all in single network thread */
if (sock->self != NULL) {
isc__nmsocket_detach(&sock->self);
@ -668,6 +691,11 @@ isc__nm_tcpdns_close(isc_nmsocket_t *sock) {
REQUIRE(VALID_NMSOCK(sock));
REQUIRE(sock->type == isc_nm_tcpdnssocket);
if (!atomic_compare_exchange_strong(&sock->closing, &(bool){ false },
true)) {
return;
}
if (sock->tid == isc_nm_tid()) {
tcpdns_close_direct(sock);
} else {
@ -684,8 +712,12 @@ void
isc__nm_async_tcpdnsclose(isc__networker_t *worker, isc__netievent_t *ev0) {
isc__netievent_tcpdnsclose_t *ievent =
(isc__netievent_tcpdnsclose_t *)ev0;
isc_nmsocket_t *sock = ievent->sock;
REQUIRE(worker->id == ievent->sock->tid);
REQUIRE(VALID_NMSOCK(sock));
REQUIRE(sock->tid == isc_nm_tid());
UNUSED(worker);
tcpdns_close_direct(ievent->sock);
}
@ -734,6 +766,7 @@ tcpdnsconnect_cb(isc_nmhandle_t *handle, isc_result_t result, void *arg) {
dnssock->timer_initialized = true;
uv_timer_start(&dnssock->timer, dnstcp_readtimeout,
dnssock->read_timeout, 0);
dnssock->timer_running = true;
/*
* The connection is now established; we start reading immediately,
@ -759,7 +792,7 @@ isc_nm_tcpdnsconnect(isc_nm_t *mgr, isc_nmiface_t *local, isc_nmiface_t *peer,
timeout, 0));
}
isc_result_t
void
isc__nm_tcpdns_read(isc_nmhandle_t *handle, isc_nm_recv_cb_t cb, void *cbarg) {
isc_nmsocket_t *sock = handle->sock;
isc__netievent_tcpdnsread_t *ievent = NULL;
@ -770,6 +803,11 @@ isc__nm_tcpdns_read(isc_nmhandle_t *handle, isc_nm_recv_cb_t cb, void *cbarg) {
REQUIRE(sock->tid == isc_nm_tid());
REQUIRE(atomic_load(&sock->client));
if (inactive(sock)) {
cb(handle, ISC_R_NOTCONNECTED, NULL, cbarg);
return;
}
/*
* This MUST be done asynchronously, no matter which thread we're
* in. The callback function for isc_nm_read() often calls
@ -789,7 +827,6 @@ isc__nm_tcpdns_read(isc_nmhandle_t *handle, isc_nm_recv_cb_t cb, void *cbarg) {
isc_nmhandle_attach(handle, &eventhandle);
isc__nm_enqueue_ievent(&sock->mgr->workers[sock->tid],
(isc__netievent_t *)ievent);
return (ISC_R_SUCCESS);
}
void
@ -806,11 +843,15 @@ isc__nm_async_tcpdnsread(isc__networker_t *worker, isc__netievent_t *ev0) {
handle = sock->statichandle;
if (sock->type != isc_nm_tcpdnssocket || sock->outerhandle == NULL) {
if (sock->recv_cb != NULL) {
sock->recv_cb(handle, ISC_R_NOTCONNECTED, NULL,
sock->recv_cbarg);
if (inactive(sock)) {
isc_nm_recv_cb_t cb = sock->recv_cb;
void *cbarg = sock->recv_cbarg;
isc__nmsocket_clearcb(sock);
if (cb != NULL) {
cb(handle, ISC_R_NOTCONNECTED, NULL, cbarg);
}
isc_nmhandle_detach(&handle);
return;
}
@ -831,6 +872,7 @@ isc__nm_async_tcpdnsread(isc__networker_t *worker, isc__netievent_t *ev0) {
if (sock->timer_initialized) {
uv_timer_start(&sock->timer, dnstcp_readtimeout,
sock->read_timeout, 0);
sock->timer_running = true;
}
isc_nm_resumeread(sock->outerhandle);
} else {