From 366f57bff44bc67009aa5c877ebc5fcb14156aeb Mon Sep 17 00:00:00 2001 From: Ivan Kruglov Date: Wed, 16 Jul 2025 05:57:56 -0700 Subject: [PATCH 1/6] basic: pidref_is_set_or_automatic() --- src/basic/pidref.h | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/basic/pidref.h b/src/basic/pidref.h index 1a7ff85935..8280910f5d 100644 --- a/src/basic/pidref.h +++ b/src/basic/pidref.h @@ -55,6 +55,10 @@ static inline bool pidref_is_set(const PidRef *pidref) { bool pidref_is_automatic(const PidRef *pidref); +static inline bool pidref_is_set_or_automatic(const PidRef *pidref) { + return pidref_is_set(pidref) || pidref_is_automatic(pidref); +} + static inline bool pidref_is_remote(const PidRef *pidref) { /* If the fd is set to -EREMOTE we assume PidRef does not refer to a local PID, but on another * machine (and we just got the PidRef initialized due to deserialization of some RPC message) */ From 4ddd3fe1fb14286fe105736ec733ecc1680552d6 Mon Sep 17 00:00:00 2001 From: Ivan Kruglov Date: Wed, 16 Jul 2025 06:47:22 -0700 Subject: [PATCH 2/6] core: make name/pid lookup params use AND logic in io.systemd.Unit.List --- src/core/varlink-unit.c | 88 ++++++++++++++++++++-------- src/shared/varlink-io.systemd.Unit.c | 4 +- 2 files changed, 65 insertions(+), 27 deletions(-) diff --git a/src/core/varlink-unit.c b/src/core/varlink-unit.c index a63e405b41..0c4a025b3e 100644 --- a/src/core/varlink-unit.c +++ b/src/core/varlink-unit.c @@ -6,6 +6,7 @@ #include "cgroup.h" #include "condition.h" #include "execute.h" +#include "format-util.h" #include "install.h" #include "json-util.h" #include "manager.h" @@ -364,6 +365,58 @@ static void unit_lookup_parameters_done(UnitLookupParameters *p) { pidref_done(&p->pidref); } +static int varlink_error_no_such_unit(sd_varlink *v, const char *name) { + return sd_varlink_errorbo( + ASSERT_PTR(v), + VARLINK_ERROR_UNIT_NO_SUCH_UNIT, + JSON_BUILD_PAIR_STRING_NON_EMPTY("parameter", name)); +} + +static int varlink_error_conflict_lookup_parameters(sd_varlink *v, const UnitLookupParameters *p) { + log_debug_errno( + ESRCH, + "Searching unit by lookup parameters name='%s' pid="PID_FMT" resulted in multiple different units", + p->name, + p->pidref.pid); + + return varlink_error_no_such_unit(v, /* name= */ NULL); +} + +static int lookup_unit_by_parameters(sd_varlink *link, Manager *manager, UnitLookupParameters *p, Unit **ret_unit) { + /* The function can return ret_unit=NULL if no lookup parameters provided */ + Unit *unit = NULL; + int r; + + assert(link); + assert(manager); + assert(p); + assert(ret_unit); + + if (p->name) { + unit = manager_get_unit(manager, p->name); + if (!unit) + return varlink_error_no_such_unit(link, "name"); + } + + if (pidref_is_set_or_automatic(&p->pidref)) { + Unit *pid_unit; + r = lookup_unit_by_pidref(link, manager, &p->pidref, &pid_unit); + if (r == -EINVAL) + return sd_varlink_error_invalid_parameter_name(link, "pid"); + if (r == -ESRCH) + return varlink_error_no_such_unit(link, "pid"); + if (r < 0) + return r; + if (pid_unit != unit && unit != NULL) + return varlink_error_conflict_lookup_parameters(link, p); + + unit = pid_unit; + } + + *ret_unit = unit; + return 0; +} + int vl_method_list_units(sd_varlink *link, sd_json_variant *parameters, sd_varlink_method_flags_t flags, void *userdata) { static const sd_json_dispatch_field dispatch_table[] = { { "name", SD_JSON_VARIANT_STRING, json_dispatch_const_unit_name, offsetof(UnitLookupParameters, name), 0 /* allows UNIT_NAME_PLAIN | UNIT_NAME_INSTANCE */ }, @@ -375,6 +428,8 @@ int vl_method_list_units(sd_varlink *link, sd_json_variant *parameters, sd_varli _cleanup_(unit_lookup_parameters_done) UnitLookupParameters p = { .pidref = PIDREF_NULL, }; + Unit *unit, *previous = NULL; + const char *k; int r; assert(link); @@ -384,37 +439,18 @@ int vl_method_list_units(sd_varlink *link, sd_json_variant *parameters, sd_varli if (r != 0) return r; - if (p.name) { - Unit *unit = manager_get_unit(manager, p.name); - if (!unit) - return sd_varlink_error(link, VARLINK_ERROR_UNIT_NO_SUCH_UNIT, NULL); - + r = lookup_unit_by_parameters(link, manager, &p, &unit); + if (r < 0) + return r; + if (unit) return list_unit_one(link, unit, /* more = */ false); - } - - if (pidref_is_set(&p.pidref) || pidref_is_automatic(&p.pidref)) { - Unit *unit; - r = lookup_unit_by_pidref(link, manager, &p.pidref, &unit); - if (r == -EINVAL) - return sd_varlink_error_invalid_parameter_name(link, "pid"); - if (r == -ESRCH) - return sd_varlink_error(link, VARLINK_ERROR_UNIT_NO_SUCH_UNIT, NULL); - if (r < 0) - return r; - - return list_unit_one(link, unit, /* more = */ false); - } - - // TODO lookup by invocationID, CGroup if (!FLAGS_SET(flags, SD_VARLINK_METHOD_MORE)) return sd_varlink_error(link, SD_VARLINK_ERROR_EXPECTED_MORE, NULL); - const char *k; - Unit *u, *previous = NULL; - HASHMAP_FOREACH_KEY(u, k, manager->units) { + HASHMAP_FOREACH_KEY(unit, k, manager->units) { /* ignore aliases */ - if (k != u->id) + if (k != unit->id) continue; if (previous) { @@ -423,7 +459,7 @@ int vl_method_list_units(sd_varlink *link, sd_json_variant *parameters, sd_varli return r; } - previous = u; + previous = unit; } if (previous) diff --git a/src/shared/varlink-io.systemd.Unit.c b/src/shared/varlink-io.systemd.Unit.c index ec6cf89b1e..3f9e7c40f2 100644 --- a/src/shared/varlink-io.systemd.Unit.c +++ b/src/shared/varlink-io.systemd.Unit.c @@ -1006,7 +1006,9 @@ static SD_VARLINK_DEFINE_STRUCT_TYPE( SD_VARLINK_FIELD_COMMENT("The cgroup runtime of the unit"), SD_VARLINK_DEFINE_FIELD_BY_TYPE(CGroup, CGroupRuntime, SD_VARLINK_NULLABLE)); -static SD_VARLINK_DEFINE_ERROR(NoSuchUnit); +static SD_VARLINK_DEFINE_ERROR( + NoSuchUnit, + SD_VARLINK_DEFINE_FIELD(parameter, SD_VARLINK_STRING, SD_VARLINK_NULLABLE)); static SD_VARLINK_DEFINE_METHOD_FULL( List, From 6b78d931cdcb10ab9ebd86d463f7e32188d925a8 Mon Sep 17 00:00:00 2001 From: Ivan Kruglov Date: Wed, 16 Jul 2025 06:56:59 -0700 Subject: [PATCH 3/6] core: io.systemd.Unit.List can lookup unit by CGroup --- src/core/varlink-unit.c | 26 +++++++++++++++++++++----- src/shared/varlink-io.systemd.Unit.c | 2 ++ 2 files changed, 23 insertions(+), 5 deletions(-) diff --git a/src/core/varlink-unit.c b/src/core/varlink-unit.c index 0c4a025b3e..5a88bb3840 100644 --- a/src/core/varlink-unit.c +++ b/src/core/varlink-unit.c @@ -10,6 +10,7 @@ #include "install.h" #include "json-util.h" #include "manager.h" +#include "path-util.h" #include "pidref.h" #include "set.h" #include "strv.h" @@ -356,7 +357,7 @@ static int lookup_unit_by_pidref(sd_varlink *link, Manager *manager, PidRef *pid } typedef struct UnitLookupParameters { - const char *name; + const char *name, *cgroup; PidRef pidref; } UnitLookupParameters; @@ -375,9 +376,10 @@ static int varlink_error_no_such_unit(sd_varlink *v, const char *name) { static int varlink_error_conflict_lookup_parameters(sd_varlink *v, const UnitLookupParameters *p) { log_debug_errno( ESRCH, - "Searching unit by lookup parameters name='%s' pid="PID_FMT" resulted in multiple different units", + "Searching unit by lookup parameters name='%s' pid="PID_FMT" cgroup='%s' resulted in multiple different units", p->name, - p->pidref.pid); + p->pidref.pid, + p->cgroup); return varlink_error_no_such_unit(v, /* name= */ NULL); } @@ -413,14 +415,28 @@ static int lookup_unit_by_parameters(sd_varlink *link, Manager *manager, UnitLoo unit = pid_unit; } + if (p->cgroup) { + if (!path_is_safe(p->cgroup)) + return sd_varlink_error_invalid_parameter_name(link, "cgroup"); + + Unit *cgroup_unit = manager_get_unit_by_cgroup(manager, p->cgroup); + if (!cgroup_unit) + return varlink_error_no_such_unit(link, "cgroup"); + if (cgroup_unit != unit && unit != NULL) + return varlink_error_conflict_lookup_parameters(link, p); + + unit = cgroup_unit; + } + *ret_unit = unit; return 0; } int vl_method_list_units(sd_varlink *link, sd_json_variant *parameters, sd_varlink_method_flags_t flags, void *userdata) { static const sd_json_dispatch_field dispatch_table[] = { - { "name", SD_JSON_VARIANT_STRING, json_dispatch_const_unit_name, offsetof(UnitLookupParameters, name), 0 /* allows UNIT_NAME_PLAIN | UNIT_NAME_INSTANCE */ }, - { "pid", _SD_JSON_VARIANT_TYPE_INVALID, json_dispatch_pidref, offsetof(UnitLookupParameters, pidref), SD_JSON_RELAX /* allows PID_AUTOMATIC */ }, + { "name", SD_JSON_VARIANT_STRING, json_dispatch_const_unit_name, offsetof(UnitLookupParameters, name), 0 /* allows UNIT_NAME_PLAIN | UNIT_NAME_INSTANCE */ }, + { "pid", _SD_JSON_VARIANT_TYPE_INVALID, json_dispatch_pidref, offsetof(UnitLookupParameters, pidref), SD_JSON_RELAX /* allows PID_AUTOMATIC */ }, + { "cgroup", SD_JSON_VARIANT_STRING, json_dispatch_const_path, offsetof(UnitLookupParameters, cgroup), SD_JSON_STRICT /* require normalized path */ }, {} }; diff --git a/src/shared/varlink-io.systemd.Unit.c b/src/shared/varlink-io.systemd.Unit.c index 3f9e7c40f2..b4c7e22b38 100644 --- a/src/shared/varlink-io.systemd.Unit.c +++ b/src/shared/varlink-io.systemd.Unit.c @@ -1017,6 +1017,8 @@ static SD_VARLINK_DEFINE_METHOD_FULL( SD_VARLINK_DEFINE_INPUT(name, SD_VARLINK_STRING, SD_VARLINK_NULLABLE), SD_VARLINK_FIELD_COMMENT("If non-null the PID of a unit. Special value 0 means to take pid of the caller."), SD_VARLINK_DEFINE_INPUT_BY_TYPE(pid, ProcessId, SD_VARLINK_NULLABLE), + SD_VARLINK_FIELD_COMMENT("If non-null the cgroup of a unit"), + SD_VARLINK_DEFINE_INPUT(cgroup, SD_VARLINK_STRING, SD_VARLINK_NULLABLE), SD_VARLINK_FIELD_COMMENT("Configuration of the unit"), SD_VARLINK_DEFINE_OUTPUT_BY_TYPE(context, UnitContext, 0), SD_VARLINK_FIELD_COMMENT("Runtime information of the unit"), From 6c2c2e059829148e8e10a71b948a84c8080a06b1 Mon Sep 17 00:00:00 2001 From: Ivan Kruglov Date: Wed, 16 Jul 2025 07:03:49 -0700 Subject: [PATCH 4/6] core: io.systemd.Unit.List can lookup by InvocationID --- src/core/varlink-unit.c | 17 +++++++++++++++-- src/shared/varlink-io.systemd.Unit.c | 2 ++ 2 files changed, 17 insertions(+), 2 deletions(-) diff --git a/src/core/varlink-unit.c b/src/core/varlink-unit.c index 5a88bb3840..f71c3d7e33 100644 --- a/src/core/varlink-unit.c +++ b/src/core/varlink-unit.c @@ -359,6 +359,7 @@ static int lookup_unit_by_pidref(sd_varlink *link, Manager *manager, PidRef *pid typedef struct UnitLookupParameters { const char *name, *cgroup; PidRef pidref; + sd_id128_t invocation_id; } UnitLookupParameters; static void unit_lookup_parameters_done(UnitLookupParameters *p) { @@ -376,10 +377,11 @@ static int varlink_error_no_such_unit(sd_varlink *v, const char *name) { static int varlink_error_conflict_lookup_parameters(sd_varlink *v, const UnitLookupParameters *p) { log_debug_errno( ESRCH, - "Searching unit by lookup parameters name='%s' pid="PID_FMT" cgroup='%s' resulted in multiple different units", + "Searching unit by lookup parameters name='%s' pid="PID_FMT" cgroup='%s' invocationID='%s' resulted in multiple different units", p->name, p->pidref.pid, - p->cgroup); + p->cgroup, + sd_id128_is_null(p->invocation_id) ? "" : SD_ID128_TO_UUID_STRING(p->invocation_id)); return varlink_error_no_such_unit(v, /* name= */ NULL); } @@ -428,6 +430,16 @@ static int lookup_unit_by_parameters(sd_varlink *link, Manager *manager, UnitLoo unit = cgroup_unit; } + if (!sd_id128_is_null(p->invocation_id)) { + Unit *id128_unit = hashmap_get(manager->units_by_invocation_id, &p->invocation_id); + if (!id128_unit) + return varlink_error_no_such_unit(link, "invocationID"); + if (id128_unit != unit && unit != NULL) + return varlink_error_conflict_lookup_parameters(link, p); + + unit = id128_unit; + } + *ret_unit = unit; return 0; } @@ -437,6 +449,7 @@ int vl_method_list_units(sd_varlink *link, sd_json_variant *parameters, sd_varli { "name", SD_JSON_VARIANT_STRING, json_dispatch_const_unit_name, offsetof(UnitLookupParameters, name), 0 /* allows UNIT_NAME_PLAIN | UNIT_NAME_INSTANCE */ }, { "pid", _SD_JSON_VARIANT_TYPE_INVALID, json_dispatch_pidref, offsetof(UnitLookupParameters, pidref), SD_JSON_RELAX /* allows PID_AUTOMATIC */ }, { "cgroup", SD_JSON_VARIANT_STRING, json_dispatch_const_path, offsetof(UnitLookupParameters, cgroup), SD_JSON_STRICT /* require normalized path */ }, + { "invocationID", SD_JSON_VARIANT_STRING, sd_json_dispatch_id128, offsetof(UnitLookupParameters, invocation_id), 0 }, {} }; diff --git a/src/shared/varlink-io.systemd.Unit.c b/src/shared/varlink-io.systemd.Unit.c index b4c7e22b38..eacc960792 100644 --- a/src/shared/varlink-io.systemd.Unit.c +++ b/src/shared/varlink-io.systemd.Unit.c @@ -1019,6 +1019,8 @@ static SD_VARLINK_DEFINE_METHOD_FULL( SD_VARLINK_DEFINE_INPUT_BY_TYPE(pid, ProcessId, SD_VARLINK_NULLABLE), SD_VARLINK_FIELD_COMMENT("If non-null the cgroup of a unit"), SD_VARLINK_DEFINE_INPUT(cgroup, SD_VARLINK_STRING, SD_VARLINK_NULLABLE), + SD_VARLINK_FIELD_COMMENT("If non-null the invocation ID of a unit"), + SD_VARLINK_DEFINE_INPUT(invocationID, SD_VARLINK_STRING, SD_VARLINK_NULLABLE), SD_VARLINK_FIELD_COMMENT("Configuration of the unit"), SD_VARLINK_DEFINE_OUTPUT_BY_TYPE(context, UnitContext, 0), SD_VARLINK_FIELD_COMMENT("Runtime information of the unit"), From 8b1f7f13f0f7551fa635ee5d4d6c57981eee3610 Mon Sep 17 00:00:00 2001 From: Ivan Kruglov Date: Wed, 16 Jul 2025 06:03:50 -0700 Subject: [PATCH 5/6] test: lookup a unit by cgroup and invocationID in TEST-74-AUX-UTILS.varlinkctl --- test/units/TEST-74-AUX-UTILS.varlinkctl.sh | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/test/units/TEST-74-AUX-UTILS.varlinkctl.sh b/test/units/TEST-74-AUX-UTILS.varlinkctl.sh index b8983adaa2..6f5f90e1fe 100755 --- a/test/units/TEST-74-AUX-UTILS.varlinkctl.sh +++ b/test/units/TEST-74-AUX-UTILS.varlinkctl.sh @@ -190,11 +190,17 @@ varlinkctl info /run/systemd/io.systemd.Manager varlinkctl introspect /run/systemd/io.systemd.Manager io.systemd.Unit varlinkctl --more call /run/systemd/io.systemd.Manager io.systemd.Unit.List '{}' varlinkctl call /run/systemd/io.systemd.Manager io.systemd.Unit.List '{"name": "multi-user.target"}' -varlinkctl call /run/systemd/io.systemd.Manager io.systemd.Unit.List '{"pid": {"pid": 0}}' +varlinkctl call /run/systemd/io.systemd.Manager io.systemd.Unit.List '{"pid": {"pid": 1}}' (! varlinkctl call /run/systemd/io.systemd.Manager io.systemd.Unit.List '{}' |& grep -q "called without 'more' flag") +varlinkctl call /run/systemd/io.systemd.Manager io.systemd.Unit.List '{"name": "init.scope", "pid": {"pid": 1}}' (! varlinkctl call /run/systemd/io.systemd.Manager io.systemd.Unit.List '{"name": ""}') (! varlinkctl call /run/systemd/io.systemd.Manager io.systemd.Unit.List '{"name": "non-existent.service"}') (! varlinkctl call /run/systemd/io.systemd.Manager io.systemd.Unit.List '{"pid": {"pid": -1}}' ) +(! varlinkctl call /run/systemd/io.systemd.Manager io.systemd.Unit.List '{"name": "multi-user.target", "pid": {"pid": 1}}') + +varlinkctl call /run/systemd/io.systemd.Manager io.systemd.Unit.List '{"cgroup": "/init.scope"}' +invocation_id=$(varlinkctl call --collect /run/systemd/io.systemd.Manager io.systemd.Unit.List '{}' | jq -r '.[] | .runtime.InvocationID' | grep -v null | tail -n 1) +varlinkctl call /run/systemd/io.systemd.Manager io.systemd.Unit.List "{\"invocationID\": \"$invocation_id\"}" # test io.systemd.Manager in user manager testuser_uid=$(id -u testuser) From 959a8e49f4e2f5676a360f8b41f096c77ba029ce Mon Sep 17 00:00:00 2001 From: Ivan Kruglov Date: Wed, 16 Jul 2025 07:06:55 -0700 Subject: [PATCH 6/6] core: enable SELinux checks in io.systemd.Unit.List --- src/core/varlink-unit.c | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/src/core/varlink-unit.c b/src/core/varlink-unit.c index f71c3d7e33..b193753c01 100644 --- a/src/core/varlink-unit.c +++ b/src/core/varlink-unit.c @@ -12,6 +12,7 @@ #include "manager.h" #include "path-util.h" #include "pidref.h" +#include "selinux-access.h" #include "set.h" #include "strv.h" #include "unit.h" @@ -330,6 +331,21 @@ static int list_unit_one(sd_varlink *link, Unit *unit, bool more) { return sd_varlink_reply(link, v); } +static int list_unit_one_with_selinux_access_check(sd_varlink *link, Unit *unit, bool more) { + int r; + + assert(link); + assert(unit); + + r = mac_selinux_unit_access_check_varlink(unit, link, "status"); + if (r < 0) + /* If mac_selinux_unit_access_check_varlink() returned a error, + * it means that SELinux enforce is on. It also does all the logging(). */ + return sd_varlink_error(link, SD_VARLINK_ERROR_PERMISSION_DENIED, NULL); + + return list_unit_one(link, unit, more); +} + static int lookup_unit_by_pidref(sd_varlink *link, Manager *manager, PidRef *pidref, Unit **ret_unit) { _cleanup_(pidref_done) PidRef peer = PIDREF_NULL; Unit *unit; @@ -472,7 +488,7 @@ int vl_method_list_units(sd_varlink *link, sd_json_variant *parameters, sd_varli if (r < 0) return r; if (unit) - return list_unit_one(link, unit, /* more = */ false); + return list_unit_one_with_selinux_access_check(link, unit, /* more = */ false); if (!FLAGS_SET(flags, SD_VARLINK_METHOD_MORE)) return sd_varlink_error(link, SD_VARLINK_ERROR_EXPECTED_MORE, NULL);