From 98fc46f2a61c4ba9b07547aec983cf8365c17d8f Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 19 May 2025 17:57:08 +0200 Subject: [PATCH 01/12] logind,machined: expose bus properties for leader PID fd ids, too --- man/org.freedesktop.login1.xml | 8 ++++++++ man/org.freedesktop.machine1.xml | 8 ++++++++ src/login/logind-session-dbus.c | 1 + src/machine/machine-dbus.c | 3 ++- src/shared/bus-get-properties.c | 21 +++++++++++++++++++++ src/shared/bus-get-properties.h | 2 ++ 6 files changed, 42 insertions(+), 1 deletion(-) diff --git a/man/org.freedesktop.login1.xml b/man/org.freedesktop.login1.xml index 32734b01c0..c344cb3014 100644 --- a/man/org.freedesktop.login1.xml +++ b/man/org.freedesktop.login1.xml @@ -1251,6 +1251,8 @@ node /org/freedesktop/login1/session/1 { @org.freedesktop.DBus.Property.EmitsChangedSignal("const") readonly u Leader = ...; @org.freedesktop.DBus.Property.EmitsChangedSignal("const") + readonly t LeaderPIDFDId = ...; + @org.freedesktop.DBus.Property.EmitsChangedSignal("const") readonly u Audit = ...; readonly s Type = '...'; readonly s Class = '...'; @@ -1351,6 +1353,8 @@ node /org/freedesktop/login1/session/1 { + + @@ -1539,6 +1543,9 @@ node /org/freedesktop/login1/session/1 { Leader encodes the PID of the process that registered the session. + LeaderPIDFDId encodes the Linux pidfd inode ID of the process that registered + the session. + Audit encodes the Kernel Audit session ID of the session if auditing is available. @@ -1660,6 +1667,7 @@ node /org/freedesktop/login1/session/1 { SetDisplay() was added in version 252. SetTTY() was added in version 254. SetClass() was added in version 256. + LeaderPIDFDId was added in version 258. diff --git a/man/org.freedesktop.machine1.xml b/man/org.freedesktop.machine1.xml index 7792320546..936f2ad7f2 100644 --- a/man/org.freedesktop.machine1.xml +++ b/man/org.freedesktop.machine1.xml @@ -513,6 +513,8 @@ node /org/freedesktop/machine1/machine/rawhide { @org.freedesktop.DBus.Property.EmitsChangedSignal("const") readonly u Leader = ...; @org.freedesktop.DBus.Property.EmitsChangedSignal("const") + readonly t LeaderPIDFDId = ...; + @org.freedesktop.DBus.Property.EmitsChangedSignal("const") readonly s Class = '...'; @org.freedesktop.DBus.Property.EmitsChangedSignal("const") readonly s RootDirectory = '...'; @@ -603,6 +605,8 @@ node /org/freedesktop/machine1/machine/rawhide { + + @@ -650,6 +654,9 @@ node /org/freedesktop/machine1/machine/rawhide { Leader is the PID of the leader process of the machine. + LeaderPIDFDId encodes the Linux pidfd inode ID of the leader process of the + machine. + Class is the class of the machine and is either the string "vm" (for real VMs based on virtualized hardware) or "container" (for lightweight userspace virtualization sharing the same kernel as the host). @@ -717,6 +724,7 @@ $ gdbus introspect --system \ CopyToWithFlags() were added in version 252. GetSSHInfo(), VSockCID, SSHAddress and SSHPrivateKeyPath were added in version 256. + LeaderPIDFDId was added in version 258. diff --git a/src/login/logind-session-dbus.c b/src/login/logind-session-dbus.c index 0277f26fe5..70e7fa63c4 100644 --- a/src/login/logind-session-dbus.c +++ b/src/login/logind-session-dbus.c @@ -993,6 +993,7 @@ static const sd_bus_vtable session_vtable[] = { SD_BUS_PROPERTY("Desktop", "s", NULL, offsetof(Session, desktop), SD_BUS_VTABLE_PROPERTY_CONST), SD_BUS_PROPERTY("Scope", "s", NULL, offsetof(Session, scope), SD_BUS_VTABLE_PROPERTY_CONST), SD_BUS_PROPERTY("Leader", "u", bus_property_get_pid, offsetof(Session, leader.pid), SD_BUS_VTABLE_PROPERTY_CONST), + SD_BUS_PROPERTY("LeaderPIDFDId", "t", bus_property_get_pidfdid, offsetof(Session, leader), SD_BUS_VTABLE_PROPERTY_CONST), SD_BUS_PROPERTY("Audit", "u", NULL, offsetof(Session, audit_id), SD_BUS_VTABLE_PROPERTY_CONST), SD_BUS_PROPERTY("Type", "s", property_get_type, offsetof(Session, type), SD_BUS_VTABLE_PROPERTY_EMITS_CHANGE), SD_BUS_PROPERTY("Class", "s", property_get_class, offsetof(Session, class), SD_BUS_VTABLE_PROPERTY_EMITS_CHANGE), diff --git a/src/machine/machine-dbus.c b/src/machine/machine-dbus.c index c19b1a5beb..c299bf71fa 100644 --- a/src/machine/machine-dbus.c +++ b/src/machine/machine-dbus.c @@ -717,7 +717,8 @@ static const sd_bus_vtable machine_vtable[] = { SD_BUS_PROPERTY("Service", "s", NULL, offsetof(Machine, service), SD_BUS_VTABLE_PROPERTY_CONST), SD_BUS_PROPERTY("Unit", "s", NULL, offsetof(Machine, unit), SD_BUS_VTABLE_PROPERTY_CONST), SD_BUS_PROPERTY("Scope", "s", NULL, offsetof(Machine, unit), SD_BUS_VTABLE_PROPERTY_CONST|SD_BUS_VTABLE_HIDDEN), - SD_BUS_PROPERTY("Leader", "u", NULL, offsetof(Machine, leader.pid), SD_BUS_VTABLE_PROPERTY_CONST), + SD_BUS_PROPERTY("Leader", "u", bus_property_get_pid, offsetof(Machine, leader.pid), SD_BUS_VTABLE_PROPERTY_CONST), + SD_BUS_PROPERTY("LeaderPIDFDId", "t", bus_property_get_pidfdid, offsetof(Machine, leader), SD_BUS_VTABLE_PROPERTY_CONST), SD_BUS_PROPERTY("Class", "s", property_get_class, offsetof(Machine, class), SD_BUS_VTABLE_PROPERTY_CONST), SD_BUS_PROPERTY("RootDirectory", "s", NULL, offsetof(Machine, root_directory), SD_BUS_VTABLE_PROPERTY_CONST), SD_BUS_PROPERTY("NetworkInterfaces", "ai", property_get_netif, 0, SD_BUS_VTABLE_PROPERTY_CONST), diff --git a/src/shared/bus-get-properties.c b/src/shared/bus-get-properties.c index e21eafc4ba..099cfa77f7 100644 --- a/src/shared/bus-get-properties.c +++ b/src/shared/bus-get-properties.c @@ -4,6 +4,7 @@ #include "bus-get-properties.h" #include "bus-message-util.h" +#include "pidref.h" #include "rlimit-util.h" #include "string-util.h" @@ -188,3 +189,23 @@ int bus_property_get_string_set( return bus_message_append_string_set(reply, *s); } + +int bus_property_get_pidfdid( + sd_bus *bus, + const char *path, + const char *interface, + const char *property, + sd_bus_message *reply, + void *userdata, + sd_bus_error *error) { + + PidRef *pidref = ASSERT_PTR(userdata); + + assert(bus); + assert(property); + assert(reply); + + (void) pidref_acquire_pidfd_id(pidref); + + return sd_bus_message_append(reply, "t", pidref->fd_id); +} diff --git a/src/shared/bus-get-properties.h b/src/shared/bus-get-properties.h index a77d3653be..3f445f0acd 100644 --- a/src/shared/bus-get-properties.h +++ b/src/shared/bus-get-properties.h @@ -54,6 +54,8 @@ int bus_property_get_rlimit(sd_bus *bus, const char *path, const char *interface 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_property_get_pidfdid(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, \ From 0c18c0deeee8e638ef70a5b9c4611c7ebed3a74b Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Thu, 15 May 2025 12:25:47 +0200 Subject: [PATCH 02/12] machine: insist in a valid root directory --- src/machine/machined-dbus.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/machine/machined-dbus.c b/src/machine/machined-dbus.c index 014c026763..900819c37f 100644 --- a/src/machine/machined-dbus.c +++ b/src/machine/machined-dbus.c @@ -284,7 +284,7 @@ static int method_create_or_register_machine( if (leader == 1) return sd_bus_error_set(error, SD_BUS_ERROR_INVALID_ARGS, "Invalid leader PID"); - if (!isempty(root_directory) && !path_is_absolute(root_directory)) + if (!isempty(root_directory) && (!path_is_absolute(root_directory) || !path_is_valid(root_directory))) return sd_bus_error_set(error, SD_BUS_ERROR_INVALID_ARGS, "Root directory must be empty or an absolute path"); if (leader == 0) { From a4dc3b16d9e4297faf577e61867d6eea9df72a58 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 19 May 2025 10:58:16 +0200 Subject: [PATCH 03/12] machine: port machined state files to fopen_tmpfile_linkable() Similar to the erlier commit for logind, switch to a more modern way to write the state files. --- src/machine/machine.c | 47 +++++++++++++++---------------------------- 1 file changed, 16 insertions(+), 31 deletions(-) diff --git a/src/machine/machine.c b/src/machine/machine.c index 9bee46a10e..989d6a5c22 100644 --- a/src/machine/machine.c +++ b/src/machine/machine.c @@ -145,8 +145,6 @@ Machine* machine_free(Machine *m) { } int machine_save(Machine *m) { - _cleanup_(unlink_and_freep) char *temp_path = NULL; - _cleanup_fclose_ FILE *f = NULL; int r; assert(m); @@ -159,13 +157,16 @@ int machine_save(Machine *m) { r = mkdir_safe_label("/run/systemd/machines", 0755, 0, 0, MKDIR_WARN_MODE); if (r < 0) - goto fail; + return log_error_errno(r, "Failed to create /run/systemd/machines/: %m"); - r = fopen_temporary(m->state_file, &f, &temp_path); + _cleanup_(unlink_and_freep) char *temp_path = NULL; + _cleanup_fclose_ FILE *f = NULL; + r = fopen_tmpfile_linkable(m->state_file, O_WRONLY|O_CLOEXEC, &temp_path, &f); if (r < 0) - goto fail; + return log_error_errno(r, "Failed to create state file '%s': %m", m->state_file); - (void) fchmod(fileno(f), 0644); + if (fchmod(fileno(f), 0644) < 0) + return log_error_errno(errno, "Failed to set access mode for state file '%s' to 0644: %m", m->state_file); fprintf(f, "# This is private data. Do not parse.\n" @@ -176,10 +177,8 @@ int machine_save(Machine *m) { _cleanup_free_ char *escaped = NULL; escaped = cescape(m->unit); - if (!escaped) { - r = -ENOMEM; - goto fail; - } + if (!escaped) + return log_oom(); fprintf(f, "SCOPE=%s\n", escaped); /* We continue to call this "SCOPE=" because it is internal only, and we want to stay compatible with old files */ } @@ -191,10 +190,8 @@ int machine_save(Machine *m) { _cleanup_free_ char *escaped = NULL; escaped = cescape(m->service); - if (!escaped) { - r = -ENOMEM; - goto fail; - } + if (!escaped) + return log_oom(); fprintf(f, "SERVICE=%s\n", escaped); } @@ -202,10 +199,8 @@ int machine_save(Machine *m) { _cleanup_free_ char *escaped = NULL; escaped = cescape(m->root_directory); - if (!escaped) { - r = -ENOMEM; - goto fail; - } + if (!escaped) + return log_oom(); fprintf(f, "ROOT=%s\n", escaped); } @@ -240,16 +235,11 @@ int machine_save(Machine *m) { fputc('\n', f); } - r = fflush_and_check(f); + r = flink_tmpfile(f, temp_path, m->state_file, LINK_TMPFILE_REPLACE); if (r < 0) - goto fail; + return log_error_errno(r, "Failed to move '%s' into place: %m", m->state_file); - if (rename(temp_path, m->state_file) < 0) { - r = -errno; - goto fail; - } - - temp_path = mfree(temp_path); + temp_path = mfree(temp_path); /* disarm auto-destroy: temporary file does not exist anymore */ if (m->unit) { char *sl; @@ -262,11 +252,6 @@ int machine_save(Machine *m) { } return 0; - -fail: - (void) unlink(m->state_file); - - return log_error_errno(r, "Failed to save machine data %s: %m", m->state_file); } static void machine_unlink(Machine *m) { From b8396f1102bd27c1ee7dfe6cb4459c4c43e539e6 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 19 May 2025 11:05:51 +0200 Subject: [PATCH 04/12] machine: properly remove unit name symlink on removal --- src/machine/machine.c | 24 ++++++++++++++---------- 1 file changed, 14 insertions(+), 10 deletions(-) diff --git a/src/machine/machine.c b/src/machine/machine.c index 989d6a5c22..fb397362b1 100644 --- a/src/machine/machine.c +++ b/src/machine/machine.c @@ -155,6 +155,13 @@ int machine_save(Machine *m) { if (!m->started) return 0; + _cleanup_(unlink_and_freep) char *sl = NULL; /* auto-unlink! */ + if (m->unit) { + sl = strjoin("/run/systemd/machines/unit:", m->unit); + if (!sl) + return log_oom(); + } + r = mkdir_safe_label("/run/systemd/machines", 0755, 0, 0, MKDIR_WARN_MODE); if (r < 0) return log_error_errno(r, "Failed to create /run/systemd/machines/: %m"); @@ -241,14 +248,13 @@ int machine_save(Machine *m) { temp_path = mfree(temp_path); /* disarm auto-destroy: temporary file does not exist anymore */ - if (m->unit) { - char *sl; - - /* Create a symlink from the unit name to the machine - * name, so that we can quickly find the machine for - * each given unit. Ignore error. */ - sl = strjoina("/run/systemd/machines/unit:", m->unit); + if (sl) { + /* Create a symlink from the unit name to the machine name, so that we can quickly find the machine + * for each given unit. Ignore error. */ (void) symlink(m->name, sl); + + /* disarm auto-removal */ + sl = mfree(sl); } return 0; @@ -258,9 +264,7 @@ static void machine_unlink(Machine *m) { assert(m); if (m->unit) { - char *sl; - - sl = strjoina("/run/systemd/machines/unit:", m->unit); + const char *sl = strjoina("/run/systemd/machines/unit:", m->unit); (void) unlink(sl); } From 4b9c91843155045e9933fe93d36b2fdd207891f2 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 19 May 2025 11:07:53 +0200 Subject: [PATCH 05/12] machine: use the correct escaping calls for machine metadata --- src/libsystemd/sd-login/sd-login.c | 16 +------------- src/machine/machine.c | 34 +++++------------------------- 2 files changed, 6 insertions(+), 44 deletions(-) diff --git a/src/libsystemd/sd-login/sd-login.c b/src/libsystemd/sd-login/sd-login.c index efe40c70f5..f49713597a 100644 --- a/src/libsystemd/sd-login/sd-login.c +++ b/src/libsystemd/sd-login/sd-login.c @@ -10,7 +10,6 @@ #include "cgroup-util.h" #include "dirent-util.h" #include "env-file.h" -#include "escape.h" #include "extract-word.h" #include "fd-util.h" #include "format-util.h" @@ -837,20 +836,7 @@ _public_ int sd_session_get_class(const char *session, char **class) { } _public_ int sd_session_get_desktop(const char *session, char **desktop) { - _cleanup_free_ char *escaped = NULL; - int r; - ssize_t l; - - assert_return(desktop, -EINVAL); - - r = session_get_string(session, "DESKTOP", &escaped); - if (r < 0) - return r; - - l = cunescape(escaped, 0, desktop); - if (l < 0) - return l; - return 0; + return session_get_string(session, "DESKTOP", desktop); } _public_ int sd_session_get_display(const char *session, char **display) { diff --git a/src/machine/machine.c b/src/machine/machine.c index fb397362b1..0d5818439d 100644 --- a/src/machine/machine.c +++ b/src/machine/machine.c @@ -180,36 +180,12 @@ int machine_save(Machine *m) { "NAME=%s\n", m->name); - if (m->unit) { - _cleanup_free_ char *escaped = NULL; + /* We continue to call this "SCOPE=" because it is internal only, and we want to stay compatible with old files */ + env_file_fputs_assignment(f, "SCOPE=", m->unit); + env_file_fputs_assignment(f, "SCOPE_JOB=", m->scope_job); - escaped = cescape(m->unit); - if (!escaped) - return log_oom(); - - fprintf(f, "SCOPE=%s\n", escaped); /* We continue to call this "SCOPE=" because it is internal only, and we want to stay compatible with old files */ - } - - if (m->scope_job) - fprintf(f, "SCOPE_JOB=%s\n", m->scope_job); - - if (m->service) { - _cleanup_free_ char *escaped = NULL; - - escaped = cescape(m->service); - if (!escaped) - return log_oom(); - fprintf(f, "SERVICE=%s\n", escaped); - } - - if (m->root_directory) { - _cleanup_free_ char *escaped = NULL; - - escaped = cescape(m->root_directory); - if (!escaped) - return log_oom(); - fprintf(f, "ROOT=%s\n", escaped); - } + env_file_fputs_assignment(f, "SERVICE=", m->service); + env_file_fputs_assignment(f, "ROOT=", m->root_directory); if (!sd_id128_is_null(m->id)) fprintf(f, "ID=" SD_ID128_FORMAT_STR "\n", SD_ID128_FORMAT_VAL(m->id)); From 20babd603814e893cd6caed2604adda67070f950 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 19 May 2025 11:08:52 +0200 Subject: [PATCH 06/12] machine: save/restore machine leader pidfdid --- src/machine/machine.c | 45 ++++++++++++++++++++++++++++++------------- 1 file changed, 32 insertions(+), 13 deletions(-) diff --git a/src/machine/machine.c b/src/machine/machine.c index 0d5818439d..51dfb77c36 100644 --- a/src/machine/machine.c +++ b/src/machine/machine.c @@ -190,8 +190,12 @@ int machine_save(Machine *m) { if (!sd_id128_is_null(m->id)) fprintf(f, "ID=" SD_ID128_FORMAT_STR "\n", SD_ID128_FORMAT_VAL(m->id)); - if (pidref_is_set(&m->leader)) + if (pidref_is_set(&m->leader)) { fprintf(f, "LEADER="PID_FMT"\n", m->leader.pid); + (void) pidref_acquire_pidfd_id(&m->leader); + if (m->leader.fd_id != 0) + fprintf(f, "LEADER_PIDFDID=%" PRIu64 "\n", m->leader.fd_id); + } if (m->class != _MACHINE_CLASS_INVALID) fprintf(f, "CLASS=%s\n", machine_class_to_string(m->class)); @@ -249,7 +253,7 @@ static void machine_unlink(Machine *m) { } int machine_load(Machine *m) { - _cleanup_free_ char *realtime = NULL, *monotonic = NULL, *id = NULL, *leader = NULL, *class = NULL, *netif = NULL; + _cleanup_free_ char *realtime = NULL, *monotonic = NULL, *id = NULL, *leader = NULL, *leader_pidfdid = NULL, *class = NULL, *netif = NULL; int r; assert(m); @@ -258,16 +262,17 @@ int machine_load(Machine *m) { return 0; r = parse_env_file(NULL, m->state_file, - "SCOPE", &m->unit, - "SCOPE_JOB", &m->scope_job, - "SERVICE", &m->service, - "ROOT", &m->root_directory, - "ID", &id, - "LEADER", &leader, - "CLASS", &class, - "REALTIME", &realtime, - "MONOTONIC", &monotonic, - "NETIF", &netif); + "SCOPE", &m->unit, + "SCOPE_JOB", &m->scope_job, + "SERVICE", &m->service, + "ROOT", &m->root_directory, + "ID", &id, + "LEADER", &leader, + "LEADER_PIDFDID", &leader_pidfdid, + "CLASS", &class, + "REALTIME", &realtime, + "MONOTONIC", &monotonic, + "NETIF", &netif); if (r == -ENOENT) return 0; if (r < 0) @@ -276,11 +281,25 @@ int machine_load(Machine *m) { if (id) (void) sd_id128_from_string(id, &m->id); + pidref_done(&m->leader); if (leader) { - pidref_done(&m->leader); r = pidref_set_pidstr(&m->leader, leader); if (r < 0) log_debug_errno(r, "Failed to set leader PID to '%s', ignoring: %m", leader); + else if (leader_pidfdid) { + uint64_t fd_id; + r = safe_atou64(leader_pidfdid, &fd_id); + if (r < 0) + log_warning_errno(r, "Failed to parse leader pidfd ID, ignoring: %s", leader_pidfdid); + else { + (void) pidref_acquire_pidfd_id(&m->leader); + + if (fd_id != m->leader.fd_id) { + log_debug("Leader PID got recycled, ignoring."); + pidref_done(&m->leader); + } + } + } } if (class) { From 320b370ab36e656da7f2b8bf41c2c45965c6bdf6 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 19 May 2025 11:19:17 +0200 Subject: [PATCH 07/12] machine: modernizations of serializing/deserializing netif data --- src/machine/machine.c | 23 +++++++++-------------- 1 file changed, 9 insertions(+), 14 deletions(-) diff --git a/src/machine/machine.c b/src/machine/machine.c index 51dfb77c36..c52883d9c2 100644 --- a/src/machine/machine.c +++ b/src/machine/machine.c @@ -208,18 +208,13 @@ int machine_save(Machine *m) { m->timestamp.monotonic); if (m->n_netif > 0) { - size_t i; - - fputs("NETIF=", f); - - for (i = 0; i < m->n_netif; i++) { - if (i != 0) + fputs("NETIF=\"", f); + FOREACH_ARRAY(ifi, m->netif, m->n_netif) { + if (*ifi != 0) fputc(' ', f); - - fprintf(f, "%i", m->netif[i]); + fprintf(f, "%i", *ifi); } - - fputc('\n', f); + fputs("\"\n", f); } r = flink_tmpfile(f, temp_path, m->state_file, LINK_TMPFILE_REPLACE); @@ -315,13 +310,13 @@ int machine_load(Machine *m) { if (monotonic) (void) deserialize_usec(monotonic, &m->timestamp.monotonic); + m->netif = mfree(m->netif); + m->n_netif = 0; if (netif) { _cleanup_free_ int *ni = NULL; size_t nr = 0; - const char *p; - p = netif; - for (;;) { + for (const char *p = netif;;) { _cleanup_free_ char *word = NULL; r = extract_first_word(&p, &word, NULL, 0); @@ -344,7 +339,7 @@ int machine_load(Machine *m) { ni[nr++] = r; } - free_and_replace(m->netif, ni); + m->netif = TAKE_PTR(ni); m->n_netif = nr; } From 6981e465a76fa392413aadd2922efc5751d64a97 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 19 May 2025 11:19:33 +0200 Subject: [PATCH 08/12] machine: also save/restore vsock CID properly --- src/machine/machine.c | 41 +++++++++++++++++++++++++++++------------ 1 file changed, 29 insertions(+), 12 deletions(-) diff --git a/src/machine/machine.c b/src/machine/machine.c index c52883d9c2..e59ee25715 100644 --- a/src/machine/machine.c +++ b/src/machine/machine.c @@ -217,6 +217,12 @@ int machine_save(Machine *m) { fputs("\"\n", f); } + if (m->vsock_cid != 0) + fprintf(f, "VSOCK_CID=%u\n", m->vsock_cid); + + env_file_fputs_assignment(f, "SSH_ADDRESS=", m->ssh_address); + env_file_fputs_assignment(f, "SSH_PRIVATE_KEY_PATH=", m->ssh_private_key_path); + r = flink_tmpfile(f, temp_path, m->state_file, LINK_TMPFILE_REPLACE); if (r < 0) return log_error_errno(r, "Failed to move '%s' into place: %m", m->state_file); @@ -248,7 +254,8 @@ static void machine_unlink(Machine *m) { } int machine_load(Machine *m) { - _cleanup_free_ char *realtime = NULL, *monotonic = NULL, *id = NULL, *leader = NULL, *leader_pidfdid = NULL, *class = NULL, *netif = NULL; + _cleanup_free_ char *realtime = NULL, *monotonic = NULL, *id = NULL, *leader = NULL, *leader_pidfdid = NULL, + *class = NULL, *netif = NULL, *vsock_cid = NULL; int r; assert(m); @@ -257,17 +264,20 @@ int machine_load(Machine *m) { return 0; r = parse_env_file(NULL, m->state_file, - "SCOPE", &m->unit, - "SCOPE_JOB", &m->scope_job, - "SERVICE", &m->service, - "ROOT", &m->root_directory, - "ID", &id, - "LEADER", &leader, - "LEADER_PIDFDID", &leader_pidfdid, - "CLASS", &class, - "REALTIME", &realtime, - "MONOTONIC", &monotonic, - "NETIF", &netif); + "SCOPE", &m->unit, + "SCOPE_JOB", &m->scope_job, + "SERVICE", &m->service, + "ROOT", &m->root_directory, + "ID", &id, + "LEADER", &leader, + "LEADER_PIDFDID", &leader_pidfdid, + "CLASS", &class, + "REALTIME", &realtime, + "MONOTONIC", &monotonic, + "NETIF", &netif, + "VSOCK_CID", &vsock_cid, + "SSH_ADDRESS", &m->ssh_address, + "SSH_PRIVATE_KEY_PATH", &m->ssh_private_key_path); if (r == -ENOENT) return 0; if (r < 0) @@ -343,6 +353,13 @@ int machine_load(Machine *m) { m->n_netif = nr; } + m->vsock_cid = 0; + if (vsock_cid) { + r = safe_atou(vsock_cid, &m->vsock_cid); + if (r < 0) + log_warning_errno(r, "Failed to parse AF_VSOCK CID, ignoring: %s", vsock_cid); + } + return r; } From ee5622f9f03d3c2ab11ae66131a82d5e7906910c Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 19 May 2025 11:12:14 +0200 Subject: [PATCH 09/12] machine: as safety precaution also check parsed machine name --- src/machine/machine.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/machine/machine.c b/src/machine/machine.c index e59ee25715..377e01484b 100644 --- a/src/machine/machine.c +++ b/src/machine/machine.c @@ -254,7 +254,7 @@ static void machine_unlink(Machine *m) { } int machine_load(Machine *m) { - _cleanup_free_ char *realtime = NULL, *monotonic = NULL, *id = NULL, *leader = NULL, *leader_pidfdid = NULL, + _cleanup_free_ char *name = NULL, *realtime = NULL, *monotonic = NULL, *id = NULL, *leader = NULL, *leader_pidfdid = NULL, *class = NULL, *netif = NULL, *vsock_cid = NULL; int r; @@ -264,6 +264,7 @@ int machine_load(Machine *m) { return 0; r = parse_env_file(NULL, m->state_file, + "NAME", &name, "SCOPE", &m->unit, "SCOPE_JOB", &m->scope_job, "SERVICE", &m->service, @@ -283,6 +284,9 @@ int machine_load(Machine *m) { if (r < 0) return log_error_errno(r, "Failed to read %s: %m", m->state_file); + if (!streq_ptr(name, m->name)) + return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "State file '%s' for machine '%s' reports a different name '%s', refusing", m->state_file, m->name, name); + if (id) (void) sd_id128_from_string(id, &m->id); From 0c38bc6227627dffc87e77f37dc72992b1ed0d5b Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 19 May 2025 11:12:35 +0200 Subject: [PATCH 10/12] machine: shorten code --- src/machine/machine.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/machine/machine.c b/src/machine/machine.c index 377e01484b..cf91eef029 100644 --- a/src/machine/machine.c +++ b/src/machine/machine.c @@ -312,9 +312,7 @@ int machine_load(Machine *m) { } if (class) { - MachineClass c; - - c = machine_class_from_string(class); + MachineClass c = machine_class_from_string(class); if (c >= 0) m->class = c; } From ca02f658e0800d43a3692271e4a6b2ee44e01727 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 19 May 2025 13:24:14 +0200 Subject: [PATCH 11/12] machine: rework machine_gc() Let's check the leader alive state, and let's log about dbus errors. This mimics (but is not quite identical to) what we do these days in logind for GC'ing user sessions. --- src/machine/machine.c | 33 +++++++++++++++++++++++++++------ src/machine/machined-dbus.c | 6 ++++-- src/machine/machined.h | 4 ++-- 3 files changed, 33 insertions(+), 10 deletions(-) diff --git a/src/machine/machine.c b/src/machine/machine.c index cf91eef029..9fb184a031 100644 --- a/src/machine/machine.c +++ b/src/machine/machine.c @@ -609,22 +609,43 @@ int machine_finalize(Machine *m) { } bool machine_may_gc(Machine *m, bool drop_not_started) { + int r; + assert(m); if (m->class == MACHINE_HOST) return false; - if (!pidref_is_set(&m->leader)) - return true; - if (drop_not_started && !m->started) return true; - if (m->scope_job && manager_job_is_active(m->manager, m->scope_job)) + r = pidref_is_alive(&m->leader); + if (r == -ESRCH) + return true; + if (r < 0) + log_debug_errno(r, "Unable to determine if leader PID " PID_FMT " is still alive, assuming not: %m", m->leader.pid); + if (r > 0) return false; - if (m->unit && manager_unit_is_active(m->manager, m->unit)) - return false; + if (m->scope_job) { + _cleanup_(sd_bus_error_free) sd_bus_error error = SD_BUS_ERROR_NULL; + + r = manager_job_is_active(m->manager, m->scope_job, &error); + if (r < 0) + log_debug_errno(r, "Failed to determine whether job '%s' is active, assuming it is: %s", m->scope_job, bus_error_message(&error, r)); + if (r != 0) + return false; + } + + if (m->unit) { + _cleanup_(sd_bus_error_free) sd_bus_error error = SD_BUS_ERROR_NULL; + + r = manager_unit_is_active(m->manager, m->unit, &error); + if (r < 0) + log_debug_errno(r, "Failed to determine whether unit '%s' is active, assuming it is: %s", m->unit, bus_error_message(&error, r)); + if (r != 0) + return false; + } return true; } diff --git a/src/machine/machined-dbus.c b/src/machine/machined-dbus.c index 900819c37f..616bea54f8 100644 --- a/src/machine/machined-dbus.c +++ b/src/machine/machined-dbus.c @@ -1262,7 +1262,7 @@ int manager_kill_unit(Manager *manager, const char *unit, int signo, sd_bus_erro return bus_call_method(manager->bus, bus_systemd_mgr, "KillUnit", error, NULL, "ssi", unit, "all", signo); } -int manager_unit_is_active(Manager *manager, const char *unit) { +int manager_unit_is_active(Manager *manager, const char *unit, sd_bus_error *reterr_error) { _cleanup_(sd_bus_error_free) sd_bus_error error = SD_BUS_ERROR_NULL; _cleanup_(sd_bus_message_unrefp) sd_bus_message *reply = NULL; _cleanup_free_ char *path = NULL; @@ -1294,6 +1294,7 @@ int manager_unit_is_active(Manager *manager, const char *unit) { BUS_ERROR_LOAD_FAILED)) return false; + sd_bus_error_move(reterr_error, &error); return r; } @@ -1304,7 +1305,7 @@ int manager_unit_is_active(Manager *manager, const char *unit) { return !STR_IN_SET(state, "inactive", "failed"); } -int manager_job_is_active(Manager *manager, const char *path) { +int manager_job_is_active(Manager *manager, const char *path, sd_bus_error *reterr_error) { _cleanup_(sd_bus_error_free) sd_bus_error error = SD_BUS_ERROR_NULL; _cleanup_(sd_bus_message_unrefp) sd_bus_message *reply = NULL; int r; @@ -1329,6 +1330,7 @@ int manager_job_is_active(Manager *manager, const char *path) { if (sd_bus_error_has_name(&error, SD_BUS_ERROR_UNKNOWN_OBJECT)) return false; + sd_bus_error_move(reterr_error, &error); return r; } diff --git a/src/machine/machined.h b/src/machine/machined.h index b1d9ce1d55..c4753b8bfa 100644 --- a/src/machine/machined.h +++ b/src/machine/machined.h @@ -46,8 +46,8 @@ int match_job_removed(sd_bus_message *message, void *userdata, sd_bus_error *err int manager_stop_unit(Manager *manager, const char *unit, sd_bus_error *error, char **job); int manager_kill_unit(Manager *manager, const char *unit, int signo, sd_bus_error *error); int manager_unref_unit(Manager *m, const char *unit, sd_bus_error *error); -int manager_unit_is_active(Manager *manager, const char *unit); -int manager_job_is_active(Manager *manager, const char *path); +int manager_unit_is_active(Manager *manager, const char *unit, sd_bus_error *reterr_errno); +int manager_job_is_active(Manager *manager, const char *path, sd_bus_error *reterr_errno); int manager_find_machine_for_uid(Manager *m, uid_t host_uid, Machine **ret_machine, uid_t *ret_internal_uid); int manager_find_machine_for_gid(Manager *m, gid_t host_gid, Machine **ret_machine, gid_t *ret_internal_gid); From 48764e2e58829a165690bcbec1c9b869284f6cf9 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 19 May 2025 13:26:07 +0200 Subject: [PATCH 12/12] machine: fix log message, doesn't have to be scope unit, can by any --- src/machine/machine.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/machine/machine.c b/src/machine/machine.c index 9fb184a031..2098405dd0 100644 --- a/src/machine/machine.c +++ b/src/machine/machine.c @@ -572,7 +572,7 @@ int machine_stop(Machine *m) { r = manager_stop_unit(m->manager, m->unit, &error, &job); if (r < 0) - return log_error_errno(r, "Failed to stop machine scope: %s", bus_error_message(&error, r)); + return log_error_errno(r, "Failed to stop machine unit: %s", bus_error_message(&error, r)); free_and_replace(m->scope_job, job); }