From: Ondřej Surý <ondrej@sury.org>
Date: Tue, 20 Oct 2020 06:07:44 +0000 (+0200)
Subject: Fix the way udp_send_direct() is used
X-Git-Tag: v9.17.7~46^2~10
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=afca2e3b2198eb43a97fe6edb3c5a46915389e09;p=thirdparty%2Fbind9.git

Fix the way udp_send_direct() is used

There were two problems how udp_send_direct() was used:

1. The udp_send_direct() can return ISC_R_CANCELED (or translated error
   from uv_udp_send()), but the isc__nm_async_udpsend() wasn't checking
   the error code and not releasing the uvreq in case of an error.

2. In isc__nm_udp_send(), when the UDP send is already in the right
   netthread, it uses udp_send_direct() to send the UDP packet right
   away.  When that happened the uvreq was not freed, and the error code
   was returned to the caller.  We need to return ISC_R_SUCCESS and
   rather use the callback to report an error in such case.
---

diff --git a/lib/isc/netmgr/udp.c b/lib/isc/netmgr/udp.c
index f87ab95a5fc..ed49b47274b 100644
--- a/lib/isc/netmgr/udp.c
+++ b/lib/isc/netmgr/udp.c
@@ -475,9 +475,14 @@ isc__nm_udp_send(isc_nmhandle_t *handle, isc_region_t *region, isc_nm_cb_t cb,
 	if (isc_nm_tid() == rsock->tid) {
 		/*
 		 * If we're in the same thread as the socket we can send the
-		 * data directly
+		 * data directly, but we need to emulate returning the error via
+		 * the callback, not directly to keep the API.
 		 */
-		return (udp_send_direct(rsock, uvreq, peer));
+		isc_result_t result = udp_send_direct(rsock, uvreq, peer);
+		if (result != ISC_R_SUCCESS) {
+			uvreq->cb.send(uvreq->handle, result, uvreq->cbarg);
+			isc__nm_uvreq_put(&uvreq, sock);
+		}
 	} else {
 		/*
 		 * We need to create an event and pass it using async channel
@@ -489,8 +494,9 @@ isc__nm_udp_send(isc_nmhandle_t *handle, isc_region_t *region, isc_nm_cb_t cb,
 
 		isc__nm_enqueue_ievent(&sock->mgr->workers[rsock->tid],
 				       (isc__netievent_t *)ievent);
-		return (ISC_R_SUCCESS);
 	}
+
+	return (ISC_R_SUCCESS);
 }
 
 /*
@@ -498,16 +504,19 @@ isc__nm_udp_send(isc_nmhandle_t *handle, isc_region_t *region, isc_nm_cb_t cb,
  */
 void
 isc__nm_async_udpsend(isc__networker_t *worker, isc__netievent_t *ev0) {
+	isc_result_t result;
 	isc__netievent_udpsend_t *ievent = (isc__netievent_udpsend_t *)ev0;
+	isc_nmsocket_t *sock = ievent->sock;
+	isc__nm_uvreq_t *uvreq = ievent->req;
 
-	REQUIRE(worker->id == ievent->sock->tid);
+	REQUIRE(sock->type == isc_nm_udpsocket);
+	REQUIRE(worker->id == sock->tid);
 
-	if (isc__nmsocket_active(ievent->sock)) {
-		udp_send_direct(ievent->sock, ievent->req, &ievent->peer);
-	} else {
-		ievent->req->cb.send(ievent->req->handle, ISC_R_CANCELED,
-				     ievent->req->cbarg);
-		isc__nm_uvreq_put(&ievent->req, ievent->req->sock);
+	result = udp_send_direct(sock, uvreq, &ievent->peer);
+	if (result != ISC_R_SUCCESS) {
+		isc__nm_incstats(sock->mgr, sock->statsindex[STATID_SENDFAIL]);
+		uvreq->cb.send(uvreq->handle, result, uvreq->cbarg);
+		isc__nm_uvreq_put(&uvreq, sock);
 	}
 }
 
@@ -515,14 +524,14 @@ static void
 udp_send_cb(uv_udp_send_t *req, int status) {
 	isc_result_t result = ISC_R_SUCCESS;
 	isc__nm_uvreq_t *uvreq = (isc__nm_uvreq_t *)req->data;
+	isc_nmsocket_t *sock = uvreq->sock;
 
 	REQUIRE(VALID_UVREQ(uvreq));
 	REQUIRE(VALID_NMHANDLE(uvreq->handle));
 
 	if (status < 0) {
 		result = isc__nm_uverr2result(status);
-		isc__nm_incstats(uvreq->sock->mgr,
-				 uvreq->sock->statsindex[STATID_SENDFAIL]);
+		isc__nm_incstats(sock->mgr, sock->statsindex[STATID_SENDFAIL]);
 	}
 
 	uvreq->cb.send(uvreq->handle, result, uvreq->cbarg);
@@ -537,8 +546,10 @@ static isc_result_t
 udp_send_direct(isc_nmsocket_t *sock, isc__nm_uvreq_t *req,
 		isc_sockaddr_t *peer) {
 	const struct sockaddr *sa = NULL;
-	int rv;
+	int r;
 
+	REQUIRE(VALID_NMSOCK(sock));
+	REQUIRE(VALID_UVREQ(req));
 	REQUIRE(sock->tid == isc_nm_tid());
 	REQUIRE(sock->type == isc_nm_udpsocket);
 
@@ -547,12 +558,10 @@ udp_send_direct(isc_nmsocket_t *sock, isc__nm_uvreq_t *req,
 	}
 
 	sa = atomic_load(&sock->connected) ? NULL : &peer->type.sa;
-	rv = uv_udp_send(&req->uv_req.udp_send, &sock->uv_handle.udp,
-			 &req->uvbuf, 1, sa, udp_send_cb);
-	if (rv < 0) {
-		isc__nm_incstats(req->sock->mgr,
-				 req->sock->statsindex[STATID_SENDFAIL]);
-		return (isc__nm_uverr2result(rv));
+	r = uv_udp_send(&req->uv_req.udp_send, &sock->uv_handle.udp,
+			&req->uvbuf, 1, sa, udp_send_cb);
+	if (r < 0) {
+		return (isc__nm_uverr2result(r));
 	}
 
 	return (ISC_R_SUCCESS);