(RHEL-53112) Don't terminate sessions after cold (re)start of systemd-logind

src/login/logind-session.c

@@ -465,6 +465,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,
@@ -474,6 +514,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,
@@ -507,6 +548,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,
@@ -658,8 +700,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);
@@ -669,19 +709,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;
+                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 = pidref_set_pid(&p, s->deserialized_pid);
-                if (r >= 0)
-                        r = session_set_leader_consume(s, TAKE_PIDREF(p));
+                r = session_load_leader(s, pidfdid);
                 if (r < 0)
-                        log_warning_errno(r, "Failed to set leader PID for session '%s': %m", s->id);
+                        return r;
         }
 
         return r;

src/login/logind.c

@@ -36,6 +36,7 @@
 #include "selinux-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"
@@ -442,19 +443,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)

test/units/TEST-35-LOGIN.sh

@@ -32,13 +32,15 @@ Environment=SYSTEMD_LOG_LEVEL=debug
 EOF
 
     # We test "coldplug" (completely stop and start logind) here. So we need to preserve
-    # the fdstore, which might contain session leader pidfds. This is extremely rare use case
-    # and shall not be considered fully supported.
+    # the fdstore, which might contain session leader pidfds, but only if pidfd id isn't
+    # a thing. This is extremely rare use case and shall not be considered fully supported.
     # See also: https://github.com/systemd/systemd/pull/30610#discussion_r1440507850
-    systemctl edit --runtime --stdin systemd-logind.service --drop-in=fdstore-preserve.conf <<EOF
+    if systemd-analyze compare-versions "$(uname -r)" lt 6.9; then
+        systemctl edit --runtime --stdin systemd-logind.service --drop-in=fdstore-preserve.conf <<EOF
 [Service]
 FileDescriptorStorePreserve=yes
 EOF
+    fi
 
     systemctl restart systemd-logind.service
 }