From: Allison Karlitskaya Date: Mon, 15 Dec 2025 09:27:04 +0000 (+0100) Subject: sd-bus: allow receiving messages with MSG_CTRUNC set X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=6c8de404c98fb9b965cba68360c2db1e3f55b776;p=thirdparty%2Fsystemd.git 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 999e62f0081..19a3b67d12f 100644 --- 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 a77f45e634b..9d71823b38f 100644 --- 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 90939b750b3..c15e947fbf8 100644 --- 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 6bb2ab4c871..45f74cc7c71 100644 --- 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 e2c4167f3c9..e971059610f 100644 --- 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 d52683ea87e..04263f4a1ef 100644 --- 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);