From c54112bdee1d32934e688961ca53e81ffff0c99a Mon Sep 17 00:00:00 2001 From: Mike Yuan Date: Thu, 6 Nov 2025 19:31:18 +0100 Subject: [PATCH 1/2] logind: fix potential fd leak in deliver_session_leader_fd_consume() Follow-up for 45eea629e3b3a640bf6a5cd13f4c73c86b426b11 --- src/login/logind.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/login/logind.c b/src/login/logind.c index 847ebbdde9..e643189338 100644 --- a/src/login/logind.c +++ b/src/login/logind.c @@ -441,13 +441,13 @@ static int deliver_session_device_fd(Session *s, const char *fdname, int fd, dev return 0; } -static int deliver_session_leader_fd_consume(Session *s, const char *fdname, int fd) { +static int deliver_session_leader_fd_consume(Session *s, const char *fdname, int fd_consume) { + _cleanup_close_ int fd = ASSERT_FD(fd_consume); _cleanup_(pidref_done) PidRef leader_fdstore = PIDREF_NULL; int r; assert(s); assert(fdname); - assert(fd >= 0); /* Already deserialized via pidfd id? */ if (pidref_is_set(&s->leader)) { @@ -479,6 +479,7 @@ static int deliver_session_leader_fd_consume(Session *s, const char *fdname, int log_warning_errno(r, "Failed to create reference to leader of session '%s': %m", s->id); goto fail_close; } + TAKE_FD(fd); if (leader_fdstore.pid != s->deserialized_pid) log_warning("Leader from pidfd (" PID_FMT ") doesn't match with LEADER=" PID_FMT " for session '%s', proceeding anyway.", @@ -491,7 +492,7 @@ static int deliver_session_leader_fd_consume(Session *s, const char *fdname, int return 0; fail_close: - close_and_notify_warn(fd, fdname); + close_and_notify_warn(TAKE_FD(fd), fdname); return r; } From a6590235dfffc8a676a75b373d97f1db2ebf7079 Mon Sep 17 00:00:00 2001 From: Mike Yuan Date: Tue, 4 Nov 2025 21:13:49 +0100 Subject: [PATCH 2/2] logind: handle session leader termination during deserialization more gracefully We track session leaders by pidfd precisely to make restarts reliable, as leader exiting before deserialization is somewhat expected. Such case is already handled gracefully (we'd GC sessions without leader before kicking off the new cycle), but let's also tweak the log message a bit to reduce annoyance. Closes #39556 --- src/login/logind-session.c | 8 +++++--- src/login/logind.c | 10 +++++----- 2 files changed, 10 insertions(+), 8 deletions(-) diff --git a/src/login/logind-session.c b/src/login/logind-session.c index 027906277c..d0c36f1b41 100644 --- a/src/login/logind-session.c +++ b/src/login/logind-session.c @@ -423,6 +423,8 @@ static int session_load_leader(Session *s, uint64_t pidfdid) { return 0; r = pidref_set_pid(&pidref, s->deserialized_pid); + if (r == -ESRCH) + return log_warning_errno(r, "Leader of session '%s' is gone while deserializing.", s->id); if (r < 0) return log_error_errno(r, "Failed to deserialize leader PID for session '%s': %m", s->id); if (pidref.fd < 0) @@ -437,9 +439,9 @@ static int session_load_leader(Session *s, uint64_t pidfdid) { 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); + return log_warning_errno(SYNTHETIC_ERRNO(ESRCH), + "Deserialized pidfd id for process " PID_FMT " (%" PRIu64 ") doesn't match the current one (%" PRIu64 "). PID recycled while deserializing?", + pidref.pid, pidfdid, pidref.fd_id); } r = session_set_leader_consume(s, TAKE_PIDREF(pidref)); diff --git a/src/login/logind.c b/src/login/logind.c index e643189338..5cdfc2a00d 100644 --- a/src/login/logind.c +++ b/src/login/logind.c @@ -473,10 +473,10 @@ static int deliver_session_leader_fd_consume(Session *s, const char *fdname, int r = pidref_set_pidfd_take(&leader_fdstore, fd); if (r < 0) { - if (r == -ESRCH) - log_debug_errno(r, "Leader of session '%s' is gone while deserializing.", s->id); - else - log_warning_errno(r, "Failed to create reference to leader of session '%s': %m", s->id); + log_warning_errno(r, + r == -ESRCH ? "Leader of session '%s' is gone while deserializing." + : "Failed to create reference to leader of session '%s': %m", + s->id); goto fail_close; } TAKE_FD(fd); @@ -566,7 +566,7 @@ static int manager_enumerate_sessions(Manager *m) { session_add_to_gc_queue(s); k = session_load(s); - if (k < 0) + if (k < 0 && k != -ESRCH) RET_GATHER(r, log_warning_errno(k, "Failed to deserialize session '%s', ignoring: %m", s->id)); }