packages/dbus-broker
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity
dbus-broker/add-unit-test-for-dbus-broker.patch
hongjinghao 0dc98d4d48 fix buffer leakage 
(cherry picked from commit 47c772518dfeee51c3f5e3ef7badca5fa4d58407)
2023-10-10 09:21:45 +08:00

579 lines
22 KiB
Diff
Raw Blame History

From 2c51932c86b5dfc3a81b9203875ca45216f45c3a Mon Sep 17 00:00:00 2001
From: rpm-build <rpm-build>
Date: Fri, 10 Mar 2023 11:46:16 +0800
Subject: [PATCH 2/2] add unit test for dbus-broker
---
test/dbus/meson.build | 6 ++
test/dbus/test-reexecute.c | 121 +++++++++++++++++++++++++++++++
test/dbus/test-serialize.c | 144 +++++++++++++++++++++++++++++++++++++
test/dbus/util-broker.c | 138 ++++++++++++++++++++++++++++++-----
test/dbus/util-broker.h | 12 +++-
5 files changed, 401 insertions(+), 20 deletions(-)
create mode 100755 test/dbus/test-reexecute.c
create mode 100755 test/dbus/test-serialize.c
diff --git a/test/dbus/meson.build b/test/dbus/meson.build
index c111283..c2d1d79 100644
--- a/test/dbus/meson.build
+++ b/test/dbus/meson.build
@@ -67,6 +67,12 @@ test('Client Lifetime', test_lifetime)
test_matches = executable('test-matches', ['test-matches.c'], dependencies: [ dep_test ])
test('Signals and Matches', test_matches)
+test_serialize = executable('test-serialize', ['test-serialize.c'], dependencies: [ dep_test ])
+test('Serialize and Deserialize', test_serialize)
+
+test_reexecute = executable('test-reexecute', ['test-reexecute.c'], dependencies: [ dep_test ])
+test('Reexecute', test_reexecute)
+
if use_reference_test
dbus_bin = dep_dbus.get_pkgconfig_variable('bindir') + '/dbus-daemon'
diff --git a/test/dbus/test-reexecute.c b/test/dbus/test-reexecute.c
new file mode 100755
index 0000000..43025e7
--- /dev/null
+++ b/test/dbus/test-reexecute.c
@@ -0,0 +1,121 @@
+/*
+ * Reexecute Tests
+ */
+
+#undef NDEBUG
+#include <c-stdaux.h>
+#include <systemd/sd-bus.h>
+#include <unistd.h>
+#include "util-broker.h"
+#include "util/string.h"
+#include "syslog.h"
+
+#define PATH_LENGTH_MAX 4096
+#define TEST_ARG_MAX 12
+
+static void test_send_reexecute() {
+ Broker *broker = NULL;
+ int r, sync_pairs[2], sync_value;
+
+ r = socketpair(AF_UNIX, SOCK_STREAM, 0, sync_pairs);
+ assert(r >= 0);
+
+ util_broker_new(&broker);
+ broker->test_reexec = true;
+ util_broker_spawn(broker);
+ pid_t pid = fork();
+ if (pid != 0) {
+ close(sync_pairs[1]);
+ /* Wait the child process exits. */
+ read(sync_pairs[0], &sync_value, sizeof(sync_value));
+ util_broker_terminate(broker);
+ util_broker_free(broker);
+ return;
+ }
+
+ close(sync_pairs[0]);
+ _c_cleanup_(sd_bus_flush_close_unrefp) sd_bus *server = NULL, *cmd_bus = NULL;
+ _c_cleanup_(sd_bus_message_unrefp) sd_bus_message *reply = NULL;
+ const char *unique_name = NULL, *owner = NULL, *reply_str = NULL;
+
+ /* RequestName before reexecuting. */
+ {
+ util_broker_connect(broker, &server);
+ r = sd_bus_get_unique_name(server, &unique_name);
+ c_assert(r >= 0);
+ r = sd_bus_call_method(server, "org.freedesktop.DBus", "/org/freedesktop/DBus", "org.freedesktop.DBus",
+ "RequestName", NULL, NULL, "su", "com.example.foo", 0);
+ c_assert(r >= 0);
+ }
+
+ /* Make broker reexecute. */
+ {
+ util_broker_connect(broker, &cmd_bus);
+ r = sd_bus_call_method(cmd_bus, "org.freedesktop.DBus", "/org/freedesktop/DBus", "org.freedesktop.DBus",
+ "Reexecute", NULL, &reply, "");
+ c_assert(r >= 0);
+
+ r = sd_bus_message_read(reply, "s", &reply_str);
+ c_assert(r >= 0);
+ c_assert(!strcmp(reply_str, "OK"));
+
+ sd_bus_flush_close_unref(cmd_bus);
+ }
+
+ /* Wait to make sure dbus-broker has exited. */
+ sleep(1);
+
+ /* GetNameOwner after reexecuting. */
+ {
+ r = sd_bus_call_method(server, "org.freedesktop.DBus", "/org/freedesktop/DBus", "org.freedesktop.DBus",
+ "GetNameOwner", NULL, &reply, "s", "com.example.foo");
+ c_assert(r >= 0);
+
+ r = sd_bus_message_read(reply, "s", &owner);
+ c_assert(r >= 0);
+ c_assert(!strcmp(owner, unique_name));
+ syslog(LOG_INFO, "Got the right owner!");
+ }
+
+ /* Clean.
+ * We can't call util_broker_terminate, because the forked process
+ * is single thread, util_broker_thread doesn't exist and can't do
+ * this for us.
+ */
+
+ broker->pipe_fds[0] = c_close(broker->pipe_fds[0]);
+ broker->pipe_fds[1] = c_close(broker->pipe_fds[1]);
+ broker->listener_fd = c_close(broker->listener_fd);
+ util_broker_free(broker);
+ write(sync_pairs[1], &sync_value, sizeof(sync_value));
+}
+
+static void test_generate_args_string() {
+ char *args[TEST_ARG_MAX];
+ for (int i = 0; i < TEST_ARG_MAX; i++) {
+ args[i] = NULL;
+ }
+ int i = 0;
+ generate_args_string(false, args, TEST_ARG_MAX, &i, "--log", "1");
+ c_assert(i == 0 && args[i] == NULL);
+ generate_args_string(true, args, TEST_ARG_MAX, &i, "--controller", "2");
+ c_assert(i == 2 && !strcmp(args[0], "--controller") && !strcmp(args[1], "2"));
+ generate_args_string(true, args, TEST_ARG_MAX, &i, "--machine-id", "3");
+ c_assert(i == 4 && !strcmp(args[2], "--machine-id") && !strcmp(args[3], "3"));
+ generate_args_string(true, args, TEST_ARG_MAX, &i, "--max-bytes", "123456");
+ c_assert(i == 6 && !strcmp(args[4], "--max-bytes") && !strcmp(args[5], "123456"));
+ generate_args_string(true, args, TEST_ARG_MAX, &i, "--max-fds", "12");
+ c_assert(i == 8 && !strcmp(args[6], "--max-fds") && !strcmp(args[7], "12"));
+ generate_args_string(true, args, TEST_ARG_MAX, &i, "--max-matches", "12345abcde");
+ c_assert(i == 10 && !strcmp(args[8], "--max-matches") && !strcmp(args[9], "12345abcde"));
+ generate_args_string(true, args, TEST_ARG_MAX, &i, "--reexec", "13");
+ c_assert(i == 10 && args[10] == NULL && args[11] == NULL);
+}
+
+int main(int argc, char **argv) {
+ test_generate_args_string();
+ /* Reexecute can only be run under privileged user */
+ if (getuid() == 0)
+ test_send_reexecute();
+ return 0;
+}
diff --git a/test/dbus/test-serialize.c b/test/dbus/test-serialize.c
new file mode 100755
index 0000000..e287647
--- /dev/null
+++ b/test/dbus/test-serialize.c
@@ -0,0 +1,144 @@
+/*
+ * Serialize and Deserialize Tests
+ */
+
+#undef NDEBUG
+#include <c-stdaux.h>
+#include <stdlib.h>
+#include "util/serialize.h"
+#include "util/string.h"
+
+static void test_basic() {
+ FILE *f = NULL;
+ _c_cleanup_(c_freep) char *buf = malloc(LINE_LENGTH_MAX);
+ int mem_fd;
+
+ mem_fd = state_file_init(&f);
+ c_assert(mem_fd > 0);
+
+ c_assert(!serialize_basic(f, "test_u", "%u", 1234));
+ c_assert(!serialize_basic(f, "test_d", "%d", -1234));
+ c_assert(!serialize_basic(f, "test_s", "%s", "test"));
+ c_assert(!serialize_basic(f, "test_uds", "%u;%d;%s", 1234, -1234, "test"));
+
+ fseeko(f, 0, SEEK_SET);
+
+ if (fgets(buf, LINE_LENGTH_MAX, f) != NULL)
+ c_assert(!strcmp(buf, "test_u=1234\n"));
+ if (fgets(buf, LINE_LENGTH_MAX, f) != NULL)
+ c_assert(!strcmp(buf, "test_d=-1234\n"));
+ if (fgets(buf, LINE_LENGTH_MAX, f) != NULL)
+ c_assert(!strcmp(buf, "test_s=test\n"));
+ if (fgets(buf, LINE_LENGTH_MAX, f) != NULL)
+ c_assert(!strcmp(buf, "test_uds=1234;-1234;test\n"));
+
+ c_assert(!fgets(buf, LINE_LENGTH_MAX, f));
+}
+
+static void test_extract_world_inlist() {
+ char *list1 = ";;a;b;;c;;;";
+ char *list2 = "a;;bb;ccc;;;";
+ char *res1[] = {"a", "b", "c"};
+ char *res2[] = {"a", "bb", "ccc"};
+ char *res = malloc(10);
+
+ int i = 0;
+ while (true) {
+ list1 = extract_word_inlist(list1, &res, 1);
+ if (!list1)
+ break;
+ c_assert(!strcmp(res, res1[i++]));
+ }
+
+ i = 0;
+ while (true) {
+ list2 = extract_word_inlist(list2, &res, 3);
+ if (!list2)
+ break;
+ c_assert(!strcmp(res, res2[i++]));
+ }
+ free(res);
+}
+
+static void test_extract_list_element() {
+ char *list1 = "[{}]";
+ char *list2 = "[{a}{b}{c}]";
+ char *list3 = "[{a}{{{b}}{}}{c}]";
+ char *res2_3[] = {"a", "b", "c"};
+ char *res = NULL;
+
+ int i = 0;
+ while (true) {
+ list1 = extract_list_element(list1, &res);
+ if (!list1)
+ break;
+ c_assert(!res);
+ free(res);
+ res = NULL;
+ }
+
+ if (res) {
+ free(res);
+ res = NULL;
+ }
+
+ i = 0;
+ while (true) {
+ list2 = extract_list_element(list2, &res);
+ if (!list2)
+ break;
+ c_assert(!strcmp(res, res2_3[i++]));
+ free(res);
+ res = NULL;
+ }
+ if (res) {
+ free(res);
+ res = NULL;
+ }
+
+ i = 0;
+ while (true) {
+ list3 = extract_list_element(list3, &res);
+ if (!list3)
+ break;
+ c_assert(!strcmp(res, res2_3[i++]));
+ free(res);
+ res = NULL;
+ }
+ if (res) {
+ free(res);
+ res = NULL;
+ }
+ return;
+}
+
+static void test_serialize_validate_rule() {
+ const char *rule_str_ok[3] = {"[{123}{234}{.a-*(*&^@#*^)}]", "[{123}]", "[]"};
+ const char *rule_str_bad[8] = {"oaeu", "[{{]", "[{123}}", "{123}{345}", "[{{11111}}]", "[aoeuaou]", "[{{}}]", "[aa{b}aa{b}]"};
+
+ for (int i = 0; i < 3; i++)
+ c_assert(serialize_validate_rule(rule_str_ok[i]) == true);
+
+ for (int i = 0; i < 8; i++)
+ c_assert(serialize_validate_rule(rule_str_bad[i]) == false);
+}
+
+static void test_serialize_validate_sasl() {
+ const char *sasl_str_ok[2] = {"[{0}{0}{1}]", "[{1}{1}{3}"};
+ const char *sasl_str_bad[6] = {"[{-1}{}{}]", "foo", "[{-1}{0}{-1}]", "[{5}{1}{0}]", "[123", "123323"};
+
+ for (int i = 0; i < 2; i++)
+ c_assert(serialize_validate_sasl(sasl_str_ok[i]) == true);
+
+ for (int i = 0; i < 6; i++)
+ c_assert(serialize_validate_sasl(sasl_str_bad[i]) == false);
+}
+
+int main(int argc, char **argv) {
+ test_basic();
+ test_extract_world_inlist();
+ test_extract_list_element();
+ test_serialize_validate_rule();
+ test_serialize_validate_sasl();
+ return 0;
+}
diff --git a/test/dbus/util-broker.c b/test/dbus/util-broker.c
index 0bdcefc..5d33b52 100644
--- a/test/dbus/util-broker.c
+++ b/test/dbus/util-broker.c
@@ -15,6 +15,7 @@
#include <systemd/sd-bus.h>
#include <systemd/sd-event.h>
#include "dbus/protocol.h"
+#include "util/proc.h"
#include "util/syscall.h"
#include "util-broker.h"
@@ -35,7 +36,8 @@ void util_event_new(sd_event **eventp) {
}
static int util_event_sigchld(sd_event_source *source, const siginfo_t *si, void *userdata) {
- int status;
+ int status, r;
+ Broker* broker = userdata;
if (si->si_code == CLD_EXITED)
status = si->si_status;
@@ -47,6 +49,70 @@ static int util_event_sigchld(sd_event_source *source, const siginfo_t *si, void
return sd_event_exit(sd_event_source_get_event(source), status);
}
+static int util_event_sigchld_reexec(sd_event_source *source, const struct signalfd_siginfo *ssi, void *userdata) {
+ int status, r;
+ Broker* broker = userdata;
+
+ if (ssi->ssi_pid == broker->child_pid)
+ return sd_event_source_set_enabled(broker->defer_event_source, SD_EVENT_ONESHOT);
+}
+
+static int util_event_defer(sd_event_source *source, void *userdata) {
+ Broker *broker = userdata;
+ sd_bus *bus = broker->bus;
+ /* new_bus shares the dbus fd (pair[0] or lc_fd) with broker->bus.
+ * Both of them use this fd to communicate with broker, which use
+ * pair[1]. So we can't close new_bus here, or we will fail when
+ * closing broker->bus.
+ * And, before sending messages to the broker by new_bus, we should
+ * detach broker->bus. If not, the reply message will be processed
+ * by broker->bus, but broker->bus have no idea why this message even
+ * exist, because it doesn't send any message.
+ */
+ sd_bus_detach_event(bus);
+ sd_bus *new_bus;
+ _c_cleanup_(sd_bus_message_unrefp) sd_bus_message *message = NULL;
+ int r = 0;
+
+ create_broker_listener(broker);
+
+ r = sd_bus_new(&new_bus);
+ assert(r >= 0);
+
+ r = sd_bus_set_fd(new_bus, broker->lc_fd, broker->lc_fd);
+ c_assert(r >= 0);
+
+ r = sd_bus_start(new_bus);
+ assert(r >= 0);
+
+ r = sd_bus_message_new_method_call(new_bus,
+ &message,
+ NULL,
+ "/org/bus1/DBus/Broker",
+ "org.bus1.DBus.Broker",
+ "AddListener");
+ c_assert(r >= 0);
+
+ r = sd_bus_message_append(message,
+ "oh",
+ "/org/bus1/DBus/Listener/0",
+ broker->listener_fd);
+ c_assert(r >= 0);
+
+ r = util_append_policy(message);
+ c_assert(r >= 0);
+
+ r = sd_bus_call(new_bus, message, -1, NULL, NULL);
+ c_assert(r >= 0);
+
+ /* close the new listener_fd just like what we have done in util_broker_thread. */
+ broker->listener_fd = c_close(broker->listener_fd);
+
+ /* We still need broker->bus to deal some message, reattach it. */
+ sd_bus_attach_event(bus, sd_event_source_get_event(source), SD_EVENT_PRIORITY_NORMAL);
+ return 0;
+}
+
#define POLICY_T_BATCH \
"bt" \
"a(btbs)" \
@@ -59,7 +125,7 @@ static int util_event_sigchld(sd_event_source *source, const siginfo_t *si, void
"a(ss)" \
"b"
-static int util_append_policy(sd_bus_message *m) {
+int util_append_policy(sd_bus_message *m) {
int r;
r = sd_bus_message_open_container(m, 'v', "(" POLICY_T ")");
@@ -165,18 +231,49 @@ static int util_method_reload_config(sd_bus_message *message, void *userdata, sd
return sd_bus_reply_method_return(message, NULL);
}
+static int util_method_reexecute(sd_bus_message *message, void *userdata, sd_bus_error *error) {
+ /* Careful: Don't return 0, or we will get ECONNRESET in libsystemd. */
+ /* backtrace:
+ bus_process_object
+ ↳ object_find_and_run
+ ↳ method_callbacks_run
+ ↳ r = c->vtable->x.method.handler(m, u, &error);
+ */
+ return sd_bus_reply_method_return(message, NULL);
+}
+
const sd_bus_vtable util_vtable[] = {
SD_BUS_VTABLE_START(0),
SD_BUS_METHOD("ReloadConfig", NULL, NULL, util_method_reload_config, 0),
+ SD_BUS_METHOD("Reexecute", NULL, NULL, util_method_reexecute, 0),
SD_BUS_VTABLE_END
};
-void util_fork_broker(sd_bus **busp, sd_event *event, int listener_fd, pid_t *pidp) {
- _c_cleanup_(sd_bus_flush_close_unrefp) sd_bus *bus = NULL;
+int create_broker_listener(Broker *broker) {
+ int r;
+
+ broker->listener_fd = socket(AF_UNIX, SOCK_STREAM | SOCK_NONBLOCK | SOCK_CLOEXEC, 0);
+ c_assert(broker->listener_fd >= 0);
+
+ r = bind(broker->listener_fd, (struct sockaddr *)&broker->address, offsetof(struct sockaddr_un, sun_path));
+ c_assert(r >= 0);
+
+ r = getsockname(broker->listener_fd, (struct sockaddr *)&broker->address, &broker->n_address);
+ c_assert(r >= 0);
+
+ r = listen(broker->listener_fd, 256);
+ c_assert(r >= 0);
+
+ return 0;
+}
+
+void util_fork_broker(sd_bus **busp, sd_event *event, int listener_fd, pid_t *pidp, int *lc_fd, Broker* broker) {
+ sd_bus *bus = NULL;
_c_cleanup_(sd_bus_message_unrefp) sd_bus_message *message = NULL;
_c_cleanup_(c_freep) char *fdstr = NULL;
+ sd_event_source *defer_event_source = NULL;
int r, pair[2];
pid_t pid;
@@ -214,7 +311,13 @@ void util_fork_broker(sd_bus **busp, sd_event *event, int listener_fd, pid_t *pi
if (pidp)
*pidp = pid;
- r = sd_event_add_child(event, NULL, pid, WEXITED, util_event_sigchld, NULL);
+ r = sd_event_add_child(event, NULL, pid, WCONTINUED | WEXITED, util_event_sigchld, broker);
+ c_assert(r >= 0);
+
+ r = sd_event_add_defer(event, &defer_event_source, util_event_defer, broker);
+ c_assert(r >= 0);
+ broker->defer_event_source = defer_event_source;
+ r = sd_event_source_set_enabled(defer_event_source, SD_EVENT_OFF);
c_assert(r >= 0);
r = sd_bus_new(&bus);
@@ -223,11 +326,12 @@ void util_fork_broker(sd_bus **busp, sd_event *event, int listener_fd, pid_t *pi
/* consumes the fd */
r = sd_bus_set_fd(bus, pair[0], pair[0]);
c_assert(r >= 0);
+ *lc_fd = pair[0];
r = sd_bus_attach_event(bus, event, SD_EVENT_PRIORITY_NORMAL);
c_assert(r >= 0);
- r = sd_bus_add_object_vtable(bus, NULL, "/org/bus1/DBus/Controller", "org.bus1.DBus.Controller", util_vtable, NULL);
+ r = sd_bus_add_object_vtable(bus, NULL, "/org/bus1/DBus/Controller", "org.bus1.DBus.Controller", util_vtable, broker);
c_assert(r >= 0);
r = sd_bus_start(bus);
@@ -381,10 +485,16 @@ static void *util_broker_thread(void *userdata) {
r = sd_event_add_signal(event, NULL, SIGUSR1, util_event_sigusr1, broker);
c_assert(r >= 0);
+ if (broker->test_reexec) {
+ r = sd_event_add_signal(event, NULL, SIGCHLD, util_event_sigchld_reexec, broker);
+ c_assert(r >= 0);
+ }
+
if (broker->listener_fd >= 0) {
- util_fork_broker(&bus, event, broker->listener_fd, &broker->child_pid);
+ util_fork_broker(&bus, event, broker->listener_fd, &broker->child_pid, &broker->lc_fd, broker);
/* dbus-broker reports its controller in GetConnectionUnixProcessID */
broker->pid = getpid();
+ broker->bus = bus;
broker->listener_fd = c_close(broker->listener_fd);
} else {
c_assert(broker->listener_fd < 0);
@@ -478,18 +588,7 @@ void util_broker_spawn(Broker *broker) {
* run and babysit the broker.
*/
- broker->listener_fd = socket(AF_UNIX, SOCK_STREAM | SOCK_NONBLOCK | SOCK_CLOEXEC, 0);
- c_assert(broker->listener_fd >= 0);
-
- r = bind(broker->listener_fd, (struct sockaddr *)&broker->address, offsetof(struct sockaddr_un, sun_path));
- c_assert(r >= 0);
-
- r = getsockname(broker->listener_fd, (struct sockaddr *)&broker->address, &broker->n_address);
- c_assert(r >= 0);
-
- r = listen(broker->listener_fd, 256);
- c_assert(r >= 0);
-
+ create_broker_listener(broker);
r = pthread_create(&broker->thread, NULL, util_broker_thread, broker);
c_assert(r >= 0);
}
@@ -512,6 +611,7 @@ void util_broker_terminate(Broker *broker) {
r = pthread_join(broker->thread, &value);
c_assert(!r);
+
c_assert(!value);
c_assert(broker->listener_fd < 0);
diff --git a/test/dbus/util-broker.h b/test/dbus/util-broker.h
index a3f5f49..fa6de9d 100644
--- a/test/dbus/util-broker.h
+++ b/test/dbus/util-broker.h
@@ -20,6 +20,10 @@ struct Broker {
pthread_t thread;
struct sockaddr_un address;
socklen_t n_address;
+ sd_bus *bus;
+ sd_event_source *defer_event_source;
+ bool test_reexec;
+ int lc_fd; /* launcher controller fd */
int listener_fd;
int pipe_fds[2];
pid_t pid;
@@ -29,6 +33,10 @@ struct Broker {
#define BROKER_NULL { \
.address.sun_family = AF_UNIX, \
.n_address = sizeof(struct sockaddr_un), \
+ .bus = NULL, \
+ .defer_event_source = NULL, \
+ .test_reexec = false, \
+ .lc_fd = -1, \
.listener_fd = -1, \
.pipe_fds[0] = -1, \
.pipe_fds[1] = -1, \
@@ -37,7 +45,9 @@ struct Broker {
/* misc */
void util_event_new(sd_event **eventp);
-void util_fork_broker(sd_bus **busp, sd_event *event, int listener_fd, pid_t *pidp);
+int util_append_policy(sd_bus_message *m);
+int create_broker_listener(Broker *broker);
+void util_fork_broker(sd_bus **busp, sd_event *event, int listener_fd, pid_t *pidp, int *lc_fd, Broker *broker);
void util_fork_daemon(sd_event *event, int pipe_fd, pid_t *pidp);
/* broker */
--
2.30.2
Reference in New Issue View Git Blame Copy Permalink