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

Fix a race in TCP accepting.

Browse Source 
There's a possibility of a race in TCP accepting code:
T1 accepts a connection C1
T2 accepts a connection C2
T1 tries to accept a connection C3, but we hit a quota,
   isc_quota_cb_init() sets quota_accept_cb for the socket,
   we return from accept_connection
T2 drops C2, but we race in quota_release with accepting C3 so
   we don't see quota->waiting is > 0, we don't launch the callback
T1 accepts a connection C4, we are able to get the quota we clear
   the quota_accept_cb from sock->quotacb
T1 drops C1, tries to call the callback which is zeroed, sigsegv.
This commit is contained in:
Witold Kręcicki 2020-06-10 16:19:16 +02:00 committed by Evan Hunt
parent c449cab63a
commit 85d8e4bf76
2 changed files with 9 additions and 5 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
CHANGES
View File

@ -1,3 +1,5 @@
5438. [bug] Fix a race in TCP accepting code. [GL #1930]
5437. [bug] Fix a data race in resolver log_formerr. [GL #1808]
5436. [placeholder]

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

@ -73,6 +73,9 @@ tcp_listenclose_cb(uv_handle_t *handle);
static isc_result_t
accept_connection(isc_nmsocket_t *ssock, isc_quota_t *quota);
static void
quota_accept_cb(isc_quota_t *quota, void *sock0);
static int
tcp_connect_direct(isc_nmsocket_t *sock, isc__nm_uvreq_t *req) {
isc__networker_t *worker = NULL;
@ -179,6 +182,7 @@ isc_nm_listentcp(isc_nm_t *mgr, isc_nmiface_t *iface, isc_nm_cb_t cb,
*/
nsock->pquota = quota;
}
isc_quota_cb_init(&nsock->quotacb, quota_accept_cb, nsock);
ievent = isc__nm_get_ievent(mgr, netievent_tcplisten);
ievent->sock = nsock;
@ -733,12 +737,11 @@ accept_connection(isc_nmsocket_t *ssock, isc_quota_t *quota) {
* We need to attach to ssock, because it might be queued
* waiting for a TCP quota slot. If so, then we'll detach it
* later when the connection is accepted. (XXX: This may be
* suboptimal, it might be better to attach unless
* we need to.)
* suboptimal, it might be better not to attach unless
* we need to - but we risk a race then.)
*/
isc_nmsocket_t *tsock = NULL;
isc_nmsocket_attach(ssock, &tsock);
isc_quota_cb_init(&ssock->quotacb, quota_accept_cb, tsock);
result = isc_quota_attach_cb(ssock->pquota, &quota,
&ssock->quotacb);
if (result == ISC_R_QUOTA) {
@ -749,9 +752,8 @@ accept_connection(isc_nmsocket_t *ssock, isc_quota_t *quota) {
/*
* We're under quota, so there's no need to wait;
* clear the quota callback and and detach the socket.
* Detach the socket.
*/
isc_quota_cb_init(&ssock->quotacb, NULL, NULL);
isc_nmsocket_detach(&tsock);
}