From: Mike Yuan Date: Fri, 24 Oct 2025 21:09:50 +0000 (+0200) Subject: logind: support deserializing session leader through pidfdid X-Git-Tag: v259-rc1~237^2~1 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=45eea629e3b3a640bf6a5cd13f4c73c86b426b11;p=thirdparty%2Fsystemd.git logind: support deserializing session leader through pidfdid People make weird assumptions around state preservation and expect logind to be stoppable. While this is realistically not OK we can probably improve things a little. This complements f01d8658a3a57d05a5156aefd32d8137c3ee3996 and adds support for deserializing the LEADER_PIDFDID= field. We still prioritize pidfd if got one from fdstore (as with service_notify_message_parse_new_pid() in pid1), but otherwise this should make logind restart more robust when fdstore gets spuriously cleared. Fixes #39437 --- diff --git a/src/login/logind-session.c b/src/login/logind-session.c index be8645bfd80..027906277c1 100644 --- a/src/login/logind-session.c +++ b/src/login/logind-session.c @@ -409,6 +409,46 @@ static int session_load_devices(Session *s, const char *devices) { return r; } +static int session_load_leader(Session *s, uint64_t pidfdid) { + _cleanup_(pidref_done) PidRef pidref = PIDREF_NULL; + int r; + + assert(s); + assert(pid_is_valid(s->deserialized_pid)); + assert(!pidref_is_set(&s->leader)); + + if (pidfdid == 0 && s->leader_fd_saved) + /* We have no pidfd id for stable reference, but the pidfd has been submitted to fdstore. + * manager_enumerate_fds() will dispatch the leader fd for us later. */ + return 0; + + r = pidref_set_pid(&pidref, s->deserialized_pid); + if (r < 0) + return log_error_errno(r, "Failed to deserialize leader PID for session '%s': %m", s->id); + if (pidref.fd < 0) + return log_error_errno(SYNTHETIC_ERRNO(ENOTRECOVERABLE), + "Failed to acquire pidfd for session leader '" PID_FMT "', refusing.", + pidref.pid); + + if (pidfdid > 0) { + r = pidref_acquire_pidfd_id(&pidref); + if (r < 0) + return log_error_errno(r, "Failed to acquire pidfd id of deserialized leader '" PID_FMT "': %m", + pidref.pid); + + if (pidref.fd_id != pidfdid) + return log_error_errno(SYNTHETIC_ERRNO(ESRCH), + "Deserialized pidfd id for process " PID_FMT " (%" PRIu64 ") doesn't match with the current one (%" PRIu64 "), refusing.", + pidref.pid, pidfdid, pidref.fd_id); + } + + r = session_set_leader_consume(s, TAKE_PIDREF(pidref)); + if (r < 0) + return log_error_errno(r, "Failed to set leader PID for session '%s': %m", s->id); + + return 1; +} + int session_load(Session *s) { _cleanup_free_ char *remote = NULL, *seat = NULL, @@ -418,6 +458,7 @@ int session_load(Session *s) { *position = NULL, *leader_pid = NULL, *leader_fd_saved = NULL, + *leader_pidfdid = NULL, *type = NULL, *original_type = NULL, *class = NULL, @@ -452,6 +493,7 @@ int session_load(Session *s) { "POSITION", &position, "LEADER", &leader_pid, "LEADER_FD_SAVED", &leader_fd_saved, + "LEADER_PIDFDID", &leader_pidfdid, "TYPE", &type, "ORIGINAL_TYPE", &original_type, "CLASS", &class, @@ -594,8 +636,6 @@ int session_load(Session *s) { } if (leader_pid) { - assert(!pidref_is_set(&s->leader)); - r = parse_pid(leader_pid, &s->deserialized_pid); if (r < 0) return log_error_errno(r, "Failed to parse LEADER=%s: %m", leader_pid); @@ -605,25 +645,19 @@ int session_load(Session *s) { if (r < 0) return log_error_errno(r, "Failed to parse LEADER_FD_SAVED=%s: %m", leader_fd_saved); s->leader_fd_saved = r > 0; - - if (s->leader_fd_saved) - /* The leader fd will be acquired from fdstore later */ - return 0; } - _cleanup_(pidref_done) PidRef p = PIDREF_NULL; - - r = pidref_set_pid(&p, s->deserialized_pid); - if (r < 0) - return log_error_errno(r, "Failed to deserialize leader PID for session '%s': %m", s->id); - if (p.fd < 0) - return log_error_errno(SYNTHETIC_ERRNO(ENOTRECOVERABLE), - "Failed to acquire pidfd for session leader '" PID_FMT "', refusing.", - p.pid); + uint64_t pidfdid; + if (leader_pidfdid) { + r = safe_atou64(leader_pidfdid, &pidfdid); + if (r < 0) + return log_error_errno(r, "Failed to parse LEADER_PIDFDID=%s: %m", leader_pidfdid); + } else + pidfdid = 0; - r = session_set_leader_consume(s, TAKE_PIDREF(p)); + r = session_load_leader(s, pidfdid); if (r < 0) - return log_error_errno(r, "Failed to set leader PID for session '%s': %m", s->id); + return r; } return 0; diff --git a/src/login/logind.c b/src/login/logind.c index e1219fe73a8..847ebbdde9a 100644 --- a/src/login/logind.c +++ b/src/login/logind.c @@ -40,6 +40,7 @@ #include "process-util.h" #include "service-util.h" #include "signal-util.h" +#include "stat-util.h" #include "strv.h" #include "terminal-util.h" #include "udev-util.h" @@ -448,19 +449,28 @@ static int deliver_session_leader_fd_consume(Session *s, const char *fdname, int assert(fdname); assert(fd >= 0); - if (!pid_is_valid(s->deserialized_pid)) { + /* Already deserialized via pidfd id? */ + if (pidref_is_set(&s->leader)) { + assert(s->leader.pid == s->deserialized_pid); + assert(s->leader.fd >= 0); + + r = fd_inode_same(fd, s->leader.fd); + if (r < 0) + return log_warning_errno(r, "Failed to compare pidfd with deserialized leader for session '%s': %m", + s->id); + if (r > 0) + return 0; + + log_warning("Got leader pidfd for session '%s' which mismatches with the deserialized process, resetting with pidfd.", + s->id); + + } else if (!pid_is_valid(s->deserialized_pid)) { r = log_warning_errno(SYNTHETIC_ERRNO(EOWNERDEAD), "Got leader pidfd for session '%s', but LEADER= is not set, refusing.", s->id); goto fail_close; } - if (!s->leader_fd_saved) - log_warning("Got leader pidfd for session '%s', but not recorded in session state, proceeding anyway.", - s->id); - else - assert(!pidref_is_set(&s->leader)); - r = pidref_set_pidfd_take(&leader_fdstore, fd); if (r < 0) { if (r == -ESRCH)