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 733cd19d3b8d27a148d91ef8e5c9fa9f422ec213..ef065def4d9d90c660f863a80b26493cde81550a 100644 (file)
--- 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 b63652ca1a5a21da98bbd79270820630878e4db4..8641a9dad564a2b14ca9d00164952f7a24509dba 100644 (file)
--- 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);