machined: look up Varlink-registered machines in the right manager (#44270)

Properly use cg_pidref_get_unit_full() for the system service and
cg_pidref_get_user_unit_full() for the user service.

Previously, calling `machinectl kill` on an unprivileged
`systemd-nspawn` instance would call `KillUnitSubgroup()` on
`user@UID.service`.

The Varlink method also didn't record the cgroup path of machines in a
subgroup, so machined didn't watch it for becoming empty. The D-Bus
method handled both of these correctly, so this PR just factors out the
logic into a common helper function.

src/basic/cgroup-util.c

@@ -742,6 +742,29 @@ int cg_pid_get_path_shifted(pid_t pid, const char *root, char **ret_cgroup) {
         return strdup_to(ret_cgroup, c);
 }
 
+int cg_pidref_get_path_shifted(const PidRef *pidref, const char *root, char **ret_cgroup) {
+        _cleanup_free_ char *raw = NULL;
+        const char *c;
+        int r;
+
+        assert(ret_cgroup);
+
+        r = cg_pidref_get_path(pidref, &raw);
+        if (r < 0)
+                return r;
+
+        r = cg_shift_path(raw, root, &c);
+        if (r < 0)
+                return r;
+
+        if (c == raw) {
+                *ret_cgroup = TAKE_PTR(raw);
+                return 0;
+        }
+
+        return strdup_to(ret_cgroup, c);
+}
+
 int cg_path_decode_unit(const char *cgroup, char **ret_unit) {
         assert(cgroup);
 

src/basic/cgroup-util.h

@@ -196,6 +196,7 @@ int cg_path_get_user_slice(const char *path, char **ret_slice);
 
 int cg_shift_path(const char *cgroup, const char *cached_root, const char **ret_shifted);
 int cg_pid_get_path_shifted(pid_t pid, const char *cached_root, char **ret_cgroup);
+int cg_pidref_get_path_shifted(const PidRef *pidref, const char *cached_root, char **ret_cgroup);
 
 int cg_pid_get_session(pid_t pid, char **ret_session);
 int cg_pidref_get_session(const PidRef *pidref, char **ret);

src/coredump/coredump-context.c

@@ -303,7 +303,7 @@ int coredump_context_build_iovw(CoredumpContext *context) {
         if (pidref_get_cmdline(&context->pidref, SIZE_MAX, PROCESS_CMDLINE_QUOTE_POSIX, &t) >= 0)
                 (void) iovw_put_string_field_free(&context->iovw, "COREDUMP_CMDLINE=", t);
 
-        if (cg_pid_get_path_shifted(pid, NULL, &t) >= 0)
+        if (cg_pidref_get_path_shifted(&context->pidref, NULL, &t) >= 0)
                 (void) iovw_put_string_field_free(&context->iovw, "COREDUMP_CGROUP=", t);
 
         if (compose_open_fds(pid, &t) >= 0)

src/machine/machine-varlink.c

@@ -215,7 +215,7 @@ int vl_method_register(sd_varlink *link, sd_json_variant *parameters, sd_varlink
                 return r;
 
         if (!machine->allocate_unit) {
-                r = cg_pidref_get_unit_full(&machine->leader, &machine->unit, &machine->subgroup);
+                r = machine_determine_unit(machine);
                 if (r < 0)
                         return r;
         }

src/machine/machine.c

@@ -13,6 +13,7 @@
 #include "bus-internal.h"
 #include "bus-locator.h"
 #include "bus-unit-util.h"
+#include "cgroup-util.h"
 #include "env-file.h"
 #include "errno-util.h"
 #include "extract-word.h"
@@ -630,6 +631,30 @@ static int machine_watch_cgroup(Machine *m) {
         return 0;
 }
 
+int machine_determine_unit(Machine *m) {
+        int r;
+
+        assert(m);
+        assert(m->manager);
+        assert(!m->unit);
+        assert(!m->subgroup);
+        assert(!m->cgroup);
+
+        r = manager_get_unit_by_pidref(m->manager, &m->leader, &m->unit, &m->subgroup);
+        if (r < 0)
+                return log_debug_errno(r, "Failed to determine unit of process " PID_FMT ": %m", m->leader.pid);
+
+        if (!empty_or_root(m->subgroup)) {
+                /* If this is not a top-level cgroup, then we need the cgroup path to be able to watch when
+                 * it empties */
+                r = cg_pidref_get_path(&m->leader, &m->cgroup);
+                if (r < 0)
+                        return log_debug_errno(r, "Failed to determine cgroup of process " PID_FMT ": %m", m->leader.pid);
+        }
+
+        return 0;
+}
+
 int machine_start(Machine *m, sd_bus_message *properties, sd_bus_error *error) {
         int r;
 

src/machine/machine.h

@@ -108,6 +108,7 @@ int machine_link(Manager *manager, Machine *machine);
 Machine* machine_free(Machine *m);
 bool machine_may_gc(Machine *m, bool drop_not_started);
 void machine_add_to_gc_queue(Machine *m);
+int machine_determine_unit(Machine *m);
 int machine_start(Machine *m, sd_bus_message *properties, sd_bus_error *error);
 int machine_stop(Machine *m);
 int machine_finalize(Machine *m);

src/machine/machined-core.c

@@ -21,6 +21,64 @@
 #include "strv.h"
 #include "user-util.h"
 
+static int user_manager_get_unit_by_pidref(const PidRef *pidref, char **ret_unit, char **ret_subgroup) {
+        int r;
+
+        assert(pidref);
+
+        _cleanup_free_ char *cgroup = NULL;
+        r = cg_pidref_get_path_shifted(pidref, /* cached_root= */ NULL, &cgroup);
+        if (r < 0)
+                return r;
+
+        /* cg_path_get_user_unit_full() strips any user manager prefix, hence make sure the process
+         * actually belongs to our user manager, and not to some other user's that happens to have a unit
+         * of the same name. */
+        _cleanup_free_ char *manager_unit = NULL;
+        r = cg_path_get_unit(cgroup, &manager_unit);
+        if (r < 0)
+                return r;
+
+        /* The system unit we are running in, i.e. our own user@.service or capsule@.service */
+        _cleanup_free_ char *our_manager_unit = NULL;
+        r = cg_pid_get_unit(0, &our_manager_unit);
+        if (r < 0)
+                return r;
+
+        if (!streq(manager_unit, our_manager_unit))
+                return -ENXIO;
+
+        _cleanup_free_ char *unit = NULL, *subgroup = NULL;
+        r = cg_path_get_user_unit_full(cgroup, &unit, &subgroup);
+        if (r < 0)
+                return r;
+
+        if (ret_unit)
+                *ret_unit = TAKE_PTR(unit);
+        if (ret_subgroup)
+                *ret_subgroup = TAKE_PTR(subgroup);
+        return 0;
+}
+
+int manager_get_unit_by_pidref(Manager *m, const PidRef *pidref, char **ret_unit, char **ret_subgroup) {
+        assert(m);
+        assert(pidref);
+
+        /* Determines the unit of the specified process in the service manager we are running for */
+
+        switch (m->runtime_scope) {
+
+        case RUNTIME_SCOPE_USER:
+                return user_manager_get_unit_by_pidref(pidref, ret_unit, ret_subgroup);
+
+        case RUNTIME_SCOPE_SYSTEM:
+                return cg_pidref_get_unit_full(pidref, ret_unit, ret_subgroup);
+
+        default:
+                assert_not_reached();
+        }
+}
+
 int manager_get_machine_by_pidref(Manager *m, const PidRef *pidref, Machine **ret) {
         _cleanup_(pidref_done) PidRef current = PIDREF_NULL;
         Machine *mm = NULL;
@@ -37,7 +95,7 @@ int manager_get_machine_by_pidref(Manager *m, const PidRef *pidref, Machine **re
 
                 /* Then look for the unit */
                 _cleanup_free_ char *unit = NULL;
-                if (cg_pidref_get_unit(pidref, &unit) >= 0) {
+                if (manager_get_unit_by_pidref(m, pidref, &unit, /* ret_subgroup= */ NULL) >= 0) {
                         mm = hashmap_get(m->machines_by_unit, unit);
                         if (mm)
                                 break;

src/machine/machined-dbus.c

@@ -12,7 +12,6 @@
 #include "bus-object.h"
 #include "bus-polkit.h"
 #include "bus-util.h"
-#include "cgroup-util.h"
 #include "discover-image.h"
 #include "errno-util.h"
 #include "fd-util.h"
@@ -730,38 +729,14 @@ static int method_register_machine(sd_bus_message *message, void *userdata, sd_b
         if (r == 0)
                 return 1; /* Will call us back */
 
-        switch (manager->runtime_scope) {
-        case RUNTIME_SCOPE_USER:
-                r = cg_pidref_get_user_unit_full(&m->leader, &m->unit, &m->subgroup);
-                break;
-
-        case RUNTIME_SCOPE_SYSTEM:
-                r = cg_pidref_get_unit_full(&m->leader, &m->unit, &m->subgroup);
-                break;
-
-        default:
-                assert_not_reached();
-        }
+        r = machine_determine_unit(m);
         if (r < 0) {
                 r = sd_bus_error_set_errnof(error, r,
-                                            "Failed to determine unit of process "PID_FMT" : %m",
+                                            "Failed to determine unit or cgroup of process "PID_FMT": %m",
                                             m->leader.pid);
                 goto fail;
         }
 
-        if (!empty_or_root(m->subgroup)) {
-                /* If this is not a top-level cgroup, then we need the cgroup path to be able to watch when
-                 * it empties */
-
-                r = cg_pidref_get_path(&m->leader, &m->cgroup);
-                if (r < 0) {
-                        r = sd_bus_error_set_errnof(error, r,
-                                                    "Failed to determine cgroup of process "PID_FMT" : %m",
-                                                    m->leader.pid);
-                        goto fail;
-                }
-        }
-
         r = machine_start(m, NULL, error);
         if (r < 0)
                 goto fail;

src/machine/machined.h

@@ -40,6 +40,7 @@ typedef struct Manager {
 } Manager;
 
 int manager_add_machine(Manager *m, const char *name, Machine **ret);
+int manager_get_unit_by_pidref(Manager *m, const PidRef *pidref, char **ret_unit, char **ret_subgroup);
 int manager_get_machine_by_pidref(Manager *m, const PidRef *pidref, Machine **ret);
 
 int match_reloading(sd_bus_message *message, void *userdata, sd_bus_error *error);

test/units/TEST-13-NSPAWN.unpriv.sh

@@ -69,12 +69,19 @@ loginctl enable-linger testuser
 
 run0 -u testuser mkdir -p .config/systemd/nspawn/
 run0 -u testuser -i "echo -e \"[Exec]\nKillSignal=SIGKILL\n\" >.config/systemd/nspawn/zurps.nspawn"
+run0 -u testuser systemctl start --user systemd-machined.socket
 run0 -u testuser systemctl start --user systemd-nspawn@zurps.service
 
 machinectl status zurps
 machinectl status zurps | grep "UID:" | grep "$(id -u testuser)"
 machinectl status zurps | grep "Unit: user@" | grep "$(id -u testuser)"
 machinectl status zurps | grep "Subgroup: machine.slice/systemd-nspawn@zurps.service/payload"
+
+# The per-user machined refers to the unit in the user's service manager
+run0 -u testuser machinectl --user status zurps
+run0 -u testuser machinectl --user status zurps | grep "Unit: systemd-nspawn@zurps.service"
+run0 -u testuser machinectl --user status zurps | grep "Subgroup: payload"
+run0 -u testuser machinectl --user kill --signal=SIGCONT zurps
 machinectl terminate zurps
 
 (! run0 -u testuser systemctl is-active --user systemd-nspawn@zurps.service)