sd-bus: allow receiving messages with MSG_CTRUNC set (#40089)

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

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;

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) {
 

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);
 

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);
+                FOREACH_ARRAY(i, fds, n_fds)
+                        bus->fds[bus->n_fds++] = fd_move_above_stdio(*i);
+
+                TAKE_PTR(fds);
+                n_fds = 0;
+        }
+
+        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);

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);

src/libsystemd/sd-bus/test-bus-chat.c

@@ -2,6 +2,7 @@
 
 #include <fcntl.h>
 #include <pthread.h>
+#include <sys/resource.h>
 #include <unistd.h>
 
 #include "sd-bus.h"
@@ -10,10 +11,13 @@
 #include "bus-error.h"
 #include "bus-internal.h"
 #include "bus-match.h"
+#include "bus-message.h"
 #include "errno-util.h"
 #include "fd-util.h"
 #include "format-util.h"
 #include "log.h"
+#include "memfd-util.h"
+#include "stat-util.h"
 #include "string-util.h"
 #include "tests.h"
 #include "time-util.h"
@@ -493,6 +497,128 @@ finish:
         return INT_TO_PTR(r);
 }
 
+static ino_t get_inode(int fd) {
+        struct stat st;
+        assert_se(fstat(fd, &st) >= 0);
+        return st.st_ino;
+}
+
+static int get_one_message(sd_bus *bus, sd_bus_message **m) {
+        int r;
+
+        assert (m);
+
+        while (!*m) {
+                r = sd_bus_wait(bus, UINT64_MAX);
+                if (r < 0)
+                        return log_error_errno(r, "Failed to wait: %m");
+                r = sd_bus_process(bus, m);
+                if (r < 0)
+                        return log_error_errno(r, "Failed to process requests: %m");
+        }
+
+        return 0;
+}
+
+TEST(ctrunc) {
+        _cleanup_(sd_bus_flush_close_unrefp) sd_bus *bus = NULL;
+        _cleanup_(sd_bus_message_unrefp) sd_bus_message *recvd = NULL, *sent = NULL;
+        struct rlimit orig_rl, new_rl;
+        const char *unique;
+        const int n_fds_to_send = 64;
+        ino_t memfd_st_ino[n_fds_to_send];
+        int r;
+
+        /* Connect to the session bus and eat the NamedAcquired message */
+        r = sd_bus_open_user(&bus);
+        if (r < 0)
+                return (void) log_error_errno(r, "Cannot connect to bus: %m");
+        ASSERT_OK(get_one_message(bus, &recvd));
+        recvd = sd_bus_message_unref(recvd);
+
+        if (!sd_bus_can_send(bus, 'h'))
+                return (void) log_error("Bus does not support fd passing: %m");
+
+        /* We will create a message with 64 fds in it and set a fd limit of 128 and try to receive it.  We'll
+         * hold on to that message after we send it and then attempt to receive it back. Since various other
+         * fds will be open, with both copies of the message, we'll definitely hit the limit of 128.
+         */
+        ASSERT_OK(sd_bus_get_unique_name(bus, &unique));
+        ASSERT_OK(sd_bus_message_new_method_call(bus, &sent, unique, "/", "org.freedesktop.systemd.test", "SendFds"));
+        ASSERT_OK(sd_bus_message_open_container(sent, SD_BUS_TYPE_ARRAY, "h"));
+
+        /* Create a series of memfds, appending each to the message */
+        for (int i = 0; i < n_fds_to_send; i++) {
+                _cleanup_close_ int memfd = memfd_create_wrapper("ctrunc-test", 0);
+                ASSERT_OK(memfd);
+                memfd_st_ino[i] = get_inode(memfd);
+                ASSERT_OK(sd_bus_message_append(sent, "h", memfd));
+        }
+        ASSERT_OK(sd_bus_message_close_container(sent));
+
+        /* Send the message - keep 'sent' alive to hold the duplicated fd references */
+        ASSERT_OK(sd_bus_send(bus, sent, NULL));
+
+        /* Now turn down the fd limit, receive the message, and turn it back up again */
+        ASSERT_OK_ERRNO(getrlimit(RLIMIT_NOFILE, &orig_rl));
+        new_rl.rlim_cur = n_fds_to_send * 2;
+        new_rl.rlim_max = orig_rl.rlim_max;
+        ASSERT_OK_ERRNO(setrlimit(RLIMIT_NOFILE, &new_rl));
+
+        /* The very first message should be the one we expect */
+        ASSERT_OK(get_one_message(bus, &recvd));
+        ASSERT_TRUE(sd_bus_message_is_method_call(recvd, "org.freedesktop.systemd.test", "SendFds"));
+
+        /* This needs to succeed or the following tests are going to be unhappy... */
+        ASSERT_EQ(setrlimit(RLIMIT_NOFILE, &orig_rl), 0);
+
+        /* Try to read all the fds. We expect at least one to fail with -EBADMSG due to
+         * truncation, and all subsequent reads must also fail with -EBADMSG. */
+        int i;
+        ASSERT_OK(sd_bus_message_enter_container(recvd, SD_BUS_TYPE_ARRAY, "h"));
+        for (i = 0; i < n_fds_to_send; i++) {
+                int fd; /* weakly owned: the fd belongs to the message */
+                r = sd_bus_message_read_basic(recvd, 'h', &fd);
+                if (r == -EBADMSG)
+                        /* Good!  We were expecting this! */
+                        break;
+                ASSERT_OK(r);
+                ASSERT_EQ(get_inode(fd), memfd_st_ino[i]);
+        }
+
+        /* Make sure we successfully sent at least one fd but not all of them */
+        ASSERT_GT(i, 0);
+        ASSERT_LT(i, n_fds_to_send);
+        log_info("fds truncated at %i", i);
+
+        /* At this point we're stuck.  We can call sd_bus_message_read_basic() as often as we want, but we
+         * won't be able to make progress and won't be able to close the array or read anything else in the
+         * message.
+         */
+        for (i = 0; i < 2 * n_fds_to_send; i++) {
+                int fd; /* weakly owned: the fd belongs to the message */
+                ASSERT_ERROR(sd_bus_message_read_basic(recvd, 'h', &fd), EBADMSG);
+        }
+        ASSERT_ERROR(sd_bus_message_exit_container(recvd), EBUSY);
+        recvd = sd_bus_message_unref(recvd);
+
+        /* Send the message again without the fd limits to make sure the connection still works */
+        ASSERT_OK(sd_bus_send(bus, sent, NULL));
+        ASSERT_OK(get_one_message(bus, &recvd));
+        ASSERT_TRUE(sd_bus_message_is_method_call(recvd, "org.freedesktop.systemd.test", "SendFds"));
+
+        /* Read all the fds. */
+        ASSERT_EQ(sd_bus_message_enter_container(recvd, SD_BUS_TYPE_ARRAY, "h"), 1);
+        for (i = 0; i < n_fds_to_send; i++) {
+                int fd; /* weakly owned: the fd belongs to the message */
+                ASSERT_OK(sd_bus_message_read_basic(recvd, 'h', &fd));
+                ASSERT_EQ(get_inode(fd), memfd_st_ino[i]);
+        }
+        ASSERT_OK(sd_bus_message_exit_container(recvd));
+
+        log_info("MSG_CTRUNC test passed");
+}
+
 TEST(chat) {
         _cleanup_(sd_bus_flush_close_unrefp) sd_bus *bus = NULL;
         pthread_t c1, c2;

src/libsystemd/sd-bus/test-bus-marshal.c

@@ -19,12 +19,16 @@ REENABLE_WARNING
 #include "sd-bus.h"
 
 #include "alloc-util.h"
+#include "bus-internal.h"
 #include "bus-label.h"
 #include "bus-message.h"
 #include "bus-util.h"
 #include "escape.h"
+#include "fd-util.h"
 #include "log.h"
+#include "memfd-util.h"
 #include "memstream-util.h"
+#include "stat-util.h"
 #include "tests.h"
 
 static void test_bus_path_encode_unique(void) {
@@ -99,6 +103,119 @@ static void test_bus_label_escape_one(const char *a, const char *b) {
         assert_se(streq(a, y));
 }
 
+static ino_t get_inode(int fd) {
+        struct stat st;
+        assert_se(fstat(fd, &st) >= 0);
+        return st.st_ino;
+}
+
+static void test_bus_fds_truncated(void) {
+        _cleanup_(sd_bus_unrefp) sd_bus *bus = NULL;
+        _cleanup_(sd_bus_message_unrefp) sd_bus_message *m = NULL;
+        _cleanup_close_ int memfd0 = -EBADF, memfd1 = -EBADF;
+        _cleanup_free_ void *blob = NULL;
+        _cleanup_free_ int *fds = NULL;
+        ino_t ino0, ino1;
+        size_t blob_size;
+        int fd;
+
+        /* Create two memfds and record their inodes for later verification */
+        memfd0 = ASSERT_OK(memfd_create_wrapper("test-fd-0", 0));
+        ino0 = get_inode(memfd0);
+
+        memfd1 = ASSERT_OK(memfd_create_wrapper("test-fd-1", 0));
+        ino1 = get_inode(memfd1);
+
+        /* Create a bus for message operations (no actual connection needed) */
+        ASSERT_OK(sd_bus_new(&bus));
+        bus->state = BUS_RUNNING; /* Fake state to allow message creation */
+        bus->can_fds = true;      /* Allow fd passing */
+
+        /* Build a message containing two fds */
+        ASSERT_OK(sd_bus_message_new_method_call(bus, &m, "foo.bar", "/", "foo.bar", "Ping"));
+        ASSERT_OK(sd_bus_message_append(m, "hh", memfd0, memfd1));
+        ASSERT_OK(sd_bus_message_seal(m, 1, 0));
+
+        /* Serialize the message to a blob */
+        ASSERT_OK(bus_message_get_blob(m, &blob, &blob_size));
+        m = sd_bus_message_unref(m);
+
+        /* Duplicate the fds since bus_message_from_malloc() takes ownership */
+        fds = ASSERT_NOT_NULL(new(int, 2));
+        fds[0] = ASSERT_OK_ERRNO(fcntl(memfd0, F_DUPFD_CLOEXEC, 3));
+        fds[1] = ASSERT_OK_ERRNO(fcntl(memfd1, F_DUPFD_CLOEXEC, 3));
+
+        /* Test 1: Parse with correct fd count, no truncation - should succeed */
+        log_info("Test 1: Exact fd count match, got_ctrunc=false");
+        void *blob_copy = ASSERT_NOT_NULL(memdup(blob, blob_size));
+        ASSERT_OK(bus_message_from_malloc(bus, blob_copy, blob_size, fds, 2, /* got_ctrunc= */ false, NULL, &m));
+
+        /* Verify we can read both fds and they have the expected inodes */
+        ASSERT_OK(sd_bus_message_read_basic(m, 'h', &fd));
+        ASSERT_EQ(get_inode(fd), ino0);
+        ASSERT_OK(sd_bus_message_read_basic(m, 'h', &fd));
+        ASSERT_EQ(get_inode(fd), ino1);
+
+        m = sd_bus_message_unref(m);
+        fds = NULL; /* ownership transferred */
+
+        /* Test 2: Parse with fewer fds than declared, no truncation flag - should fail */
+        log_info("Test 2: Fewer fds than declared, got_ctrunc=false");
+        fds = ASSERT_NOT_NULL(new(int, 1));
+        fds[0] = ASSERT_OK_ERRNO(fcntl(memfd0, F_DUPFD_CLOEXEC, 3));
+
+        blob_copy = ASSERT_NOT_NULL(memdup(blob, blob_size));
+        ASSERT_ERROR(bus_message_from_malloc(bus, blob_copy, blob_size, fds, 1, /* got_ctrunc= */ false, NULL, &m), EBADMSG);
+        free(blob_copy);
+        close(fds[0]);
+        fds = mfree(fds);
+
+        /* Test 3: Parse with fewer fds than declared, with truncation flag - parsing should succeed */
+        log_info("Test 3: Fewer fds than declared, got_ctrunc=true");
+        fds = ASSERT_NOT_NULL(new(int, 1));
+        fds[0] = ASSERT_OK_ERRNO(fcntl(memfd0, F_DUPFD_CLOEXEC, 3));
+
+        blob_copy = ASSERT_NOT_NULL(memdup(blob, blob_size));
+        ASSERT_OK(bus_message_from_malloc(bus, blob_copy, blob_size, fds, 1, /* got_ctrunc= */ true, NULL, &m));
+
+        /* First fd should be readable and have correct inode */
+        ASSERT_OK(sd_bus_message_read_basic(m, 'h', &fd));
+        ASSERT_EQ(get_inode(fd), ino0);
+
+        /* Second fd was truncated - reading it should fail */
+        ASSERT_ERROR(sd_bus_message_read_basic(m, 'h', &fd), EBADMSG);
+
+        m = sd_bus_message_unref(m);
+        fds = NULL; /* ownership transferred */
+
+        /* Test 4: Parse with more fds than declared, with truncation flag - should fail */
+        log_info("Test 4: More fds than declared, got_ctrunc=true");
+        fds = ASSERT_NOT_NULL(new(int, 3));
+        fds[0] = ASSERT_OK_ERRNO(fcntl(memfd0, F_DUPFD_CLOEXEC, 3));
+        fds[1] = ASSERT_OK_ERRNO(fcntl(memfd1, F_DUPFD_CLOEXEC, 3));
+        fds[2] = ASSERT_OK_ERRNO(fcntl(memfd0, F_DUPFD_CLOEXEC, 3));
+
+        blob_copy = ASSERT_NOT_NULL(memdup(blob, blob_size));
+        ASSERT_ERROR(bus_message_from_malloc(bus, blob_copy, blob_size, fds, 3, /* got_ctrunc= */ true, NULL, &m), EBADMSG);
+        free(blob_copy);
+        close(fds[0]);
+        close(fds[1]);
+        close(fds[2]);
+        fds = mfree(fds);
+
+        /* Test 5: Parse with zero fds when two were declared, with truncation flag - should succeed */
+        log_info("Test 5: Zero fds when some declared, got_ctrunc=true");
+        blob_copy = ASSERT_NOT_NULL(memdup(blob, blob_size));
+        ASSERT_OK(bus_message_from_malloc(bus, blob_copy, blob_size, NULL, 0, /* got_ctrunc= */ true, NULL, &m));
+
+        /* Both fd reads should fail since all were truncated */
+        ASSERT_ERROR(sd_bus_message_read_basic(m, 'h', &fd), EBADMSG);
+
+        m = sd_bus_message_unref(m);
+
+        log_info("All fd truncation tests passed");
+}
+
 static void test_bus_label_escape(void) {
         test_bus_label_escape_one("foo123bar", "foo123bar");
         test_bus_label_escape_one("foo.bar", "foo_2ebar");
@@ -243,7 +360,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);
@@ -415,6 +532,7 @@ int main(int argc, char *argv[]) {
         test_bus_path_encode();
         test_bus_path_encode_unique();
         test_bus_path_encode_many();
+        test_bus_fds_truncated();
 
         return 0;
 }