From: Allison Karlitskaya Date: Mon, 15 Dec 2025 09:35:56 +0000 (+0100) Subject: sd-bus: add test cases for truncated fds X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=refs%2Fpull%2F40089%2Fhead;p=thirdparty%2Fsystemd.git sd-bus: add test cases for truncated fds We add some test cases for the previous commits: first (with Claude's help) we exercise the message creation API internally by passing it various combinations of incorrect fds with the might_be_truncated flag set to true or false. Then we try more of a "real world" test by lowering our fd limit and sending ourselves a message via the bus and making sure that we successfully receive a message that has had at least some of its fds truncated. --- diff --git a/src/libsystemd/sd-bus/test-bus-chat.c b/src/libsystemd/sd-bus/test-bus-chat.c index 7298f4d46c9..3544b4580b7 100644 --- a/src/libsystemd/sd-bus/test-bus-chat.c +++ b/src/libsystemd/sd-bus/test-bus-chat.c @@ -2,6 +2,7 @@ #include #include +#include #include #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; diff --git a/src/libsystemd/sd-bus/test-bus-marshal.c b/src/libsystemd/sd-bus/test-bus-marshal.c index 04263f4a1ef..e26ca433447 100644 --- a/src/libsystemd/sd-bus/test-bus-marshal.c +++ b/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"); @@ -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; }