From ab33edb05b7d4c90fb80f46aa6b951c505048798 Mon Sep 17 00:00:00 2001 From: Mike Yuan Date: Sat, 11 Jan 2025 16:52:05 +0100 Subject: [PATCH 01/11] shared/bus-util: move bus_message_read_id128() to bus-message-util --- src/core/dbus-manager.c | 1 + src/hostname/hostnamectl.c | 1 + src/machine/machinectl.c | 1 + src/machine/machined-dbus.c | 1 + src/run/run.c | 1 + src/shared/bus-map-properties.c | 1 + src/shared/bus-message-util.c | 29 +++++++++++++++++++++++++++++ src/shared/bus-message-util.h | 2 ++ src/shared/bus-util.c | 28 ---------------------------- src/shared/bus-util.h | 2 -- 10 files changed, 37 insertions(+), 30 deletions(-) diff --git a/src/core/dbus-manager.c b/src/core/dbus-manager.c index 99e3ea12ac..4b61002eac 100644 --- a/src/core/dbus-manager.c +++ b/src/core/dbus-manager.c @@ -12,6 +12,7 @@ #include "bus-common-errors.h" #include "bus-get-properties.h" #include "bus-log-control-api.h" +#include "bus-message-util.h" #include "bus-util.h" #include "chase.h" #include "confidential-virt.h" diff --git a/src/hostname/hostnamectl.c b/src/hostname/hostnamectl.c index cd4f86af80..2d1334f654 100644 --- a/src/hostname/hostnamectl.c +++ b/src/hostname/hostnamectl.c @@ -18,6 +18,7 @@ #include "bus-error.h" #include "bus-locator.h" #include "bus-map-properties.h" +#include "bus-message-util.h" #include "format-table.h" #include "hostname-setup.h" #include "hostname-util.h" diff --git a/src/machine/machinectl.c b/src/machine/machinectl.c index 39d3f3aed8..91bef1a5e9 100644 --- a/src/machine/machinectl.c +++ b/src/machine/machinectl.c @@ -21,6 +21,7 @@ #include "bus-error.h" #include "bus-locator.h" #include "bus-map-properties.h" +#include "bus-message-util.h" #include "bus-print-properties.h" #include "bus-unit-procs.h" #include "bus-unit-util.h" diff --git a/src/machine/machined-dbus.c b/src/machine/machined-dbus.c index fc50d3f147..e50ed7e0df 100644 --- a/src/machine/machined-dbus.c +++ b/src/machine/machined-dbus.c @@ -10,6 +10,7 @@ #include "bus-common-errors.h" #include "bus-get-properties.h" #include "bus-locator.h" +#include "bus-message-util.h" #include "bus-polkit.h" #include "cgroup-util.h" #include "discover-image.h" diff --git a/src/run/run.c b/src/run/run.c index 28953a6514..4a7a5d939c 100644 --- a/src/run/run.c +++ b/src/run/run.c @@ -15,6 +15,7 @@ #include "bus-error.h" #include "bus-locator.h" #include "bus-map-properties.h" +#include "bus-message-util.h" #include "bus-unit-util.h" #include "bus-wait-for-jobs.h" #include "calendarspec.h" diff --git a/src/shared/bus-map-properties.c b/src/shared/bus-map-properties.c index a6cd752894..18d83c336f 100644 --- a/src/shared/bus-map-properties.c +++ b/src/shared/bus-map-properties.c @@ -5,6 +5,7 @@ #include "bus-util.h" #include "strv.h" #include "bus-message.h" +#include "bus-message-util.h" int bus_map_id128(sd_bus *bus, const char *member, sd_bus_message *m, sd_bus_error *error, void *userdata) { sd_id128_t *p = userdata; diff --git a/src/shared/bus-message-util.c b/src/shared/bus-message-util.c index d8c483ef62..e93be9b3c5 100644 --- a/src/shared/bus-message-util.c +++ b/src/shared/bus-message-util.c @@ -7,6 +7,35 @@ #include "copy.h" #include "resolve-util.h" +int bus_message_read_id128(sd_bus_message *m, sd_id128_t *ret) { + const void *a; + size_t sz; + int r; + + assert(m); + + r = sd_bus_message_read_array(m, 'y', &a, &sz); + if (r < 0) + return r; + + switch (sz) { + + case 0: + if (ret) + *ret = SD_ID128_NULL; + return 0; + + case sizeof(sd_id128_t): + if (ret) + memcpy(ret, a, sz); + return !memeqzero(a, sz); /* This mimics sd_id128_is_null(), but ret may be NULL, + * and a may be misaligned, so use memeqzero() here. */ + + default: + return -EINVAL; + } +} + int bus_message_read_ifindex(sd_bus_message *message, sd_bus_error *error, int *ret) { int ifindex, r; diff --git a/src/shared/bus-message-util.h b/src/shared/bus-message-util.h index 50025766c2..698960561c 100644 --- a/src/shared/bus-message-util.h +++ b/src/shared/bus-message-util.h @@ -6,6 +6,8 @@ #include "in-addr-util.h" #include "socket-netlink.h" +int bus_message_read_id128(sd_bus_message *m, sd_id128_t *ret); + int bus_message_read_ifindex(sd_bus_message *message, sd_bus_error *error, int *ret); int bus_message_read_family(sd_bus_message *message, sd_bus_error *error, int *ret); int bus_message_read_in_addr_auto(sd_bus_message *message, sd_bus_error *error, int *ret_family, union in_addr_union *ret_addr); diff --git a/src/shared/bus-util.c b/src/shared/bus-util.c index e38433ddc0..d6189deb18 100644 --- a/src/shared/bus-util.c +++ b/src/shared/bus-util.c @@ -953,34 +953,6 @@ int bus_query_sender_pidref( return bus_creds_get_pidref(creds, ret); } -int bus_message_read_id128(sd_bus_message *m, sd_id128_t *ret) { - const void *a; - size_t sz; - int r; - - assert(m); - - r = sd_bus_message_read_array(m, 'y', &a, &sz); - if (r < 0) - return r; - - switch (sz) { - case 0: - if (ret) - *ret = SD_ID128_NULL; - return 0; - - case sizeof(sd_id128_t): - if (ret) - memcpy(ret, a, sz); - return !memeqzero(a, sz); /* This mimics sd_id128_is_null(), but ret may be NULL, - * and a may be misaligned, so use memeqzero() here. */ - - default: - return -EINVAL; - } -} - static const char* const bus_transport_table[] = { [BUS_TRANSPORT_LOCAL] = "local", [BUS_TRANSPORT_REMOTE] = "remote", diff --git a/src/shared/bus-util.h b/src/shared/bus-util.h index 95b9adb7b1..b08635fd36 100644 --- a/src/shared/bus-util.h +++ b/src/shared/bus-util.h @@ -87,6 +87,4 @@ int bus_property_get_string_set(sd_bus *bus, const char *path, const char *inter int bus_creds_get_pidref(sd_bus_creds *c, PidRef *ret); int bus_query_sender_pidref(sd_bus_message *m, PidRef *ret); -int bus_message_read_id128(sd_bus_message *m, sd_id128_t *ret); - const char* bus_transport_to_string(BusTransport transport) _const_; From e3d37628aabff92e4b756e63ef0a6cd4569ce743 Mon Sep 17 00:00:00 2001 From: Mike Yuan Date: Sat, 11 Jan 2025 17:10:43 +0100 Subject: [PATCH 02/11] shared/bus-util: move bus_message_hash_ops to bus-message-util --- src/home/homed-manager-bus.c | 9 +++++---- src/login/logind-brightness.c | 4 ++-- src/shared/bus-message-util.c | 4 ++++ src/shared/bus-message-util.h | 2 ++ src/shared/bus-util.c | 10 ---------- src/shared/bus-util.h | 2 -- src/systemctl/systemctl-list-units.c | 1 + src/timedate/timedated.c | 1 + 8 files changed, 15 insertions(+), 18 deletions(-) diff --git a/src/home/homed-manager-bus.c b/src/home/homed-manager-bus.c index 69c7680b9e..08c917aee2 100644 --- a/src/home/homed-manager-bus.c +++ b/src/home/homed-manager-bus.c @@ -4,6 +4,7 @@ #include "alloc-util.h" #include "bus-common-errors.h" +#include "bus-message-util.h" #include "bus-polkit.h" #include "format-util.h" #include "home-util.h" @@ -704,17 +705,17 @@ static int method_rebalance(sd_bus_message *message, void *userdata, sd_bus_erro int r; r = manager_schedule_rebalance(m, /* immediately= */ true); - if (r == 0) - return sd_bus_reply_method_errorf(message, BUS_ERROR_REBALANCE_NOT_NEEDED, "No home directories need rebalancing."); if (r < 0) return r; + if (r == 0) + return sd_bus_reply_method_errorf(message, BUS_ERROR_REBALANCE_NOT_NEEDED, "No home directories need rebalancing."); /* Keep a reference to this message, so that we can reply to it once we are done */ - r = set_ensure_put(&m->rebalance_queued_method_calls, &bus_message_hash_ops, message); + r = set_ensure_consume(&m->rebalance_queued_method_calls, &bus_message_hash_ops, sd_bus_message_ref(message)); if (r < 0) return log_error_errno(r, "Failed to track rebalance bus message: %m"); + assert(r > 0); - sd_bus_message_ref(message); return 1; } diff --git a/src/login/logind-brightness.c b/src/login/logind-brightness.c index 40bcb39ce0..b3e7718394 100644 --- a/src/login/logind-brightness.c +++ b/src/login/logind-brightness.c @@ -1,5 +1,6 @@ /* SPDX-License-Identifier: LGPL-2.1-or-later */ +#include "bus-message-util.h" #include "bus-util.h" #include "device-util.h" #include "hash-funcs.h" @@ -173,10 +174,9 @@ static int set_add_message(Set **set, sd_bus_message *message) { if (r <= 0) return r; - r = set_ensure_put(set, &bus_message_hash_ops, message); + r = set_ensure_consume(set, &bus_message_hash_ops, sd_bus_message_ref(message)); if (r <= 0) return r; - sd_bus_message_ref(message); return 1; } diff --git a/src/shared/bus-message-util.c b/src/shared/bus-message-util.c index e93be9b3c5..a6523ff00e 100644 --- a/src/shared/bus-message-util.c +++ b/src/shared/bus-message-util.c @@ -246,3 +246,7 @@ int bus_message_dump_fd(sd_bus_message *message) { return 0; } + +DEFINE_HASH_OPS_WITH_VALUE_DESTRUCTOR(bus_message_hash_ops, + void, trivial_hash_func, trivial_compare_func, + sd_bus_message, sd_bus_message_unref); diff --git a/src/shared/bus-message-util.h b/src/shared/bus-message-util.h index 698960561c..baec1cb92b 100644 --- a/src/shared/bus-message-util.h +++ b/src/shared/bus-message-util.h @@ -21,3 +21,5 @@ int bus_message_read_dns_servers( int bus_message_dump_string(sd_bus_message *message); int bus_message_dump_fd(sd_bus_message *message); + +extern const struct hash_ops bus_message_hash_ops; diff --git a/src/shared/bus-util.c b/src/shared/bus-util.c index d6189deb18..9e875751e7 100644 --- a/src/shared/bus-util.c +++ b/src/shared/bus-util.c @@ -862,16 +862,6 @@ int bus_register_malloc_status(sd_bus *bus, const char *destination) { return 0; } -static void bus_message_unref_wrapper(void *m) { - sd_bus_message_unref(m); -} - -const struct hash_ops bus_message_hash_ops = { - .hash = trivial_hash_func, - .compare = trivial_compare_func, - .free_value = bus_message_unref_wrapper, -}; - int bus_message_append_string_set(sd_bus_message *m, Set *set) { const char *s; int r; diff --git a/src/shared/bus-util.h b/src/shared/bus-util.h index b08635fd36..755421c11f 100644 --- a/src/shared/bus-util.h +++ b/src/shared/bus-util.h @@ -78,8 +78,6 @@ int bus_reply_pair_array(sd_bus_message *m, char **l); /* Listen to GetMallocInfo() calls to 'destination' and return malloc_info() via FD */ int bus_register_malloc_status(sd_bus *bus, const char *destination); -extern const struct hash_ops bus_message_hash_ops; - int bus_message_append_string_set(sd_bus_message *m, Set *s); int bus_property_get_string_set(sd_bus *bus, const char *path, const char *interface, const char *property, sd_bus_message *reply, void *userdata, sd_bus_error *error); diff --git a/src/systemctl/systemctl-list-units.c b/src/systemctl/systemctl-list-units.c index a2f3074358..b7cb103513 100644 --- a/src/systemctl/systemctl-list-units.c +++ b/src/systemctl/systemctl-list-units.c @@ -5,6 +5,7 @@ #include "ansi-color.h" #include "bus-error.h" #include "bus-locator.h" +#include "bus-message-util.h" #include "format-table.h" #include "locale-util.h" #include "path-util.h" diff --git a/src/timedate/timedated.c b/src/timedate/timedated.c index c79bb864df..b196034a25 100644 --- a/src/timedate/timedated.c +++ b/src/timedate/timedated.c @@ -17,6 +17,7 @@ #include "bus-locator.h" #include "bus-log-control-api.h" #include "bus-map-properties.h" +#include "bus-message-util.h" #include "bus-polkit.h" #include "bus-unit-util.h" #include "clock-util.h" From 91080bc9733b5b2478bfc0ed58f6a7ae5da7e639 Mon Sep 17 00:00:00 2001 From: Mike Yuan Date: Sat, 11 Jan 2025 18:04:37 +0100 Subject: [PATCH 03/11] shared/bus-util: move string set append/get funcs to bus-message-util and bus-get-properties, respectively --- src/core/dbus-cgroup.c | 1 + src/shared/bus-get-properties.c | 19 +++++++++++++++++ src/shared/bus-get-properties.h | 2 ++ src/shared/bus-message-util.c | 19 +++++++++++++++++ src/shared/bus-message-util.h | 2 ++ src/shared/bus-util.c | 37 --------------------------------- src/shared/bus-util.h | 4 ---- 7 files changed, 43 insertions(+), 41 deletions(-) diff --git a/src/core/dbus-cgroup.c b/src/core/dbus-cgroup.c index fd48f3b07c..c99f1e29ac 100644 --- a/src/core/dbus-cgroup.c +++ b/src/core/dbus-cgroup.c @@ -7,6 +7,7 @@ #include "bpf-firewall.h" #include "bpf-foreign.h" #include "bus-get-properties.h" +#include "bus-message-util.h" #include "bus-util.h" #include "cgroup-util.h" #include "cgroup.h" diff --git a/src/shared/bus-get-properties.c b/src/shared/bus-get-properties.c index 53e5d6b99f..bf267a23a5 100644 --- a/src/shared/bus-get-properties.c +++ b/src/shared/bus-get-properties.c @@ -1,6 +1,7 @@ /* SPDX-License-Identifier: LGPL-2.1-or-later */ #include "bus-get-properties.h" +#include "bus-message-util.h" #include "rlimit-util.h" #include "stdio-util.h" #include "string-util.h" @@ -164,3 +165,21 @@ int bus_property_get_rlimit( return sd_bus_message_append(reply, "t", u); } + +int bus_property_get_string_set( + sd_bus *bus, + const char *path, + const char *interface, + const char *property, + sd_bus_message *reply, + void *userdata, + sd_bus_error *error) { + + Set **s = ASSERT_PTR(userdata); + + assert(bus); + assert(property); + assert(reply); + + return bus_message_append_string_set(reply, *s); +} diff --git a/src/shared/bus-get-properties.h b/src/shared/bus-get-properties.h index 4c35126502..9ddf5454de 100644 --- a/src/shared/bus-get-properties.h +++ b/src/shared/bus-get-properties.h @@ -52,6 +52,8 @@ assert_cc(sizeof(mode_t) == sizeof(uint32_t)); int bus_property_get_rlimit(sd_bus *bus, const char *path, const char *interface, const char *property, sd_bus_message *reply, void *userdata, sd_bus_error *error); +int bus_property_get_string_set(sd_bus *bus, const char *path, const char *interface, const char *property, sd_bus_message *reply, void *userdata, sd_bus_error *error); + #define BUS_DEFINE_PROPERTY_GET_GLOBAL(function, bus_type, val) \ int function(sd_bus *bus, \ const char *path, \ diff --git a/src/shared/bus-message-util.c b/src/shared/bus-message-util.c index a6523ff00e..8da112cacc 100644 --- a/src/shared/bus-message-util.c +++ b/src/shared/bus-message-util.c @@ -216,6 +216,25 @@ clear: return r; } +int bus_message_append_string_set(sd_bus_message *m, const Set *set) { + int r; + + assert(m); + + r = sd_bus_message_open_container(m, 'a', "s"); + if (r < 0) + return r; + + const char *s; + SET_FOREACH(s, set) { + r = sd_bus_message_append(m, "s", s); + if (r < 0) + return r; + } + + return sd_bus_message_close_container(m); +} + int bus_message_dump_string(sd_bus_message *message) { const char *s; int r; diff --git a/src/shared/bus-message-util.h b/src/shared/bus-message-util.h index baec1cb92b..d4a05f5b9c 100644 --- a/src/shared/bus-message-util.h +++ b/src/shared/bus-message-util.h @@ -19,6 +19,8 @@ int bus_message_read_dns_servers( struct in_addr_full ***ret_dns, size_t *ret_n_dns); +int bus_message_append_string_set(sd_bus_message *m, const Set *s); + int bus_message_dump_string(sd_bus_message *message); int bus_message_dump_fd(sd_bus_message *message); diff --git a/src/shared/bus-util.c b/src/shared/bus-util.c index 9e875751e7..5c76346fc3 100644 --- a/src/shared/bus-util.c +++ b/src/shared/bus-util.c @@ -862,43 +862,6 @@ int bus_register_malloc_status(sd_bus *bus, const char *destination) { return 0; } -int bus_message_append_string_set(sd_bus_message *m, Set *set) { - const char *s; - int r; - - assert(m); - - r = sd_bus_message_open_container(m, 'a', "s"); - if (r < 0) - return r; - - SET_FOREACH(s, set) { - r = sd_bus_message_append(m, "s", s); - if (r < 0) - return r; - } - - return sd_bus_message_close_container(m); -} - -int bus_property_get_string_set( - sd_bus *bus, - const char *path, - const char *interface, - const char *property, - sd_bus_message *reply, - void *userdata, - sd_bus_error *error) { - - Set **s = ASSERT_PTR(userdata); - - assert(bus); - assert(property); - assert(reply); - - return bus_message_append_string_set(reply, *s); -} - int bus_creds_get_pidref( sd_bus_creds *c, PidRef *ret) { diff --git a/src/shared/bus-util.h b/src/shared/bus-util.h index 755421c11f..47c0711868 100644 --- a/src/shared/bus-util.h +++ b/src/shared/bus-util.h @@ -78,10 +78,6 @@ int bus_reply_pair_array(sd_bus_message *m, char **l); /* Listen to GetMallocInfo() calls to 'destination' and return malloc_info() via FD */ int bus_register_malloc_status(sd_bus *bus, const char *destination); -int bus_message_append_string_set(sd_bus_message *m, Set *s); - -int bus_property_get_string_set(sd_bus *bus, const char *path, const char *interface, const char *property, sd_bus_message *reply, void *userdata, sd_bus_error *error); - int bus_creds_get_pidref(sd_bus_creds *c, PidRef *ret); int bus_query_sender_pidref(sd_bus_message *m, PidRef *ret); From 3f03d39ca3b2f25f521342f2b0e49f60c51246e1 Mon Sep 17 00:00:00 2001 From: Mike Yuan Date: Mon, 13 Jan 2025 16:35:13 +0100 Subject: [PATCH 04/11] shared/serialize: make input params const --- src/shared/serialize.c | 9 ++++++--- src/shared/serialize.h | 4 ++-- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/src/shared/serialize.c b/src/shared/serialize.c index 4d9ee6cc32..67311e4fa5 100644 --- a/src/shared/serialize.c +++ b/src/shared/serialize.c @@ -164,11 +164,14 @@ int serialize_dual_timestamp(FILE *f, const char *name, const dual_timestamp *t) return serialize_item_format(f, name, USEC_FMT " " USEC_FMT, t->realtime, t->monotonic); } -int serialize_strv(FILE *f, const char *key, char **l) { +int serialize_strv(FILE *f, const char *key, char * const *l) { int ret = 0, r; /* Returns the first error, or positive if anything was serialized, 0 otherwise. */ + assert(f); + assert(key); + STRV_FOREACH(i, l) { r = serialize_item_escaped(f, key, *i); if ((ret >= 0 && r < 0) || @@ -267,8 +270,7 @@ int serialize_item_base64mem(FILE *f, const char *key, const void *p, size_t l) return 1; } -int serialize_string_set(FILE *f, const char *key, Set *s) { - const char *e; +int serialize_string_set(FILE *f, const char *key, const Set *s) { int r; assert(f); @@ -279,6 +281,7 @@ int serialize_string_set(FILE *f, const char *key, Set *s) { /* Serialize as individual items, as each element might contain separators and escapes */ + const char *e; SET_FOREACH(e, s) { r = serialize_item(f, key, e); if (r < 0) diff --git a/src/shared/serialize.h b/src/shared/serialize.h index dc753465c4..6f5fcf44fe 100644 --- a/src/shared/serialize.h +++ b/src/shared/serialize.h @@ -21,10 +21,10 @@ int serialize_fd(FILE *f, FDSet *fds, const char *key, int fd); int serialize_fd_many(FILE *f, FDSet *fds, const char *key, const int fd_array[], size_t n_fd_array); int serialize_usec(FILE *f, const char *key, usec_t usec); int serialize_dual_timestamp(FILE *f, const char *key, const dual_timestamp *t); -int serialize_strv(FILE *f, const char *key, char **l); +int serialize_strv(FILE *f, const char *key, char * const *l); int serialize_pidref(FILE *f, FDSet *fds, const char *key, PidRef *pidref); int serialize_ratelimit(FILE *f, const char *key, const RateLimit *rl); -int serialize_string_set(FILE *f, const char *key, Set *s); +int serialize_string_set(FILE *f, const char *key, const Set *s); int serialize_image_policy(FILE *f, const char *key, const ImagePolicy *p); static inline int serialize_bool(FILE *f, const char *key, bool b) { From 38a2c2bf6a89def24007c0dac529c07da713abfb Mon Sep 17 00:00:00 2001 From: Mike Yuan Date: Mon, 13 Jan 2025 16:35:58 +0100 Subject: [PATCH 05/11] shared/serialize: introduce serialize_id128() --- src/core/unit-serialize.c | 3 +-- src/shared/serialize.c | 10 ++++++++++ src/shared/serialize.h | 3 +++ 3 files changed, 14 insertions(+), 2 deletions(-) diff --git a/src/core/unit-serialize.c b/src/core/unit-serialize.c index 82789fdbf4..78ceb006a4 100644 --- a/src/core/unit-serialize.c +++ b/src/core/unit-serialize.c @@ -117,8 +117,7 @@ int unit_serialize_state(Unit *u, FILE *f, FDSet *fds, bool switching_root) { if (gid_is_valid(u->ref_gid)) (void) serialize_item_format(f, "ref-gid", GID_FMT, u->ref_gid); - if (!sd_id128_is_null(u->invocation_id)) - (void) serialize_item_format(f, "invocation-id", SD_ID128_FORMAT_STR, SD_ID128_FORMAT_VAL(u->invocation_id)); + (void) serialize_id128(f, "invocation-id", u->invocation_id); (void) serialize_item(f, "freezer-state", freezer_state_to_string(u->freezer_state)); diff --git a/src/shared/serialize.c b/src/shared/serialize.c index 67311e4fa5..425e7f1ac3 100644 --- a/src/shared/serialize.c +++ b/src/shared/serialize.c @@ -182,6 +182,16 @@ int serialize_strv(FILE *f, const char *key, char * const *l) { return ret; } +int serialize_id128(FILE *f, const char *key, sd_id128_t id) { + assert(f); + assert(key); + + if (sd_id128_is_null(id)) + return 0; + + return serialize_item_format(f, key, SD_ID128_FORMAT_STR, SD_ID128_FORMAT_VAL(id)); +} + int serialize_pidref(FILE *f, FDSet *fds, const char *key, PidRef *pidref) { int r; diff --git a/src/shared/serialize.h b/src/shared/serialize.h index 6f5fcf44fe..b573e6cc15 100644 --- a/src/shared/serialize.h +++ b/src/shared/serialize.h @@ -3,6 +3,8 @@ #include +#include "sd-id128.h" + #include "fdset.h" #include "image-policy.h" #include "macro.h" @@ -22,6 +24,7 @@ int serialize_fd_many(FILE *f, FDSet *fds, const char *key, const int fd_array[] int serialize_usec(FILE *f, const char *key, usec_t usec); int serialize_dual_timestamp(FILE *f, const char *key, const dual_timestamp *t); int serialize_strv(FILE *f, const char *key, char * const *l); +int serialize_id128(FILE *f, const char *key, sd_id128_t id); int serialize_pidref(FILE *f, FDSet *fds, const char *key, PidRef *pidref); int serialize_ratelimit(FILE *f, const char *key, const RateLimit *rl); int serialize_string_set(FILE *f, const char *key, const Set *s); From 33eeea4128f31df7ab4bd8866b582062d70114ae Mon Sep 17 00:00:00 2001 From: Mike Yuan Date: Sat, 11 Jan 2025 16:26:55 +0100 Subject: [PATCH 06/11] bus-util: do not reset the count returned by sd_bus_track_count_name() Follow-up for 8402ca04d1a063c3d8a9e3d5c16df8bb8778ae98 While at it, turn the retval check for sd_bus_track_count_name() into assertion, given we're working with already established tracks (service_name_is_valid() should never yield false in this case). Addresses https://github.com/systemd/systemd/pull/35406#discussion_r1912066774 --- src/shared/bus-util.c | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/src/shared/bus-util.c b/src/shared/bus-util.c index 5c76346fc3..99be9d5395 100644 --- a/src/shared/bus-util.c +++ b/src/shared/bus-util.c @@ -700,16 +700,15 @@ int bus_track_add_name_many(sd_bus_track *t, char **l) { int bus_track_to_strv(sd_bus_track *t, char ***ret) { _cleanup_strv_free_ char **subscribed = NULL; - int r = 0; + int r; assert(ret); for (const char *n = sd_bus_track_first(t); n; n = sd_bus_track_next(t)) { - r = sd_bus_track_count_name(t, n); - if (r < 0) - return r; + int c = sd_bus_track_count_name(t, n); + assert(c >= 0); - for (int j = 0; j < r; j++) { + for (int j = 0; j < c; j++) { r = strv_extend(&subscribed, n); if (r < 0) return r; @@ -717,7 +716,7 @@ int bus_track_to_strv(sd_bus_track *t, char ***ret) { } *ret = TAKE_PTR(subscribed); - return r; + return 0; } int bus_open_system_watch_bind_with_description(sd_bus **ret, const char *description) { From a7516260b32dd26fb61b1dd702b9bc718cd420f9 Mon Sep 17 00:00:00 2001 From: Mike Yuan Date: Mon, 13 Jan 2025 17:06:21 +0100 Subject: [PATCH 07/11] core/manager: use FOREACH_ARRAY at one more place --- src/core/manager.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/core/manager.c b/src/core/manager.c index c4c1b9022c..3091d6ccfe 100644 --- a/src/core/manager.c +++ b/src/core/manager.c @@ -1817,15 +1817,16 @@ Manager* manager_free(Manager *m) { hashmap_free(m->uid_refs); hashmap_free(m->gid_refs); - for (ExecDirectoryType dt = 0; dt < _EXEC_DIRECTORY_TYPE_MAX; dt++) - m->prefix[dt] = mfree(m->prefix[dt]); + FOREACH_ARRAY(i, m->prefix, _EXEC_DIRECTORY_TYPE_MAX) + free(*i); + free(m->received_credentials_directory); free(m->received_encrypted_credentials_directory); free(m->watchdog_pretimeout_governor); free(m->watchdog_pretimeout_governor_overridden); - m->fw_ctx = fw_ctx_free(m->fw_ctx); + fw_ctx_free(m->fw_ctx); #if BPF_FRAMEWORK bpf_restrict_fs_destroy(m->restrict_fs); From af0e10354e567bfd0b9521376b2aad55f12a4e3d Mon Sep 17 00:00:00 2001 From: Mike Yuan Date: Sat, 11 Jan 2025 18:38:49 +0100 Subject: [PATCH 08/11] core/manager: drop duplicate bus track deserialization bus_init_api() now does this internally (after 8402ca04d1a063c3d8a9e3d5c16df8bb8778ae98). --- src/core/manager.c | 6 ------ 1 file changed, 6 deletions(-) diff --git a/src/core/manager.c b/src/core/manager.c index 3091d6ccfe..b749b0e1ca 100644 --- a/src/core/manager.c +++ b/src/core/manager.c @@ -2143,12 +2143,6 @@ int manager_startup(Manager *m, FILE *serialization, FDSet *fds, const char *roo /* Connect to the bus if we are good for it */ manager_setup_bus(m); - /* Now that we are connected to all possible buses, let's deserialize who is tracking us. */ - r = bus_track_coldplug(m->api_bus, &m->subscribed, false, m->subscribed_as_strv); - if (r < 0) - log_warning_errno(r, "Failed to deserialized tracked clients, ignoring: %m"); - m->subscribed_as_strv = strv_free(m->subscribed_as_strv); - r = manager_varlink_init(m); if (r < 0) log_warning_errno(r, "Failed to set up Varlink, ignoring: %m"); From a9a8d2e12fe01b928135895f00c5bca465b7d13b Mon Sep 17 00:00:00 2001 From: Mike Yuan Date: Mon, 13 Jan 2025 16:42:34 +0100 Subject: [PATCH 09/11] bus-util: introduce bus_get_instance_id() --- src/shared/bus-util.c | 23 +++++++++++++++++++++++ src/shared/bus-util.h | 2 ++ 2 files changed, 25 insertions(+) diff --git a/src/shared/bus-util.c b/src/shared/bus-util.c index 99be9d5395..0a887aec79 100644 --- a/src/shared/bus-util.c +++ b/src/shared/bus-util.c @@ -905,6 +905,29 @@ int bus_query_sender_pidref( return bus_creds_get_pidref(creds, ret); } +int bus_get_instance_id(sd_bus *bus, sd_id128_t *ret) { + _cleanup_(sd_bus_message_unrefp) sd_bus_message *reply = NULL; + int r; + + assert(bus); + assert(ret); + + r = sd_bus_call_method(bus, + "org.freedesktop.DBus", "/org/freedesktop/DBus", "org.freedesktop.DBus", "GetId", + /* error = */ NULL, &reply, + NULL); + if (r < 0) + return r; + + const char *id; + + r = sd_bus_message_read_basic(reply, 's', &id); + if (r < 0) + return r; + + return sd_id128_from_string(id, ret); +} + static const char* const bus_transport_table[] = { [BUS_TRANSPORT_LOCAL] = "local", [BUS_TRANSPORT_REMOTE] = "remote", diff --git a/src/shared/bus-util.h b/src/shared/bus-util.h index 47c0711868..5fb4bcca9f 100644 --- a/src/shared/bus-util.h +++ b/src/shared/bus-util.h @@ -81,4 +81,6 @@ int bus_register_malloc_status(sd_bus *bus, const char *destination); int bus_creds_get_pidref(sd_bus_creds *c, PidRef *ret); int bus_query_sender_pidref(sd_bus_message *m, PidRef *ret); +int bus_get_instance_id(sd_bus *bus, sd_id128_t *ret); + const char* bus_transport_to_string(BusTransport transport) _const_; From 1446e3c3921067e3a6228a3e172b5dfd95437136 Mon Sep 17 00:00:00 2001 From: Mike Yuan Date: Mon, 13 Jan 2025 17:06:35 +0100 Subject: [PATCH 10/11] core: serialize API bus id and validate before deserializing bus tracks --- TODO | 5 ---- src/core/dbus.c | 55 +++++++++++++++++++----------------- src/core/dbus.h | 1 - src/core/manager-serialize.c | 8 +++++- src/core/manager.h | 11 +++++--- src/shared/bus-util.c | 2 +- src/shared/bus-util.h | 2 +- 7 files changed, 45 insertions(+), 39 deletions(-) diff --git a/TODO b/TODO index c796b3e101..fdd119434e 100644 --- a/TODO +++ b/TODO @@ -131,11 +131,6 @@ Features: * in pid1: include ExecStart= cmdlines (and other Exec*= cmdlines) in polkit request, so that policies can match against command lines. -* in pid1: before processing subscribed_as_strv when we come back from a - bus disconnect, let's check the bus 128bit id reported by the bus driver: if - it doesn't equal the one from before we should skip reinstalling the bus - tracker objects. - * account number of units currently in activating/active/deactivating state in each slice, and expose this as a property of the slice, given this is a key metric of the resource management entity that a slice is. (maybe add a 2nd diff --git a/src/core/dbus.c b/src/core/dbus.c index 58cd1ee175..a2a2611a03 100644 --- a/src/core/dbus.c +++ b/src/core/dbus.c @@ -5,6 +5,7 @@ #include #include "sd-bus.h" +#include "sd-id128.h" #include "alloc-util.h" #include "bus-common-errors.h" @@ -774,6 +775,24 @@ static int bus_on_connection(sd_event_source *s, int fd, uint32_t revents, void return 0; } +static int bus_track_coldplug(sd_bus *bus, sd_bus_track **t, char * const *l) { + int r; + + assert(bus); + assert(t); + + if (strv_isempty(l)) + return 0; + + if (!*t) { + r = sd_bus_track_new(bus, t, NULL, NULL); + if (r < 0) + return r; + } + + return bus_track_add_name_many(*t, l); +} + static int bus_setup_api(Manager *m, sd_bus *bus) { char *name; Unit *u; @@ -860,10 +879,15 @@ int bus_init_api(Manager *m) { if (r < 0) return log_error_errno(r, "Failed to set up API bus: %m"); - (void) bus_track_coldplug(bus, &m->subscribed, /* recursive= */ false, m->subscribed_as_strv); + r = bus_get_instance_id(bus, &m->bus_id); + if (r < 0) + log_warning_errno(r, "Failed to query API bus instance ID, not deserializing subscriptions: %m"); + else if (sd_id128_is_null(m->deserialized_bus_id) || sd_id128_equal(m->bus_id, m->deserialized_bus_id)) + (void) bus_track_coldplug(bus, &m->subscribed, m->subscribed_as_strv); m->subscribed_as_strv = strv_free(m->subscribed_as_strv); - m->api_bus = TAKE_PTR(bus); + m->deserialized_bus_id = SD_ID128_NULL; + m->api_bus = TAKE_PTR(bus); return 0; } @@ -1015,6 +1039,9 @@ static void destroy_bus(Manager *m, sd_bus **bus) { log_warning_errno(r, "Failed to serialize api subscribers, ignoring: %m"); strv_free_and_replace(m->subscribed_as_strv, subscribed); + m->deserialized_bus_id = m->bus_id; + m->bus_id = SD_ID128_NULL; + m->subscribed = sd_bus_track_unref(m->subscribed); } @@ -1163,30 +1190,6 @@ void bus_track_serialize(sd_bus_track *t, FILE *f, const char *prefix) { } } -int bus_track_coldplug(sd_bus *bus, sd_bus_track **t, bool recursive, char **l) { - int r; - - assert(t); - - if (strv_isempty(l)) - return 0; - - if (!bus) - return 0; - - if (!*t) { - r = sd_bus_track_new(bus, t, NULL, NULL); - if (r < 0) - return r; - } - - r = sd_bus_track_set_recursive(*t, recursive); - if (r < 0) - return r; - - return bus_track_add_name_many(*t, l); -} - uint64_t manager_bus_n_queued_write(Manager *m) { uint64_t c = 0; sd_bus *b; diff --git a/src/core/dbus.h b/src/core/dbus.h index eb1baf6049..0f81102fc1 100644 --- a/src/core/dbus.h +++ b/src/core/dbus.h @@ -19,7 +19,6 @@ void bus_done(Manager *m); int bus_fdset_add_all(Manager *m, FDSet *fds); void bus_track_serialize(sd_bus_track *t, FILE *f, const char *prefix); -int bus_track_coldplug(sd_bus *bus, sd_bus_track **t, bool recursive, char **l); int bus_foreach_bus(Manager *m, sd_bus_track *subscribed2, int (*send_message)(sd_bus *bus, void *userdata), void *userdata); diff --git a/src/core/manager-serialize.c b/src/core/manager-serialize.c index 4aecb6de2f..fbc1475bcf 100644 --- a/src/core/manager-serialize.c +++ b/src/core/manager-serialize.c @@ -158,6 +158,7 @@ int manager_serialize( (void) serialize_ratelimit(f, "dump-ratelimit", &m->dump_ratelimit); (void) serialize_ratelimit(f, "reload-reexec-ratelimit", &m->reload_reexec_ratelimit); + (void) serialize_id128(f, "bus-id", m->bus_id); bus_track_serialize(m->subscribed, f, "subscribed"); r = dynamic_user_serialize(m, f, fds); @@ -487,7 +488,12 @@ int manager_deserialize(Manager *m, FILE *f, FDSet *fds) { manager_deserialize_gid_refs_one(m, val); else if ((val = startswith(l, "exec-runtime="))) (void) exec_shared_runtime_deserialize_one(m, val, fds); - else if ((val = startswith(l, "subscribed="))) { + else if ((val = startswith(l, "bus-id="))) { + + r = sd_id128_from_string(val, &m->deserialized_bus_id); + if (r < 0) + return r; + } else if ((val = startswith(l, "subscribed="))) { r = strv_extend(&m->subscribed_as_strv, val); if (r < 0) diff --git a/src/core/manager.h b/src/core/manager.h index 7016eab2d3..c5bd242968 100644 --- a/src/core/manager.h +++ b/src/core/manager.h @@ -335,13 +335,16 @@ struct Manager { int private_listen_fd; sd_event_source *private_listen_event_source; - /* Contains all the clients that are subscribed to signals via - the API bus. Note that private bus connections are always - considered subscribes, since they last for very short only, - and it is much simpler that way. */ + /* Contains all the clients that are subscribed to signals via the API bus. Note that private bus + * connections are always considered subscribes, since they last for very short only, and it is + * much simpler that way. */ sd_bus_track *subscribed; char **subscribed_as_strv; + /* The bus id of API bus acquired through org.freedesktop.DBus.GetId, which before deserializing + * subscriptions we'd use to verify the bus is still the same instance as before. */ + sd_id128_t bus_id, deserialized_bus_id; + /* This is used during reloading: before the reload we queue * the reply message here, and afterwards we send it */ sd_bus_message *pending_reload_message; diff --git a/src/shared/bus-util.c b/src/shared/bus-util.c index 0a887aec79..05926771d2 100644 --- a/src/shared/bus-util.c +++ b/src/shared/bus-util.c @@ -686,7 +686,7 @@ int bus_path_decode_unique(const char *path, const char *prefix, char **ret_send return 1; } -int bus_track_add_name_many(sd_bus_track *t, char **l) { +int bus_track_add_name_many(sd_bus_track *t, char * const *l) { int r = 0; assert(t); diff --git a/src/shared/bus-util.h b/src/shared/bus-util.h index 5fb4bcca9f..63c3ba5ee3 100644 --- a/src/shared/bus-util.h +++ b/src/shared/bus-util.h @@ -65,7 +65,7 @@ static inline int bus_log_connect_error(int r, BusTransport transport, RuntimeSc int bus_path_encode_unique(sd_bus *b, const char *prefix, const char *sender_id, const char *external_id, char **ret_path); int bus_path_decode_unique(const char *path, const char *prefix, char **ret_sender, char **ret_external); -int bus_track_add_name_many(sd_bus_track *t, char **l); +int bus_track_add_name_many(sd_bus_track *t, char * const *l); int bus_track_to_strv(sd_bus_track *t, char ***ret); int bus_open_system_watch_bind_with_description(sd_bus **ret, const char *description); From 34f4b817f67b002eae7e2c09b19bf4b66c4791b6 Mon Sep 17 00:00:00 2001 From: Mike Yuan Date: Mon, 13 Jan 2025 17:30:51 +0100 Subject: [PATCH 11/11] core/manager: restore bus track deserialization cleanup in manager_reload() There's zero explanation why it got (spuriously) removed in 8402ca04d1a063c3d8a9e3d5c16df8bb8778ae98... --- src/core/manager.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/core/manager.c b/src/core/manager.c index b749b0e1ca..8f1dc626c2 100644 --- a/src/core/manager.c +++ b/src/core/manager.c @@ -3801,6 +3801,11 @@ int manager_reload(Manager *m) { (void) manager_setup_handoff_timestamp_fd(m); (void) manager_setup_pidref_transport_fd(m); + /* Clean up deserialized bus track information. They're never consumed during reload (as opposed to + * reexec) since we do not disconnect from the bus. */ + m->subscribed_as_strv = strv_free(m->subscribed_as_strv); + m->deserialized_bus_id = SD_ID128_NULL; + /* Third, fire things up! */ manager_coldplug(m);