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 be8645bfd809bd0725d085edcf95648308889b9c..027906277c15ab03c9d87ccf263f076bf6034ce0 100644 (file)
--- 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 e1219fe73a8c8e78445745f005d1143bbcc1a849..847ebbdde9aa47c0b80f492edcfcd9b776b0f94f 100644 (file)
--- 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)