From 882032faaf9e2d2524936e82ccc770903d4c38d6 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Thu, 10 Oct 2024 11:49:47 +0200 Subject: [PATCH 1/7] meson: sort includes --- meson.build | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/meson.build b/meson.build index 651fb30584..caed1d75f0 100644 --- a/meson.build +++ b/meson.build @@ -561,10 +561,10 @@ conf.set_quoted('LONG_MAX_STR', '@0@'.format(long_max)) decl_headers = ''' #include -#include +#include #include #include -#include +#include ''' foreach decl : ['char16_t', From b71ac3e5095ee111561f35cddfec472aff0d7c56 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 9 Oct 2024 21:51:02 +0200 Subject: [PATCH 2/7] test: also dump varlink IDL for Machine interface in the test --- src/test/test-varlink-idl.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/test/test-varlink-idl.c b/src/test/test-varlink-idl.c index bdac083571..519056d044 100644 --- a/src/test/test-varlink-idl.c +++ b/src/test/test-varlink-idl.c @@ -14,6 +14,7 @@ #include "varlink-io.systemd.Credentials.h" #include "varlink-io.systemd.Import.h" #include "varlink-io.systemd.Journal.h" +#include "varlink-io.systemd.Machine.h" #include "varlink-io.systemd.ManagedOOM.h" #include "varlink-io.systemd.MountFileSystem.h" #include "varlink-io.systemd.NamespaceResource.h" @@ -187,6 +188,8 @@ TEST(parse_format) { print_separator(); test_parse_format_one(&vl_interface_io_systemd_Import); print_separator(); + test_parse_format_one(&vl_interface_io_systemd_Machine); + print_separator(); test_parse_format_one(&vl_interface_xyz_test); } From 44fd4548b56865929a32bfdaf386e27d2ba864d4 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 9 Oct 2024 18:00:31 +0200 Subject: [PATCH 3/7] macro: add voffsetof() helper, that operates like offsetof() but on variables --- src/fundamental/macro-fundamental.h | 1 + 1 file changed, 1 insertion(+) diff --git a/src/fundamental/macro-fundamental.h b/src/fundamental/macro-fundamental.h index 913c8b253c..7b79732326 100644 --- a/src/fundamental/macro-fundamental.h +++ b/src/fundamental/macro-fundamental.h @@ -549,6 +549,7 @@ static inline uint64_t ALIGN_OFFSET_U64(uint64_t l, uint64_t ali) { #define sizeof_field(struct_type, member) sizeof(((struct_type *) 0)->member) #define endoffsetof_field(struct_type, member) (offsetof(struct_type, member) + sizeof_field(struct_type, member)) +#define voffsetof(v, member) offsetof(typeof(v), member) #define _FOREACH_ARRAY(i, array, num, m, end) \ for (typeof(array[0]) *i = (array), *end = ({ \ From 47bbcfff06449f11f5e749daac8ac1dfe3b4da02 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Thu, 10 Oct 2024 10:33:50 +0200 Subject: [PATCH 4/7] varlink-idl: introduce c/.h file for common varlink IDL structures Some structures we'll use in various varlink interfaces, move them to a common .c/.h file. For now this is only the dual timestamp object, but there will be more soon. --- src/shared/meson.build | 1 + src/shared/varlink-idl-common.c | 12 ++++++++++++ src/shared/varlink-idl-common.h | 6 ++++++ src/shared/varlink-io.systemd.Machine.c | 8 +------- 4 files changed, 20 insertions(+), 7 deletions(-) create mode 100644 src/shared/varlink-idl-common.c create mode 100644 src/shared/varlink-idl-common.h diff --git a/src/shared/meson.build b/src/shared/meson.build index e759293364..42dd32024a 100644 --- a/src/shared/meson.build +++ b/src/shared/meson.build @@ -177,6 +177,7 @@ shared_sources = files( 'user-record.c', 'userdb-dropin.c', 'userdb.c', + 'varlink-idl-common.c', 'varlink-io.systemd.BootControl.c', 'varlink-io.systemd.Credentials.c', 'varlink-io.systemd.Hostname.c', diff --git a/src/shared/varlink-idl-common.c b/src/shared/varlink-idl-common.c new file mode 100644 index 0000000000..9ed5230883 --- /dev/null +++ b/src/shared/varlink-idl-common.c @@ -0,0 +1,12 @@ +/* SPDX-License-Identifier: LGPL-2.1-or-later */ + +#include "sd-varlink-idl.h" + +#include "varlink-idl-common.h" + +SD_VARLINK_DEFINE_STRUCT_TYPE( + Timestamp, + SD_VARLINK_FIELD_COMMENT("Timestamp in µs in the CLOCK_REALTIME clock (wallclock)"), + SD_VARLINK_DEFINE_FIELD(realtime, SD_VARLINK_INT, SD_VARLINK_NULLABLE), + SD_VARLINK_FIELD_COMMENT("Timestamp in µs in the CLOCK_MONOTONIC clock"), + SD_VARLINK_DEFINE_FIELD(monotonic, SD_VARLINK_INT, SD_VARLINK_NULLABLE)); diff --git a/src/shared/varlink-idl-common.h b/src/shared/varlink-idl-common.h new file mode 100644 index 0000000000..088d19bc2f --- /dev/null +++ b/src/shared/varlink-idl-common.h @@ -0,0 +1,6 @@ +/* SPDX-License-Identifier: LGPL-2.1-or-later */ +#pragma once + +#include "sd-varlink-idl.h" + +extern const sd_varlink_symbol vl_type_Timestamp; diff --git a/src/shared/varlink-io.systemd.Machine.c b/src/shared/varlink-io.systemd.Machine.c index 17e0d79ec4..d9a0b3fbaf 100644 --- a/src/shared/varlink-io.systemd.Machine.c +++ b/src/shared/varlink-io.systemd.Machine.c @@ -3,6 +3,7 @@ #include "sd-varlink-idl.h" #include "bus-polkit.h" +#include "varlink-idl-common.h" #include "varlink-io.systemd.Machine.h" static SD_VARLINK_DEFINE_METHOD( @@ -21,13 +22,6 @@ static SD_VARLINK_DEFINE_METHOD( SD_VARLINK_DEFINE_INPUT(allocateUnit, SD_VARLINK_BOOL, SD_VARLINK_NULLABLE), VARLINK_DEFINE_POLKIT_INPUT); -static SD_VARLINK_DEFINE_STRUCT_TYPE( - Timestamp, - SD_VARLINK_FIELD_COMMENT("Timestamp in µs in the CLOCK_REALTIME clock (wallclock)"), - SD_VARLINK_DEFINE_FIELD(realtime, SD_VARLINK_INT, SD_VARLINK_NULLABLE), - SD_VARLINK_FIELD_COMMENT("Timestamp in µs in the CLOCK_MONOTONIC clock"), - SD_VARLINK_DEFINE_FIELD(monotonic, SD_VARLINK_INT, SD_VARLINK_NULLABLE)); - static SD_VARLINK_DEFINE_METHOD( Unregister, SD_VARLINK_FIELD_COMMENT("The name of a machine to unregister."), From 1eb8a560a65eef3e32337845d65aa3a1f0f150da Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 9 Oct 2024 17:48:29 +0200 Subject: [PATCH 5/7] =?UTF-8?q?json:=20add=20builder/dispatcher=20for=20Pi?= =?UTF-8?q?dRef=20=E2=86=92=20JSON=20and=20back?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit So far, at the one place we sent a PID over Varlink we did so as a simple numeric pid_t value. That's of course is racy, since classic PIDs are recycled too eagerly. Let's address that, by passing around JSON objects distantly resembling our PidRef structure. Note that this JSON object does *not* contain the pidfd, however, but just the pidfd inode number if known. I originally planned to include the pidfd in some direct form, but I figured that's not really the best idea, since we always need a side-channel of some form for that (i.e. AF_UNIX/SCM_RIGHTS), but we should be able to report about PIDs even without that. Moreover, while sending the pid number and pidfd id around should always be OK to do, it's a lot more problematic to always send a pidfd around, since that implies that fd passing is on and it is OK to install fds remotely in some IPC peers fd table. For example, when doing a wild dump of service manager service state we really shouldn't end up with a bunch of fds installed in our client's fd table. Hence, all in all I think it is cleaner to define a structure carrying pid number and pidfd inode id, wich is passed directly as JSON. And then optionally, in a separate field also pass around a pidfd where it makes sense. Note that sending around pidfds is not that beneficial anymore if we have the pidfd inode id, because we can always securely and reliably get a pidfd back from a pair of pid + inode id: first we do pidfd_open() on the pid, and then we check if it is really the right one by comparing .st_ino after fstat(). This logic is implemented gracefully: if for some reason pidfd/pidfd inode nrs are not available (too old kernel), we'll fall back to plain PID numbers. The dispatching logic knows two distinct levels of validation of the provided PID data: if SD_JSON_STRICT is specified we'll acquire a pidfd for the PID, thus verifying it currently exists and failing if it doesn't. If the flag is not set, well just store the provided info as-is, will try to acquire a pidfd for it, but not fail if we cannot. Both modes are important in different contexts. Also note that in addition to the pidfd inode nr we always store the current boot ID of the system in the JSON object, since only the combination of pidfd inode nr and boot ID of the system really is a world-wide unique reference to a process. When dispatching a JSON pid field we operate somewhat gracefully: we either support the triplet structure of pid, pid inode nr, boot id, or we accept a simple classic UNIX pid. --- src/libsystemd/sd-json/json-util.c | 137 +++++++++++++++++++++++++++++ src/libsystemd/sd-json/json-util.h | 9 ++ src/libsystemd/sd-json/sd-json.c | 56 ++++++++++++ src/test/test-json.c | 43 +++++++++ 4 files changed, 245 insertions(+) diff --git a/src/libsystemd/sd-json/json-util.c b/src/libsystemd/sd-json/json-util.c index c8d67c8d55..2ff4587565 100644 --- a/src/libsystemd/sd-json/json-util.c +++ b/src/libsystemd/sd-json/json-util.c @@ -5,7 +5,9 @@ #include "in-addr-util.h" #include "iovec-util.h" #include "json-util.h" +#include "parse-util.h" #include "path-util.h" +#include "process-util.h" #include "string-util.h" #include "user-util.h" @@ -144,3 +146,138 @@ int json_dispatch_path(const char *name, sd_json_variant *variant, sd_json_dispa return 0; } + +int json_variant_new_pidref(sd_json_variant **ret, PidRef *pidref) { + sd_id128_t boot_id; + int r; + + /* Turns a PidRef into a triplet of PID, pidfd inode nr, and the boot ID. The triplet should uniquely + * identify the process globally, and be good enough to turn back into a pidfd + PidRef */ + + if (!pidref_is_set(pidref)) + return sd_json_variant_new_null(ret); + + r = pidref_acquire_pidfd_id(pidref); + if (r < 0 && !ERRNO_IS_NEG_NOT_SUPPORTED(r) && r != -ENOMEDIUM) + return r; + + if (pidref->fd_id > 0) { + /* If we have the pidfd inode number, also acquire the boot ID, to make things universally unique */ + r = sd_id128_get_boot(&boot_id); + if (r < 0) + return r; + } + + return sd_json_buildo( + ret, + SD_JSON_BUILD_PAIR_INTEGER("pid", pidref->pid), + SD_JSON_BUILD_PAIR_CONDITION(pidref->fd_id > 0, "pidfdId", SD_JSON_BUILD_INTEGER(pidref->fd_id)), + SD_JSON_BUILD_PAIR_CONDITION(pidref->fd_id > 0, "bootId", SD_JSON_BUILD_ID128(boot_id))); +} + +int json_dispatch_pidref(const char *name, sd_json_variant *variant, sd_json_dispatch_flags_t flags, void *userdata) { + PidRef *p = ASSERT_PTR(userdata); + int r; + + assert(variant); + + /* Turns a JSON PID triplet back into a PidRef, i.e. the reverse of json_variant_new_pidref() + * above. If SD_JSON_STRICT is set this will acquire a pidfd for the process, and validate that the + * auxiliary fields match it. Otherwise, this will just store the pid and the pidfd inode number (the + * latter not if the provided boot id differs from the local one), and not attempt to get a pidfd for + * it, or authenticate it. */ + + if (sd_json_variant_is_null(variant)) { + pidref_done(p); + return 0; + } + + struct { + uint64_t pid, fd_id; + sd_id128_t boot_id; + } data = {}; + + if (sd_json_variant_is_integer(variant)) + /* Permit a simple classic integer based format */ + data.pid = sd_json_variant_integer(variant); + else if (sd_json_variant_is_string(variant)) { + /* As usual, allow integers be encoded as strings too */ + r = safe_atou64(sd_json_variant_string(variant), &data.pid); + if (r < 0) + return json_log(variant, flags, r, "JSON field '%s' is not a numeric PID.", strna(name)); + } else if (sd_json_variant_is_object(variant)) { + + static const sd_json_dispatch_field dispatch_table[] = { + { "pid", _SD_JSON_VARIANT_TYPE_INVALID, sd_json_dispatch_uint64, voffsetof(data, pid), SD_JSON_MANDATORY }, + { "pidfdId", _SD_JSON_VARIANT_TYPE_INVALID, sd_json_dispatch_uint64, voffsetof(data, fd_id), 0 }, + { "bootId", SD_JSON_VARIANT_STRING, sd_json_dispatch_id128, voffsetof(data, boot_id), 0 }, + {} + }; + + r = sd_json_dispatch(variant, dispatch_table, flags, &data); + if (r < 0) + return r; + } else + return json_log(variant, flags, SYNTHETIC_ERRNO(EINVAL), "JSON field '%s' is neither a numeric PID nor a PID object.", strna(name)); + + /* Before casting the 64bit data.pid field to pid_t, let's ensure it fits the pid_t range. */ + if (data.pid > PID_T_MAX || !pid_is_valid(data.pid)) + return json_log(variant, flags, SYNTHETIC_ERRNO(EINVAL), "JSON field '%s' does not contain a valid PID.", strna(name)); + + int local_boot_id = -1; /* tristate */ + if (!sd_id128_is_null(data.boot_id)) { + sd_id128_t my_boot_id; + + r = sd_id128_get_boot(&my_boot_id); + if (r < 0) { + json_log(variant, flags | (FLAGS_SET(flags, SD_JSON_STRICT) ? 0 : SD_JSON_DEBUG), r, "Unable to get local boot ID to validate JSON field '%s': %m", strna(name)); + if (FLAGS_SET(flags, SD_JSON_STRICT)) + return r; + } else { + local_boot_id = sd_id128_equal(data.boot_id, my_boot_id); + if (!local_boot_id) { + json_log(variant, flags | (FLAGS_SET(flags, SD_JSON_STRICT) ? 0 : SD_JSON_DEBUG), 0, "JSON field '%s' refers to non-local PID.", strna(name)); + if (FLAGS_SET(flags, SD_JSON_STRICT)) + return -ESRCH; + } + } + } + + _cleanup_(pidref_done) PidRef np = PIDREF_NULL; + if (local_boot_id != 0) { + /* Try to acquire a pidfd – unless this is definitely not a local PID */ + r = pidref_set_pid(&np, data.pid); + if (r < 0) { + json_log(variant, flags | (FLAGS_SET(flags, SD_JSON_STRICT) ? 0 : SD_JSON_DEBUG), r, "Unable to get fd for PID in JSON field '%s': %m", strna(name)); + if (FLAGS_SET(flags, SD_JSON_STRICT)) + return r; + } + } + + /* If the the PID is dead or we otherwise can't get a pidfd of it, then store at least the PID number */ + if (!pidref_is_set(&np)) + np = PIDREF_MAKE_FROM_PID(data.pid); + + /* If the pidfd inode nr is specified, validate it or at least state */ + if (data.fd_id > 0) { + if (np.fd >= 0) { + r = pidref_acquire_pidfd_id(&np); + if (r < 0 && !ERRNO_IS_NOT_SUPPORTED(r)) + return json_log(variant, flags, r, "Unable to get pidfd ID to validate JSON field '%s': %m", strna(name)); + + if (data.fd_id != np.fd_id) { + json_log(variant, flags | (FLAGS_SET(flags, SD_JSON_STRICT) ? 0 : SD_JSON_DEBUG), 0, "JSON field '%s' references PID with non-matching inode number.", strna(name)); + if (FLAGS_SET(flags, SD_JSON_STRICT)) + return -ESRCH; + } + } else if (local_boot_id != 0) { + json_log(variant, flags|SD_JSON_DEBUG, 0, "Not validating PID inode number on JSON field '%s', because operating without pidfd.", strna(name)); + np.fd_id = data.fd_id; + } + } + + pidref_done(p); + *p = TAKE_PIDREF(np); + + return 0; +} diff --git a/src/libsystemd/sd-json/json-util.h b/src/libsystemd/sd-json/json-util.h index 3821e451d4..6b72d0d93d 100644 --- a/src/libsystemd/sd-json/json-util.h +++ b/src/libsystemd/sd-json/json-util.h @@ -6,6 +6,7 @@ #include "sd-json.h" #include "macro.h" +#include "pidref.h" #define JSON_VARIANT_REPLACE(v, q) \ do { \ @@ -112,6 +113,7 @@ int json_dispatch_user_group_name(const char *name, sd_json_variant *variant, sd int json_dispatch_const_user_group_name(const char *name, sd_json_variant *variant, sd_json_dispatch_flags_t flags, void *userdata); int json_dispatch_in_addr(const char *name, sd_json_variant *variant, sd_json_dispatch_flags_t flags, void *userdata); int json_dispatch_path(const char *name, sd_json_variant *variant, sd_json_dispatch_flags_t flags, void *userdata); +int json_dispatch_pidref(const char *name, sd_json_variant *variant, sd_json_dispatch_flags_t flags, void *userdata); static inline int json_variant_unbase64_iovec(sd_json_variant *v, struct iovec *ret) { return sd_json_variant_unbase64(v, ret ? &ret->iov_base : NULL, ret ? &ret->iov_len : NULL); @@ -144,6 +146,7 @@ enum { _JSON_BUILD_DUAL_TIMESTAMP, _JSON_BUILD_RATELIMIT, _JSON_BUILD_TRISTATE, + _JSON_BUILD_PIDREF, _JSON_BUILD_PAIR_INTEGER_NON_ZERO, _JSON_BUILD_PAIR_INTEGER_NON_NEGATIVE, @@ -168,6 +171,7 @@ enum { _JSON_BUILD_PAIR_HEX_NON_EMPTY, _JSON_BUILD_PAIR_OCTESCAPE_NON_EMPTY, _JSON_BUILD_PAIR_TRISTATE_NON_NULL, + _JSON_BUILD_PAIR_PIDREF_NON_NULL, _SD_JSON_BUILD_REALLYMAX, }; @@ -187,6 +191,7 @@ enum { #define JSON_BUILD_DUAL_TIMESTAMP(t) _JSON_BUILD_DUAL_TIMESTAMP, (dual_timestamp*) { t } #define JSON_BUILD_RATELIMIT(rl) _JSON_BUILD_RATELIMIT, (const RateLimit*) { rl } #define JSON_BUILD_TRISTATE(i) _JSON_BUILD_TRISTATE, (int) { i } +#define JSON_BUILD_PIDREF(p) _JSON_BUILD_PIDREF, (const PidRef*) { p } #define JSON_BUILD_PAIR_INTEGER_NON_ZERO(name, i) _JSON_BUILD_PAIR_INTEGER_NON_ZERO, (const char*) { name }, (int64_t) { i } #define JSON_BUILD_PAIR_INTEGER_NON_NEGATIVE(name, i) _JSON_BUILD_PAIR_INTEGER_NON_NEGATIVE, (const char*) { name }, (int64_t) { i } @@ -210,6 +215,7 @@ enum { #define JSON_BUILD_PAIR_HEX_NON_EMPTY(name, v, n) _JSON_BUILD_PAIR_HEX_NON_EMPTY, (const char*) { name }, (const void*) { v }, (size_t) { n } #define JSON_BUILD_PAIR_OCTESCAPE_NON_EMPTY(name, v, n) _JSON_BUILD_PAIR_HEX_NON_EMPTY, (const char*) { name }, (const void*) { v }, (size_t) { n } #define JSON_BUILD_PAIR_TRISTATE_NON_NULL(name, i) _JSON_BUILD_PAIR_TRISTATE_NON_NULL, (const char*) { name }, (int) { i } +#define JSON_BUILD_PAIR_PIDREF_NON_NULL(name, p) _JSON_BUILD_PAIR_PIDREF_NON_NULL, (const char*) { name }, (const PidRef*) { p } #define JSON_BUILD_PAIR_IOVEC_BASE64(name, iov) SD_JSON_BUILD_PAIR(name, JSON_BUILD_IOVEC_BASE64(iov)) #define JSON_BUILD_PAIR_IOVEC_HEX(name, iov) SD_JSON_BUILD_PAIR(name, JSON_BUILD_IOVEC_HEX(iov)) @@ -223,3 +229,6 @@ enum { #define JSON_BUILD_PAIR_DUAL_TIMESTAMP(name, t) SD_JSON_BUILD_PAIR(name, JSON_BUILD_DUAL_TIMESTAMP(t)) #define JSON_BUILD_PAIR_RATELIMIT(name, rl) SD_JSON_BUILD_PAIR(name, JSON_BUILD_RATELIMIT(rl)) #define JSON_BUILD_PAIR_TRISTATE(name, i) SD_JSON_BUILD_PAIR(name, JSON_BUILD_TRISTATE(i)) +#define JSON_BUILD_PAIR_PIDREF(name, p) SD_JSON_BUILD_PAIR(name, JSON_BUILD_PIDREF(p)) + +int json_variant_new_pidref(sd_json_variant **ret, PidRef *pidref); diff --git a/src/libsystemd/sd-json/sd-json.c b/src/libsystemd/sd-json/sd-json.c index 33ccdcac5a..03ef3ef0a8 100644 --- a/src/libsystemd/sd-json/sd-json.c +++ b/src/libsystemd/sd-json/sd-json.c @@ -4194,6 +4194,34 @@ _public_ int sd_json_buildv(sd_json_variant **ret, va_list ap) { break; } + case _JSON_BUILD_PIDREF: { + PidRef *pidref; + + if (!IN_SET(current->expect, EXPECT_TOPLEVEL, EXPECT_OBJECT_VALUE, EXPECT_ARRAY_ELEMENT)) { + r = -EINVAL; + goto finish; + } + + pidref = va_arg(ap, PidRef*); + + if (current->n_suppress == 0) { + r = json_variant_new_pidref(&add, pidref); + if (r < 0) + goto finish; + } + + n_subtract = 1; + + if (current->expect == EXPECT_TOPLEVEL) + current->expect = EXPECT_END; + else if (current->expect == EXPECT_OBJECT_VALUE) + current->expect = EXPECT_OBJECT_KEY; + else + assert(current->expect == EXPECT_ARRAY_ELEMENT); + + break; + } + case _JSON_BUILD_TRISTATE: { int tristate; @@ -4769,6 +4797,34 @@ _public_ int sd_json_buildv(sd_json_variant **ret, va_list ap) { break; } + case _JSON_BUILD_PAIR_PIDREF_NON_NULL: { + PidRef *p; + const char *n; + + if (current->expect != EXPECT_OBJECT_KEY) { + r = -EINVAL; + goto finish; + } + + n = va_arg(ap, const char*); + p = va_arg(ap, PidRef*); + + if (pidref_is_set(p) && current->n_suppress == 0) { + r = sd_json_variant_new_string(&add, n); + if (r < 0) + goto finish; + + r = json_variant_new_pidref(&add_more, p); + if (r < 0) + goto finish; + } + + n_subtract = 2; /* we generated two items */ + + current->expect = EXPECT_OBJECT_KEY; + break; + } + case _JSON_BUILD_PAIR_CALLBACK_NON_NULL: { sd_json_build_callback_t cb; void *userdata; diff --git a/src/test/test-json.c b/src/test/test-json.c index 378a3d0d8d..32cd285841 100644 --- a/src/test/test-json.c +++ b/src/test/test-json.c @@ -1264,4 +1264,47 @@ TEST(parse_continue) { assert_se(sd_json_parse_with_source_continue(&p, "piff", /* flags= */ 0, &x, &line, &column) == -EINVAL); } +TEST(pidref) { + _cleanup_(pidref_done) PidRef myself = PIDREF_NULL, pid1 = PIDREF_NULL; + + assert_se(pidref_set_pid(&myself, 0) >= 0); + assert_se(pidref_set_pid(&pid1, 1) >= 0); + + _cleanup_(sd_json_variant_unrefp) sd_json_variant *v = NULL; + assert_se(sd_json_buildo(&v, + JSON_BUILD_PAIR_PIDREF("myself", &myself), + JSON_BUILD_PAIR_PIDREF("pid1", &pid1)) >= 0); + + sd_json_variant_dump(v, SD_JSON_FORMAT_COLOR|SD_JSON_FORMAT_PRETTY, NULL, NULL); + + struct { + PidRef myself, pid1; + } data = { + .myself = PIDREF_NULL, + .pid1 = PIDREF_NULL, + }; + + assert_se(sd_json_dispatch( + v, + (const sd_json_dispatch_field[]) { + { "myself", _SD_JSON_VARIANT_TYPE_INVALID, json_dispatch_pidref, voffsetof(data, myself), 0 }, + { "pid1", _SD_JSON_VARIANT_TYPE_INVALID, json_dispatch_pidref, voffsetof(data, pid1), 0 }, + {}, + }, + /* flags= */ 0, + &data) >= 0); + + assert_se(pidref_equal(&myself, &data.myself)); + assert_se(pidref_equal(&pid1, &data.pid1)); + + assert_se(!pidref_equal(&myself, &data.pid1)); + assert_se(!pidref_equal(&pid1, &data.myself)); + + assert_se((myself.fd_id > 0) == (data.myself.fd_id > 0)); + assert_se((pid1.fd_id > 0) == (data.pid1.fd_id > 0)); + + pidref_done(&data.myself); + pidref_done(&data.pid1); +} + DEFINE_TEST_MAIN(LOG_DEBUG); From 0fc530d15709a4b531412fdadf3cfb5d0433b62f Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 9 Oct 2024 17:49:43 +0200 Subject: [PATCH 6/7] machined: make List() varlink method return a full pidref JSON object for leader This new call has not been released yet, hence we can still change the encoding of the "leader" field. --- src/machine/machined-varlink.c | 2 +- src/shared/varlink-idl-common.c | 9 +++++++++ src/shared/varlink-idl-common.h | 1 + src/shared/varlink-io.systemd.Machine.c | 4 +++- test/units/TEST-13-NSPAWN.machined.sh | 2 +- 5 files changed, 15 insertions(+), 3 deletions(-) diff --git a/src/machine/machined-varlink.c b/src/machine/machined-varlink.c index 6100024174..a9e49c35e1 100644 --- a/src/machine/machined-varlink.c +++ b/src/machine/machined-varlink.c @@ -403,7 +403,7 @@ static int list_machine_one(sd_varlink *link, Machine *m, bool more) { JSON_BUILD_PAIR_STRING_NON_EMPTY("service", m->service), JSON_BUILD_PAIR_STRING_NON_EMPTY("rootDirectory", m->root_directory), JSON_BUILD_PAIR_STRING_NON_EMPTY("unit", m->unit), - SD_JSON_BUILD_PAIR_CONDITION(pidref_is_set(&m->leader), "leader", SD_JSON_BUILD_UNSIGNED(m->leader.pid)), + SD_JSON_BUILD_PAIR_CONDITION(pidref_is_set(&m->leader), "leader", JSON_BUILD_PIDREF(&m->leader)), SD_JSON_BUILD_PAIR_CONDITION(dual_timestamp_is_set(&m->timestamp), "timestamp", JSON_BUILD_DUAL_TIMESTAMP(&m->timestamp)), SD_JSON_BUILD_PAIR_CONDITION(m->vsock_cid != VMADDR_CID_ANY, "vSockCid", SD_JSON_BUILD_UNSIGNED(m->vsock_cid)), JSON_BUILD_PAIR_STRING_NON_EMPTY("sshAddress", m->ssh_address), diff --git a/src/shared/varlink-idl-common.c b/src/shared/varlink-idl-common.c index 9ed5230883..e48fd46e0f 100644 --- a/src/shared/varlink-idl-common.c +++ b/src/shared/varlink-idl-common.c @@ -10,3 +10,12 @@ SD_VARLINK_DEFINE_STRUCT_TYPE( SD_VARLINK_DEFINE_FIELD(realtime, SD_VARLINK_INT, SD_VARLINK_NULLABLE), SD_VARLINK_FIELD_COMMENT("Timestamp in µs in the CLOCK_MONOTONIC clock"), SD_VARLINK_DEFINE_FIELD(monotonic, SD_VARLINK_INT, SD_VARLINK_NULLABLE)); + +SD_VARLINK_DEFINE_STRUCT_TYPE( + ProcessId, + SD_VARLINK_FIELD_COMMENT("Numeric UNIX PID value"), + SD_VARLINK_DEFINE_FIELD(pid, SD_VARLINK_INT, 0), + SD_VARLINK_FIELD_COMMENT("64bit inode number of pidfd if known"), + SD_VARLINK_DEFINE_FIELD(pidfdId, SD_VARLINK_INT, SD_VARLINK_NULLABLE), + SD_VARLINK_FIELD_COMMENT("Boot ID of the system the inode number belongs to"), + SD_VARLINK_DEFINE_FIELD(bootId, SD_VARLINK_INT, SD_VARLINK_NULLABLE)); diff --git a/src/shared/varlink-idl-common.h b/src/shared/varlink-idl-common.h index 088d19bc2f..91b643f810 100644 --- a/src/shared/varlink-idl-common.h +++ b/src/shared/varlink-idl-common.h @@ -4,3 +4,4 @@ #include "sd-varlink-idl.h" extern const sd_varlink_symbol vl_type_Timestamp; +extern const sd_varlink_symbol vl_type_ProcessId; diff --git a/src/shared/varlink-io.systemd.Machine.c b/src/shared/varlink-io.systemd.Machine.c index d9a0b3fbaf..332f50e588 100644 --- a/src/shared/varlink-io.systemd.Machine.c +++ b/src/shared/varlink-io.systemd.Machine.c @@ -57,7 +57,7 @@ static SD_VARLINK_DEFINE_METHOD_FULL( SD_VARLINK_FIELD_COMMENT("The class of this machine"), SD_VARLINK_DEFINE_OUTPUT(class, SD_VARLINK_STRING, 0), SD_VARLINK_FIELD_COMMENT("Leader process PID of this machine"), - SD_VARLINK_DEFINE_OUTPUT(leader, SD_VARLINK_INT, SD_VARLINK_NULLABLE), + SD_VARLINK_DEFINE_OUTPUT_BY_TYPE(leader, ProcessId, SD_VARLINK_NULLABLE), SD_VARLINK_FIELD_COMMENT("Root directory of this machine, if known, relative to host file system"), SD_VARLINK_DEFINE_OUTPUT(rootDirectory, SD_VARLINK_STRING, SD_VARLINK_NULLABLE), SD_VARLINK_FIELD_COMMENT("The service manager unit this machine resides in"), @@ -77,6 +77,8 @@ static SD_VARLINK_DEFINE_ERROR(MachineExists); SD_VARLINK_DEFINE_INTERFACE( io_systemd_Machine, "io.systemd.Machine", + SD_VARLINK_SYMBOL_COMMENT("An object for referencing UNIX processes"), + &vl_type_ProcessId, SD_VARLINK_SYMBOL_COMMENT("A timestamp object consisting of both CLOCK_REALTIME and CLOCK_MONOTONIC timestamps"), &vl_type_Timestamp, &vl_method_Register, diff --git a/test/units/TEST-13-NSPAWN.machined.sh b/test/units/TEST-13-NSPAWN.machined.sh index b54f6e80d3..1ae393dd1f 100755 --- a/test/units/TEST-13-NSPAWN.machined.sh +++ b/test/units/TEST-13-NSPAWN.machined.sh @@ -251,7 +251,7 @@ varlinkctl --more call /run/systemd/machine/io.systemd.Machine io.systemd.Machin varlinkctl --more call /run/systemd/machine/io.systemd.Machine io.systemd.Machine.List '{}' | grep '.host' varlinkctl call /run/systemd/machine/io.systemd.Machine io.systemd.Machine.List '{"name":"long-running"}' -pid=$(varlinkctl call /run/systemd/machine/io.systemd.Machine io.systemd.Machine.List '{"name":"long-running"}' | jq '.leader') +pid=$(varlinkctl call /run/systemd/machine/io.systemd.Machine io.systemd.Machine.List '{"name":"long-running"}' | jq '.leader.pid') varlinkctl call /run/systemd/machine/io.systemd.Machine io.systemd.Machine.List '{"name":"long-running"}' >/tmp/expected varlinkctl call /run/systemd/machine/io.systemd.Machine io.systemd.Machine.List "{\"pid\":$pid}" >/tmp/got diff -u /tmp/expected /tmp/got From 755cb018c9b3e93245afb86ec94223756ddd70e4 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 9 Oct 2024 22:03:51 +0200 Subject: [PATCH 7/7] machined: also take new ProcessId structure when registering processes --- src/machine/machine-varlink.c | 19 +++++-------------- src/shared/varlink-io.systemd.Machine.c | 2 +- 2 files changed, 6 insertions(+), 15 deletions(-) diff --git a/src/machine/machine-varlink.c b/src/machine/machine-varlink.c index 26b1e841a6..a1ecdffa91 100644 --- a/src/machine/machine-varlink.c +++ b/src/machine/machine-varlink.c @@ -45,25 +45,16 @@ static int machine_name(const char *name, sd_json_variant *variant, sd_json_disp static int machine_leader(const char *name, sd_json_variant *variant, sd_json_dispatch_flags_t flags, void *userdata) { PidRef *leader = ASSERT_PTR(userdata); _cleanup_(pidref_done) PidRef temp = PIDREF_NULL; - uint64_t k; int r; - if (!sd_json_variant_is_unsigned(variant)) - return json_log(variant, flags, SYNTHETIC_ERRNO(EINVAL), "JSON field '%s' is not an integer.", strna(name)); + r = json_dispatch_pidref(name, variant, flags, &temp); + if (r < 0) + return r; - k = sd_json_variant_unsigned(variant); - if (k > PID_T_MAX || !pid_is_valid(k)) - return json_log(variant, flags, SYNTHETIC_ERRNO(EINVAL), "JSON field '%s' is not a valid PID.", strna(name)); - - if (k == 1) + if (temp.pid == 1) /* refuse PID 1 */ return json_log(variant, flags, SYNTHETIC_ERRNO(EINVAL), "JSON field '%s' is not a valid leader PID.", strna(name)); - r = pidref_set_pid(&temp, k); - if (r < 0) - return json_log(variant, flags, r, "Failed to pin process " PID_FMT ": %m", leader->pid); - pidref_done(leader); - *leader = TAKE_PIDREF(temp); return 0; @@ -133,7 +124,7 @@ int vl_method_register(sd_varlink *link, sd_json_variant *parameters, sd_varlink { "id", SD_JSON_VARIANT_STRING, sd_json_dispatch_id128, offsetof(Machine, id), 0 }, { "service", SD_JSON_VARIANT_STRING, sd_json_dispatch_string, offsetof(Machine, service), 0 }, { "class", SD_JSON_VARIANT_STRING, dispatch_machine_class, offsetof(Machine, class), SD_JSON_MANDATORY }, - { "leader", SD_JSON_VARIANT_UNSIGNED, machine_leader, offsetof(Machine, leader), 0 }, + { "leader", _SD_JSON_VARIANT_TYPE_INVALID, machine_leader, offsetof(Machine, leader), SD_JSON_STRICT }, { "rootDirectory", SD_JSON_VARIANT_STRING, json_dispatch_path, offsetof(Machine, root_directory), 0 }, { "ifIndices", SD_JSON_VARIANT_ARRAY, machine_ifindices, 0, 0 }, { "vSockCid", _SD_JSON_VARIANT_TYPE_INVALID, machine_cid, offsetof(Machine, vsock_cid), 0 }, diff --git a/src/shared/varlink-io.systemd.Machine.c b/src/shared/varlink-io.systemd.Machine.c index 332f50e588..ce80249123 100644 --- a/src/shared/varlink-io.systemd.Machine.c +++ b/src/shared/varlink-io.systemd.Machine.c @@ -12,7 +12,7 @@ static SD_VARLINK_DEFINE_METHOD( SD_VARLINK_DEFINE_INPUT(id, SD_VARLINK_STRING, SD_VARLINK_NULLABLE), SD_VARLINK_DEFINE_INPUT(service, SD_VARLINK_STRING, SD_VARLINK_NULLABLE), SD_VARLINK_DEFINE_INPUT(class, SD_VARLINK_STRING, 0), - SD_VARLINK_DEFINE_INPUT(leader, SD_VARLINK_INT, SD_VARLINK_NULLABLE), + SD_VARLINK_DEFINE_INPUT_BY_TYPE(leader, ProcessId, SD_VARLINK_NULLABLE), SD_VARLINK_DEFINE_INPUT(rootDirectory, SD_VARLINK_STRING, SD_VARLINK_NULLABLE), SD_VARLINK_DEFINE_INPUT(ifIndices, SD_VARLINK_INT, SD_VARLINK_ARRAY|SD_VARLINK_NULLABLE), SD_VARLINK_DEFINE_INPUT(vSockCid, SD_VARLINK_INT, SD_VARLINK_NULLABLE),