From: Witold Kręcicki <wpk@isc.org>
Date: Mon, 29 Jun 2020 06:43:54 +0000 (+0200)
Subject: Fix possible race in isc__nm_tcpconnect.
X-Git-Tag: v9.17.3~20^2~1
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=896db0f41988f66bd06d6fa840b18be66a22200a;p=thirdparty%2Fbind9.git

Fix possible race in isc__nm_tcpconnect.

There's a possibility of race in isc__nm_tcpconnect if the asynchronous
connect operation finishes with all the callbacks before we exit the
isc__nm_tcpconnect itself we might access an already freed memory.
Fix it by creating an additional reference to the socket freed at the
end of isc__nm_tcpconnect.
---

diff --git a/lib/isc/netmgr/tcp.c b/lib/isc/netmgr/tcp.c
index 5710e38baeb..430616355b8 100644
--- a/lib/isc/netmgr/tcp.c
+++ b/lib/isc/netmgr/tcp.c
@@ -195,9 +195,10 @@ tcp_connect_cb(uv_connect_t *uvreq, int status) {
 isc_result_t
 isc_nm_tcpconnect(isc_nm_t *mgr, isc_nmiface_t *local, isc_nmiface_t *peer,
 		  isc_nm_cb_t cb, void *cbarg, size_t extrahandlesize) {
-	isc_nmsocket_t *nsock = NULL;
+	isc_nmsocket_t *nsock = NULL, *tmp = NULL;
 	isc__netievent_tcpconnect_t *ievent = NULL;
 	isc__nm_uvreq_t *req = NULL;
+	isc_result_t result = ISC_R_SUCCESS;
 
 	REQUIRE(VALID_NM(mgr));
 
@@ -215,6 +216,12 @@ isc_nm_tcpconnect(isc_nm_t *mgr, isc_nmiface_t *local, isc_nmiface_t *peer,
 	ievent->sock = nsock;
 	ievent->req = req;
 
+	/*
+	 * Async callbacks can dereference the socket in the meantime,
+	 * we need to hold an additional reference to it.
+	 */
+	isc__nmsocket_attach(nsock, &tmp);
+
 	if (isc__nm_in_netthread()) {
 		nsock->tid = isc_nm_tid();
 		isc__nm_async_tcpconnect(&mgr->workers[nsock->tid],
@@ -234,12 +241,13 @@ isc_nm_tcpconnect(isc_nm_t *mgr, isc_nmiface_t *local, isc_nmiface_t *peer,
 	}
 
 	if (nsock->result != ISC_R_SUCCESS) {
-		isc_result_t result = nsock->result;
+		result = nsock->result;
 		isc__nmsocket_detach(&nsock);
-		return (result);
 	}
 
-	return (ISC_R_SUCCESS);
+	isc__nmsocket_detach(&tmp);
+
+	return (result);
 }
 
 isc_result_t