From 1d64d4cde8a4876fbf6461c54b0ecbb8604627c9 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Ond=C5=99ej=20Sur=C3=BD?= <ondrej@isc.org>
Date: Thu, 18 Mar 2021 18:14:38 +0100
Subject: [PATCH] Fix memory accounting bug in TLSDNS

After a partial write the tls.senddata buffer would be rearranged to
contain only the data tha wasn't sent and the len part would be made
shorter, which would lead to attempt to free only part of a socket's
tls.senddata buffer.
---
 lib/isc/netmgr/netmgr-int.h |  2 +-
 lib/isc/netmgr/tlsdns.c     | 35 ++++++++++++++++++-----------------
 2 files changed, 19 insertions(+), 18 deletions(-)

diff --git a/lib/isc/netmgr/netmgr-int.h b/lib/isc/netmgr/netmgr-int.h
index c558bc1f8f..70d4721f99 100644
--- a/lib/isc/netmgr/netmgr-int.h
+++ b/lib/isc/netmgr/netmgr-int.h
@@ -806,7 +806,7 @@ struct isc_nmsocket {
 			TLS_STATE_ERROR,
 			TLS_STATE_CLOSING
 		} state;
-		uv_buf_t senddata;
+		isc_region_t senddata;
 		bool cycle;
 		isc_result_t pending_error;
 		/* List of active send requests. */
diff --git a/lib/isc/netmgr/tlsdns.c b/lib/isc/netmgr/tlsdns.c
index d03bd69a8e..cbff0f5860 100644
--- a/lib/isc/netmgr/tlsdns.c
+++ b/lib/isc/netmgr/tlsdns.c
@@ -1063,12 +1063,12 @@ static void
 free_senddata(isc_nmsocket_t *sock) {
 	REQUIRE(VALID_NMSOCK(sock));
 	REQUIRE(sock->tls.senddata.base != NULL);
-	REQUIRE(sock->tls.senddata.len > 0);
+	REQUIRE(sock->tls.senddata.length > 0);
 
 	isc_mem_put(sock->mgr->mctx, sock->tls.senddata.base,
-		    sock->tls.senddata.len);
+		    sock->tls.senddata.length);
 	sock->tls.senddata.base = NULL;
-	sock->tls.senddata.len = 0;
+	sock->tls.senddata.length = 0;
 }
 
 static void
@@ -1105,7 +1105,7 @@ tls_cycle_output(isc_nmsocket_t *sock) {
 		int err;
 
 		if (sock->tls.senddata.base != NULL ||
-		    sock->tls.senddata.len > 0) {
+		    sock->tls.senddata.length > 0) {
 			break;
 		}
 
@@ -1114,42 +1114,43 @@ tls_cycle_output(isc_nmsocket_t *sock) {
 		}
 
 		sock->tls.senddata.base = isc_mem_get(sock->mgr->mctx, pending);
-		sock->tls.senddata.len = pending;
+		sock->tls.senddata.length = pending;
 
-		rv = BIO_read_ex(sock->tls.app_rbio, sock->tls.senddata.base,
-				 pending, &bytes);
+		req = isc__nm_uvreq_get(sock->mgr, sock);
+		req->uvbuf.base = (char *)sock->tls.senddata.base;
+		req->uvbuf.len = sock->tls.senddata.length;
+
+		rv = BIO_read_ex(sock->tls.app_rbio, req->uvbuf.base,
+				 req->uvbuf.len, &bytes);
 
 		RUNTIME_CHECK(rv == 1);
 		INSIST((size_t)pending == bytes);
 
-		err = uv_try_write(&sock->uv_handle.stream, &sock->tls.senddata,
-				   1);
+		err = uv_try_write(&sock->uv_handle.stream, &req->uvbuf, 1);
 
 		if (err == pending) {
 			/* Wrote everything, restart */
+			isc__nm_uvreq_put(&req, sock);
 			free_senddata(sock);
 			continue;
 		}
 
 		if (err > 0) {
 			/* Partial write, send rest asynchronously */
-			memmove(sock->tls.senddata.base,
-				sock->tls.senddata.base + err, pending - err);
-			sock->tls.senddata.len = pending - err;
+			memmove(req->uvbuf.base, req->uvbuf.base + err,
+				req->uvbuf.len - err);
+			req->uvbuf.len = req->uvbuf.len - err;
 		} else if (err == UV_ENOSYS || err == UV_EAGAIN) {
 			/* uv_try_write is not supported, send asynchronously */
 		} else {
 			result = isc__nm_uverr2result(err);
+			isc__nm_uvreq_put(&req, sock);
 			free_senddata(sock);
 			break;
 		}
 
-		req = isc__nm_uvreq_get(sock->mgr, sock);
-		req->uvbuf.base = (char *)sock->tls.senddata.base;
-		req->uvbuf.len = sock->tls.senddata.len;
-
 		err = uv_write(&req->uv_req.write, &sock->uv_handle.stream,
-			       &sock->tls.senddata, 1, tls_write_cb);
+			       &req->uvbuf, 1, tls_write_cb);
 
 		INSIST(err == 0);