sd-bus: allow receiving messages with MSG_CTRUNC set

In the event that we can't receive all of the fds from the message
(which can happen for a number of reasons including LSM denials or
hitting the fd limit of the process) the kernel will set the MSG_CTRUNC
flag.  Through our use of recvmsg_safe() we've been treating this as a
fatal error, which will result in dropping the connection.

Let's dial that back a bit: we can receive the message, but when the
user attempts to access the missing fds via sd_bus_message_read_basic()
we can return the (existing) error code of -EBADMSG to indicate that the
fd is missing.

We can do this by using recvmsg() directly, and relaxing some of the
checks on message creation: when (and only when) we have received
MSG_CTRUNC we allow a smaller than expected (per the header) number of
fds to be present.  The error check in sd_bus_message_read_basic() was
already there so we don't need to do anything about that.

This puts the receiver of the message into a difficult situation: you
can call sd_bus_message_read_basic() as often as you want but as long as
it keeps returning -EBADMSG it won't progress through the message and
you won't be able to close whatever container you're in.  That means
that the user will probably need to abandon processing the message
anyway.  So why not just drop the message up front?  This approach is
more likely to yield a useful error message, which will be invaluable
for people trying to track down problems caused by LSM denials.

Fixes #34688

diff --git a/src/libsystemd/sd-bus/bus-internal.h b/src/libsystemd/sd-bus/bus-internal.h
index 999e62f0081851cf4f08e58ac76588abb0493885..19a3b67d12f6aeb69b9b6976d568c27668e99491 100644 (file)
--- a/src/libsystemd/sd-bus/bus-internal.h
+++ b/src/libsystemd/sd-bus/bus-internal.h
@@ -255,8 +255,10 @@ typedef struct sd_bus {
 
         uint64_t creds_mask;
 
+        /* Accumulated fds from multiple recvmsg() calls for a single D-Bus message */
         int *fds;
         size_t n_fds;
+        bool got_ctrunc; /* MSG_CTRUNC was seen during any recvmsg() */
 
         char *exec_path;
         char **exec_argv;

diff --git a/src/libsystemd/sd-bus/bus-message.c b/src/libsystemd/sd-bus/bus-message.c
index a77f45e634b40919ae8d9407708d9983b5ad87b2..9d71823b38fee693d56f688e7d24b534670bb384 100644 (file)
--- a/src/libsystemd/sd-bus/bus-message.c
+++ b/src/libsystemd/sd-bus/bus-message.c
@@ -17,7 +17,7 @@
 #include "utf8.h"
 
 static int message_append_basic(sd_bus_message *m, char type, const void *p, const void **stored);
-static int message_parse_fields(sd_bus_message *m);
+static int message_parse_fields(sd_bus_message *m, bool got_ctrunc);
 
 static void* adjust_pointer(const void *p, void *old_base, size_t sz, void *new_base) {
 
@@ -408,6 +408,7 @@ int bus_message_from_malloc(
                 size_t length,
                 int *fds,
                 size_t n_fds,
+                bool got_ctrunc,
                 const char *label,
                 sd_bus_message **ret) {
 
@@ -437,7 +438,7 @@ int bus_message_from_malloc(
         m->iovec = m->iovec_fixed;
         m->iovec[0] = IOVEC_MAKE(buffer, length);
 
-        r = message_parse_fields(m);
+        r = message_parse_fields(m, got_ctrunc);
         if (r < 0)
                 return r;
 
@@ -4029,8 +4030,8 @@ static int message_skip_fields(
         }
 }
 
-static int message_parse_fields(sd_bus_message *m) {
-        uint32_t unix_fds = 0;
+static int message_parse_fields(sd_bus_message *m, bool got_ctrunc) {
+        uint32_t n_unix_fds_declared = 0;
         bool unix_fds_set = false;
         int r;
 
@@ -4183,7 +4184,7 @@ static int message_parse_fields(sd_bus_message *m) {
                         if (!streq(signature, "u"))
                                 return -EBADMSG;
 
-                        r = message_peek_field_uint32(m, &ri, item_size, &unix_fds);
+                        r = message_peek_field_uint32(m, &ri, item_size, &n_unix_fds_declared);
                         if (r < 0)
                                 return -EBADMSG;
 
@@ -4197,8 +4198,26 @@ static int message_parse_fields(sd_bus_message *m) {
                         return r;
         }
 
-        if (m->n_fds != unix_fds)
-                return -EBADMSG;
+        /* Validate that the number of fds we actually received via SCM_RIGHTS matches (or is compatible
+         * with) the number declared in the message header.
+         *
+         * Normally these must match exactly. However, when MSG_CTRUNC was set during recvmsg(), the kernel
+         * might have truncated the fd array (e.g., due to LSM denials blocking fd passing). In that case,
+         * we also accept fewer fds than declared. Any attempt to actually use a truncated fd will fail later
+         * when sd_bus_message_read_basic() finds the fd index out of range. Too many fds is always wrong. */
+        if (m->n_fds > n_unix_fds_declared)
+                return log_error_errno(SYNTHETIC_ERRNO(EBADMSG),
+                                       "Received a bus message with too many fds: %" PRIu32 " received vs. %" PRIu32 " declared",
+                                       m->n_fds, n_unix_fds_declared);
+
+        if (m->n_fds < n_unix_fds_declared && !got_ctrunc)
+                return log_error_errno(SYNTHETIC_ERRNO(EBADMSG),
+                                       "Received a bus message with too few fds: %" PRIu32 " received vs. %" PRIu32 " declared",
+                                       m->n_fds, n_unix_fds_declared);
+
+        if (got_ctrunc)
+                log_error("Received a bus message with MSG_CTRUNC set with %" PRIu32 " fds received vs %" PRIu32 " declared",
+                          m->n_fds, n_unix_fds_declared);
 
         switch (m->header->type) {
 

diff --git a/src/libsystemd/sd-bus/bus-message.h b/src/libsystemd/sd-bus/bus-message.h
index 90939b750b3f5279f025720669d4337ecee89ea6..c15e947fbf8d192a7c24c338566069a0522f3791 100644 (file)
--- a/src/libsystemd/sd-bus/bus-message.h
+++ b/src/libsystemd/sd-bus/bus-message.h
@@ -177,6 +177,7 @@ int bus_message_from_malloc(
                 size_t length,
                 int *fds,
                 size_t n_fds,
+                bool got_ctrunc,
                 const char *label,
                 sd_bus_message **ret);
 

diff --git a/src/libsystemd/sd-bus/bus-socket.c b/src/libsystemd/sd-bus/bus-socket.c
index 6bb2ab4c871b94c159ac1bc6da4de34352972e4c..45f74cc7c71177efb8cca06cb67d269a36fdaa7e 100644 (file)
--- a/src/libsystemd/sd-bus/bus-socket.c
+++ b/src/libsystemd/sd-bus/bus-socket.c
@@ -590,14 +590,25 @@ static int bus_process_cmsg(sd_bus *bus, struct msghdr *mh, bool allow_fds) {
                 return 0;
         }
 
-        if (!GREEDY_REALLOC(bus->fds, bus->n_fds + n_fds))
-                return -ENOMEM;
+        /* Check if we previously received fds with MSG_CTRUNC set. When the kernel truncates the fd array,
+         * it drops all fds from the blocked one onwards - we have no way to know how many were lost or which
+         * indexes are affected. Any fds we receive now would be appended at the wrong indexes, corrupting
+         * the message's fd table. We must silently drop them to avoid a worse mess. Reading the message will
+         * still fail later when the user tries to access a truncated fd. */
+        if (!bus->got_ctrunc) {
+                if (!GREEDY_REALLOC(bus->fds, bus->n_fds + n_fds))
+                        return -ENOMEM;
+
+                FOREACH_ARRAY(i, fds, n_fds)
+                        bus->fds[bus->n_fds++] = fd_move_above_stdio(*i);
+
+                TAKE_PTR(fds);
+                n_fds = 0;
+        }
 
-        FOREACH_ARRAY(i, fds, n_fds)
-                bus->fds[bus->n_fds++] = fd_move_above_stdio(*i);
+        if (FLAGS_SET(mh->msg_flags, MSG_CTRUNC))
+                bus->got_ctrunc = true;
 
-        TAKE_PTR(fds);
-        n_fds = 0;
         return 0;
 }
 
@@ -646,7 +657,7 @@ static int bus_socket_read_auth(sd_bus *b) {
                         .msg_controllen = sizeof(control),
                 };
 
-                k = recvmsg_safe(b->input_fd, &mh, MSG_DONTWAIT|MSG_CMSG_CLOEXEC);
+                k = RET_NERRNO(recvmsg(b->input_fd, &mh, MSG_DONTWAIT|MSG_CMSG_CLOEXEC));
                 if (k == -ENOTSOCK) {
                         b->prefer_readv = true;
                         k = readv(b->input_fd, &iov, 1);
@@ -1357,6 +1368,7 @@ static int bus_socket_make_message(sd_bus *bus, size_t size) {
         r = bus_message_from_malloc(bus,
                                     bus->rbuffer, size,
                                     bus->fds, bus->n_fds,
+                                    bus->got_ctrunc,
                                     NULL,
                                     &t);
         if (r == -EBADMSG) {
@@ -1373,6 +1385,7 @@ static int bus_socket_make_message(sd_bus *bus, size_t size) {
 
         bus->fds = NULL;
         bus->n_fds = 0;
+        bus->got_ctrunc = false;
 
         if (t) {
                 t->read_counter = ++bus->read_counter;
@@ -1423,7 +1436,7 @@ int bus_socket_read_message(sd_bus *bus) {
                         .msg_controllen = sizeof(control),
                 };
 
-                k = recvmsg_safe(bus->input_fd, &mh, MSG_DONTWAIT|MSG_CMSG_CLOEXEC);
+                k = RET_NERRNO(recvmsg(bus->input_fd, &mh, MSG_DONTWAIT|MSG_CMSG_CLOEXEC));
                 if (k == -ENOTSOCK) {
                         bus->prefer_readv = true;
                         k = readv(bus->input_fd, &iov, 1);

diff --git a/src/libsystemd/sd-bus/fuzz-bus-message.c b/src/libsystemd/sd-bus/fuzz-bus-message.c
index e2c4167f3c9d8faef448b4f51bfc221e7ddba740..e971059610fad15c17c06cd8856172d14c0f10db 100644 (file)
--- a/src/libsystemd/sd-bus/fuzz-bus-message.c
+++ b/src/libsystemd/sd-bus/fuzz-bus-message.c
@@ -23,7 +23,7 @@ int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size) {
 
         assert_se(buffer = memdup(data, size));
 
-        r = bus_message_from_malloc(bus, buffer, size, NULL, 0, NULL, &m);
+        r = bus_message_from_malloc(bus, buffer, size, NULL, 0, /* got_ctrunc= */ false, NULL, &m);
         if (r == -EBADMSG)
                 return 0;
         assert_se(r >= 0);

diff --git a/src/libsystemd/sd-bus/test-bus-marshal.c b/src/libsystemd/sd-bus/test-bus-marshal.c
index d52683ea87ee2a5fb0b96d8a7298ddb1a2207fe7..04263f4a1efae3076501225bc7337eab78128a23 100644 (file)
--- a/src/libsystemd/sd-bus/test-bus-marshal.c
+++ b/src/libsystemd/sd-bus/test-bus-marshal.c
@@ -243,7 +243,7 @@ int main(int argc, char *argv[]) {
 
         m = sd_bus_message_unref(m);
 
-        r = bus_message_from_malloc(bus, buffer, sz, NULL, 0, NULL, &m);
+        r = bus_message_from_malloc(bus, buffer, sz, NULL, 0, /* got_ctrunc= */ false, NULL, &m);
         assert_se(r >= 0);
 
         sd_bus_message_dump(m, stdout, SD_BUS_MESSAGE_DUMP_WITH_HEADER);