From: Ondřej Surý <ondrej@isc.org>
Date: Thu, 3 Dec 2020 16:58:10 +0000 (+0100)
Subject: Use sock->nchildren instead of mgr->nworkers when initializing NM
X-Git-Tag: v9.16.11~17^2~3
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=bb9b55dfba25d6b36e683da9602d0522b6cc4700;p=thirdparty%2Fbind9.git

Use sock->nchildren instead of mgr->nworkers when initializing NM

On Windows, we were limiting the number of listening children to just 1,
but we were then iterating on mgr->nworkers.  That lead to scheduling
more async_*listen() than actually allocated and out-of-bound read-write
operation on the heap.

(cherry picked from commit 87c5867202935c59dfe66321238275ba4a953b53)
---

diff --git a/lib/isc/netmgr/netmgr-int.h b/lib/isc/netmgr/netmgr-int.h
index 17e3e362766..1997ec1cc3e 100644
--- a/lib/isc/netmgr/netmgr-int.h
+++ b/lib/isc/netmgr/netmgr-int.h
@@ -724,7 +724,7 @@ struct isc_nmsocket {
 
 	/*% Child sockets for multi-socket setups */
 	isc_nmsocket_t *children;
-	int nchildren;
+	uint_fast32_t nchildren;
 	isc_nmiface_t *iface;
 	isc_nmhandle_t *statichandle;
 	isc_nmhandle_t *outerhandle;
diff --git a/lib/isc/netmgr/netmgr.c b/lib/isc/netmgr/netmgr.c
index 03928224f8f..feb21c24da0 100644
--- a/lib/isc/netmgr/netmgr.c
+++ b/lib/isc/netmgr/netmgr.c
@@ -913,7 +913,7 @@ nmsocket_cleanup(isc_nmsocket_t *sock, bool dofree FLARG) {
 		 * We shouldn't be here unless there are no active handles,
 		 * so we can clean up and free the children.
 		 */
-		for (int i = 0; i < sock->nchildren; i++) {
+		for (size_t i = 0; i < sock->nchildren; i++) {
 			if (!atomic_load(&sock->children[i].destroying)) {
 				nmsocket_cleanup(&sock->children[i],
 						 false FLARG_PASS);
@@ -1023,7 +1023,7 @@ nmsocket_maybe_destroy(isc_nmsocket_t *sock FLARG) {
 
 	active_handles = atomic_load(&sock->ah);
 	if (sock->children != NULL) {
-		for (int i = 0; i < sock->nchildren; i++) {
+		for (size_t i = 0; i < sock->nchildren; i++) {
 			LOCK(&sock->children[i].lock);
 			active_handles += atomic_load(&sock->children[i].ah);
 			UNLOCK(&sock->children[i].lock);
@@ -1065,7 +1065,7 @@ isc___nmsocket_prep_destroy(isc_nmsocket_t *sock FLARG) {
 	 * so they can be cleaned up too.
 	 */
 	if (sock->children != NULL) {
-		for (int i = 0; i < sock->nchildren; i++) {
+		for (size_t i = 0; i < sock->nchildren; i++) {
 			atomic_store(&sock->children[i].active, false);
 		}
 	}
diff --git a/lib/isc/netmgr/tcp.c b/lib/isc/netmgr/tcp.c
index 41afed53eb5..db6ef3ad8df 100644
--- a/lib/isc/netmgr/tcp.c
+++ b/lib/isc/netmgr/tcp.c
@@ -424,14 +424,14 @@ isc_nm_listentcp(isc_nm_t *mgr, isc_nmiface_t *iface,
 	memset(sock->children, 0, children_size);
 
 	sock->result = ISC_R_DEFAULT;
-	sock->tid = isc_random_uniform(mgr->nworkers);
+	sock->tid = isc_random_uniform(sock->nchildren);
 	sock->fd = -1;
 
 #if !HAVE_SO_REUSEPORT_LB && !defined(WIN32)
 	fd = isc__nm_tcp_lb_socket(sa_family);
 #endif
 
-	for (size_t i = 0; i < mgr->nworkers; i++) {
+	for (size_t i = 0; i < sock->nchildren; i++) {
 		isc__netievent_tcplisten_t *ievent = NULL;
 		isc_nmsocket_t *csock = &sock->children[i];
 
@@ -466,7 +466,7 @@ isc_nm_listentcp(isc_nm_t *mgr, isc_nmiface_t *iface,
 #endif
 
 	LOCK(&sock->lock);
-	while (sock->rchildren != mgr->nworkers) {
+	while (sock->rchildren != sock->nchildren) {
 		WAIT(&sock->cond, &sock->lock);
 	}
 	result = sock->result;
@@ -476,7 +476,7 @@ isc_nm_listentcp(isc_nm_t *mgr, isc_nmiface_t *iface,
 	INSIST(result != ISC_R_DEFAULT);
 
 	if (result == ISC_R_SUCCESS) {
-		REQUIRE(sock->rchildren == mgr->nworkers);
+		REQUIRE(sock->rchildren == sock->nchildren);
 		*sockp = sock;
 	} else {
 		atomic_store(&sock->active, false);
@@ -1323,7 +1323,7 @@ stop_tcp_parent(isc_nmsocket_t *sock) {
 	REQUIRE(VALID_NMSOCK(sock));
 	REQUIRE(sock->type == isc_nm_tcplistener);
 
-	for (int i = 0; i < sock->nchildren; i++) {
+	for (size_t i = 0; i < sock->nchildren; i++) {
 		isc__netievent_tcpstop_t *ievent = NULL;
 		isc_nmsocket_t *csock = &sock->children[i];
 		REQUIRE(VALID_NMSOCK(csock));
diff --git a/lib/isc/netmgr/tcpdns.c b/lib/isc/netmgr/tcpdns.c
index edcbff6539c..f52cbca3e9e 100644
--- a/lib/isc/netmgr/tcpdns.c
+++ b/lib/isc/netmgr/tcpdns.c
@@ -468,14 +468,14 @@ isc_nm_listentcpdns(isc_nm_t *mgr, isc_nmiface_t *iface,
 	memset(sock->children, 0, children_size);
 
 	sock->result = ISC_R_DEFAULT;
-	sock->tid = isc_random_uniform(mgr->nworkers);
+	sock->tid = isc_random_uniform(sock->nchildren);
 	sock->fd = -1;
 
 #if !HAVE_SO_REUSEPORT_LB && !defined(WIN32)
 	fd = isc__nm_tcpdns_lb_socket(sa_family);
 #endif
 
-	for (size_t i = 0; i < mgr->nworkers; i++) {
+	for (size_t i = 0; i < sock->nchildren; i++) {
 		isc__netievent_tcpdnslisten_t *ievent = NULL;
 		isc_nmsocket_t *csock = &sock->children[i];
 
@@ -512,7 +512,7 @@ isc_nm_listentcpdns(isc_nm_t *mgr, isc_nmiface_t *iface,
 #endif
 
 	LOCK(&sock->lock);
-	while (sock->rchildren != mgr->nworkers) {
+	while (sock->rchildren != sock->nchildren) {
 		WAIT(&sock->cond, &sock->lock);
 	}
 	result = sock->result;
@@ -522,7 +522,7 @@ isc_nm_listentcpdns(isc_nm_t *mgr, isc_nmiface_t *iface,
 	INSIST(result != ISC_R_DEFAULT);
 
 	if (result == ISC_R_SUCCESS) {
-		REQUIRE(sock->rchildren == mgr->nworkers);
+		REQUIRE(sock->rchildren == sock->nchildren);
 		*sockp = sock;
 	} else {
 		atomic_store(&sock->active, false);
@@ -1420,7 +1420,7 @@ stop_tcpdns_parent(isc_nmsocket_t *sock) {
 	REQUIRE(VALID_NMSOCK(sock));
 	REQUIRE(sock->type == isc_nm_tcpdnslistener);
 
-	for (int i = 0; i < sock->nchildren; i++) {
+	for (size_t i = 0; i < sock->nchildren; i++) {
 		isc__netievent_tcpdnsstop_t *ievent = NULL;
 		isc_nmsocket_t *csock = &sock->children[i];
 		REQUIRE(VALID_NMSOCK(csock));
diff --git a/lib/isc/netmgr/udp.c b/lib/isc/netmgr/udp.c
index 85469a4a1cb..f69f6feb197 100644
--- a/lib/isc/netmgr/udp.c
+++ b/lib/isc/netmgr/udp.c
@@ -132,14 +132,14 @@ isc_nm_listenudp(isc_nm_t *mgr, isc_nmiface_t *iface, isc_nm_recv_cb_t cb,
 	sock->recv_cbarg = cbarg;
 	sock->extrahandlesize = extrahandlesize;
 	sock->result = ISC_R_DEFAULT;
-	sock->tid = isc_random_uniform(mgr->nworkers);
+	sock->tid = isc_random_uniform(sock->nchildren);
 	sock->fd = -1;
 
 #if !HAVE_SO_REUSEPORT_LB && !defined(WIN32)
 	fd = isc__nm_udp_lb_socket(sa_family);
 #endif
 
-	for (size_t i = 0; i < mgr->nworkers; i++) {
+	for (size_t i = 0; i < sock->nchildren; i++) {
 		isc__netievent_udplisten_t *ievent = NULL;
 		isc_nmsocket_t *csock = &sock->children[i];
 
@@ -169,7 +169,7 @@ isc_nm_listenudp(isc_nm_t *mgr, isc_nmiface_t *iface, isc_nm_recv_cb_t cb,
 #endif
 
 	LOCK(&sock->lock);
-	while (sock->rchildren != mgr->nworkers) {
+	while (sock->rchildren != sock->nchildren) {
 		WAIT(&sock->cond, &sock->lock);
 	}
 	result = sock->result;
@@ -179,7 +179,7 @@ isc_nm_listenudp(isc_nm_t *mgr, isc_nmiface_t *iface, isc_nm_recv_cb_t cb,
 	INSIST(result != ISC_R_DEFAULT);
 
 	if (result == ISC_R_SUCCESS) {
-		REQUIRE(sock->rchildren == mgr->nworkers);
+		REQUIRE(sock->rchildren == sock->nchildren);
 		*sockp = sock;
 	} else {
 		atomic_store(&sock->active, false);
@@ -1119,7 +1119,7 @@ stop_udp_parent(isc_nmsocket_t *sock) {
 	REQUIRE(VALID_NMSOCK(sock));
 	REQUIRE(sock->type == isc_nm_udplistener);
 
-	for (int i = 0; i < sock->nchildren; i++) {
+	for (size_t i = 0; i < sock->nchildren; i++) {
 		isc__netievent_udpstop_t *ievent = NULL;
 		isc_nmsocket_t *csock = &sock->children[i];
 		REQUIRE(VALID_NMSOCK(csock));
diff --git a/lib/isc/netmgr/uv-compat.c b/lib/isc/netmgr/uv-compat.c
index 1d60ae63120..a0e1add2d78 100644
--- a/lib/isc/netmgr/uv-compat.c
+++ b/lib/isc/netmgr/uv-compat.c
@@ -52,13 +52,17 @@ int
 isc_uv_udp_freebind(uv_udp_t *handle, const struct sockaddr *addr,
 		    unsigned int flags) {
 	int r;
-	int fd;
+	uv_os_sock_t fd;
 
 	r = uv_fileno((const uv_handle_t *)handle, (uv_os_fd_t *)&fd);
 	if (r < 0) {
 		return (r);
 	}
 
+#if defined(WIN32)
+	REQUIRE(fd != INVALID_SOCKET);
+#endif
+
 	r = uv_udp_bind(handle, addr, flags);
 	if (r == UV_EADDRNOTAVAIL &&
 	    isc__nm_socket_freebind(fd, addr->sa_family) == ISC_R_SUCCESS)
@@ -104,7 +108,7 @@ int
 isc_uv_tcp_freebind(uv_tcp_t *handle, const struct sockaddr *addr,
 		    unsigned int flags) {
 	int r;
-	int fd;
+	uv_os_sock_t fd;
 
 	r = uv_fileno((const uv_handle_t *)handle, (uv_os_fd_t *)&fd);
 	if (r < 0) {