From: Volker Lendecke <vl@samba.org>
Date: Thu, 21 Jan 2021 17:33:58 +0000 (+0100)
Subject: messaging: Fix receiving file descriptors
X-Git-Tag: tevent-0.11.0~1442
X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=cf0c773ca56e0ae1b3123997d7910e0e0e248184;p=thirdparty%2Fsamba.git

messaging: Fix receiving file descriptors

Don't close unconsumed file descriptors in messaging_recv_cb(). Via
multiple registrations on different tevent contexts we might call
messaging_recv_cb() multiple times: All but the first tevent context
handled in the loop in msg_dgm_ref_recv() will not see file
descriptors anymore, it will just get a -1, even if the first
reference had no receiver interested in the fds.

Change the API such that consumers can set the file descriptor to -1
if it's consumed. If nobody wanted them, do the close where they were
created via recvmsg, in messages_dgm.c.

If you want multiple handlers to consume the file descriptors, you
should dup() them in the filter function handed to
messaging_filtered_read_send and save the duplicate in your private
data for later consumption.

Signed-off-by: Volker Lendecke <vl@samba.org>
Reviewed-by: Jeremy Allison <jra@samba.org>

Autobuild-User(master): Volker Lendecke <vl@samba.org>
Autobuild-Date(master): Fri Mar 19 08:18:26 UTC 2021 on sn-devel-184
---

diff --git a/lib/messaging/messages_dgm.c b/lib/messaging/messages_dgm.c
index 733cd19d3b8..ef065def4d9 100644
--- a/lib/messaging/messages_dgm.c
+++ b/lib/messaging/messages_dgm.c
@@ -1323,6 +1323,18 @@ static int messaging_dgm_in_msg_destructor(struct messaging_dgm_in_msg *m)
 	return 0;
 }
 
+static void messaging_dgm_close_unconsumed(int *fds, size_t num_fds)
+{
+	size_t i;
+
+	for (i=0; i<num_fds; i++) {
+		if (fds[i] != -1) {
+			close(fds[i]);
+			fds[i] = -1;
+		}
+	}
+}
+
 /*
  * Deal with identification of fragmented messages and
  * re-assembly into full messages sent, then calls the
@@ -1349,6 +1361,7 @@ static void messaging_dgm_recv(struct messaging_dgm_context *ctx,
 	if (cookie == 0) {
 		ctx->recv_cb(ev, buf, buflen, fds, num_fds,
 			     ctx->recv_cb_private_data);
+		messaging_dgm_close_unconsumed(fds, num_fds);
 		return;
 	}
 
@@ -1412,6 +1425,7 @@ static void messaging_dgm_recv(struct messaging_dgm_context *ctx,
 
 	ctx->recv_cb(ev, msg->buf, msg->msglen, fds, num_fds,
 		     ctx->recv_cb_private_data);
+	messaging_dgm_close_unconsumed(fds, num_fds);
 
 	TALLOC_FREE(msg);
 	return;
diff --git a/source3/lib/messages.c b/source3/lib/messages.c
index b63652ca1a5..8641a9dad56 100644
--- a/source3/lib/messages.c
+++ b/source3/lib/messages.c
@@ -396,21 +396,16 @@ static void messaging_recv_cb(struct tevent_context *ev,
 
 	if (msg_len < MESSAGE_HDR_LENGTH) {
 		DBG_WARNING("message too short: %zu\n", msg_len);
-		goto close_fail;
+		return;
 	}
 
 	if (num_fds > INT8_MAX) {
 		DBG_WARNING("too many fds: %zu\n", num_fds);
-		goto close_fail;
+		return;
 	}
 
-	/*
-	 * "consume" the fds by copying them and setting
-	 * the original variable to -1
-	 */
 	for (i=0; i < num_fds; i++) {
 		fds64[i] = fds[i];
-		fds[i] = -1;
 	}
 
 	rec = (struct messaging_rec) {
@@ -429,15 +424,13 @@ static void messaging_recv_cb(struct tevent_context *ev,
 
 	if (server_id_same_process(&rec.src, &msg_ctx->id)) {
 		DBG_DEBUG("Ignoring self-send\n");
-		goto close_fail;
+		return;
 	}
 
 	messaging_dispatch_rec(msg_ctx, ev, &rec);
-	return;
 
-close_fail:
-	for (i=0; i < num_fds; i++) {
-		close(fds[i]);
+	for (i=0; i<num_fds; i++) {
+		fds[i] = fds64[i];
 	}
 }
 
@@ -999,7 +992,16 @@ static struct messaging_rec *messaging_rec_dup(TALLOC_CTX *mem_ctx,
 
 	result->fds = NULL;
 	if (result->num_fds > 0) {
+		size_t i;
+
 		result->fds = talloc_memdup(result, rec->fds, fds_size);
+
+		for (i=0; i<rec->num_fds; i++) {
+			/*
+			 * fd's can only exist once
+			 */
+			rec->fds[i] = -1;
+		}
 	}
 
 	return result;
@@ -1395,16 +1397,6 @@ static void messaging_dispatch_rec(struct messaging_context *msg_ctx,
 			return;
 		}
 	}
-
-	/*
-	 * If the fd-array isn't used, just close it.
-	 */
-	for (i=0; i < rec->num_fds; i++) {
-		int fd = rec->fds[i];
-		close(fd);
-	}
-	rec->num_fds = 0;
-	rec->fds = NULL;
 }
 
 static int mess_parent_dgm_cleanup(void *private_data);