From e27333b2ccd34545dbc7d26d34a1c24179f989ce Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 19 May 2025 11:02:41 +0200 Subject: [PATCH 1/6] env-file: add helper for printing a properly escaped env var file assignment --- src/basic/env-file.c | 49 ++++++++++++++++++++++++++++++++------------ src/basic/env-file.h | 2 ++ 2 files changed, 38 insertions(+), 13 deletions(-) diff --git a/src/basic/env-file.c b/src/basic/env-file.c index b0faeab4cc..75bb47f336 100644 --- a/src/basic/env-file.c +++ b/src/basic/env-file.c @@ -570,6 +570,41 @@ int merge_env_file( return parse_env_file_internal(f, fname, merge_env_file_push, env); } +static void env_file_fputs_escaped(FILE *f, const char *p) { + assert(f); + assert(p); + + flockfile(f); + + if (string_has_cc(p, NULL) || chars_intersect(p, WHITESPACE SHELL_NEED_QUOTES)) { + fputc_unlocked('"', f); + + for (; *p; p++) { + if (strchr(SHELL_NEED_ESCAPE, *p)) + fputc_unlocked('\\', f); + + fputc_unlocked(*p, f); + } + + fputc_unlocked('"', f); + } else + fputs_unlocked(p, f); + + funlockfile(f); +} + +void env_file_fputs_assignment(FILE *f, const char *k, const char *v) { + assert(f); + assert(k); + + if (!v) + return; + + fputs(k, f); + env_file_fputs_escaped(f, v); + fputc('\n', f); +} + static void write_env_var(FILE *f, const char *v) { const char *p; @@ -587,19 +622,7 @@ static void write_env_var(FILE *f, const char *v) { p++; fwrite_unlocked(v, 1, p-v, f); - if (string_has_cc(p, NULL) || chars_intersect(p, WHITESPACE SHELL_NEED_QUOTES)) { - fputc_unlocked('"', f); - - for (; *p; p++) { - if (strchr(SHELL_NEED_ESCAPE, *p)) - fputc_unlocked('\\', f); - - fputc_unlocked(*p, f); - } - - fputc_unlocked('"', f); - } else - fputs_unlocked(p, f); + env_file_fputs_escaped(f, p); fputc_unlocked('\n', f); } diff --git a/src/basic/env-file.h b/src/basic/env-file.h index 37db30765b..df3f29500f 100644 --- a/src/basic/env-file.h +++ b/src/basic/env-file.h @@ -22,3 +22,5 @@ int merge_env_file(char ***env, FILE *f, const char *fname); int write_env_file(int dir_fd, const char *fname, char **headers, char **l); int write_vconsole_conf(int dir_fd, const char *fname, char **l); + +void env_file_fputs_assignment(FILE *f, const char *k, const char *v); From 019c8ea26a5b99584c60570caee8a573d4fafcfc Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 19 May 2025 10:15:25 +0200 Subject: [PATCH 2/6] logind: port logind state files to fopen_tmpfile_linkable() This replaces use of fopen_temporary() with fopen_tmpfile_linkable() + flink_tmpfile(). This both shortens the code and means we use O_TMPFILE for installing these files, which is always good. No change in behaviour otherwise. --- src/login/logind-inhibit.c | 41 ++++++++++------------------- src/login/logind-seat.c | 28 +++++++------------- src/login/logind-session.c | 53 +++++++++++++------------------------- src/login/logind-user.c | 29 +++++++-------------- 4 files changed, 52 insertions(+), 99 deletions(-) diff --git a/src/login/logind-inhibit.c b/src/login/logind-inhibit.c index e5070ff573..c8ba63ddfa 100644 --- a/src/login/logind-inhibit.c +++ b/src/login/logind-inhibit.c @@ -89,21 +89,22 @@ Inhibitor* inhibitor_free(Inhibitor *i) { } static int inhibitor_save(Inhibitor *i) { - _cleanup_(unlink_and_freep) char *temp_path = NULL; - _cleanup_fclose_ FILE *f = NULL; int r; assert(i); r = mkdir_safe_label("/run/systemd/inhibit", 0755, 0, 0, MKDIR_WARN_MODE); if (r < 0) - goto fail; + return log_error_errno(r, "Failed to create /run/systemd/inhibit/: %m"); - r = fopen_temporary(i->state_file, &f, &temp_path); + _cleanup_(unlink_and_freep) char *temp_path = NULL; + _cleanup_fclose_ FILE *f = NULL; + r = fopen_tmpfile_linkable(i->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", i->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", i->state_file); fprintf(f, "# This is private data. Do not parse.\n" @@ -120,10 +121,8 @@ static int inhibitor_save(Inhibitor *i) { _cleanup_free_ char *cc = NULL; cc = cescape(i->who); - if (!cc) { - r = -ENOMEM; - goto fail; - } + if (!cc) + return log_oom(); fprintf(f, "WHO=%s\n", cc); } @@ -132,10 +131,8 @@ static int inhibitor_save(Inhibitor *i) { _cleanup_free_ char *cc = NULL; cc = cescape(i->why); - if (!cc) { - r = -ENOMEM; - goto fail; - } + if (!cc) + return log_oom(); fprintf(f, "WHY=%s\n", cc); } @@ -143,22 +140,12 @@ static int inhibitor_save(Inhibitor *i) { if (i->fifo_path) fprintf(f, "FIFO=%s\n", i->fifo_path); - r = fflush_and_check(f); + r = flink_tmpfile(f, temp_path, i->state_file, LINK_TMPFILE_REPLACE); if (r < 0) - goto fail; + return log_error_errno(r, "Failed to move '%s' into place: %m", i->state_file); - if (rename(temp_path, i->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 */ return 0; - -fail: - (void) unlink(i->state_file); - - return log_error_errno(r, "Failed to save inhibit data %s: %m", i->state_file); } static int bus_manager_send_inhibited_change(Inhibitor *i) { diff --git a/src/login/logind-seat.c b/src/login/logind-seat.c index fb52c5475e..22690dcf66 100644 --- a/src/login/logind-seat.c +++ b/src/login/logind-seat.c @@ -88,8 +88,6 @@ Seat* seat_free(Seat *s) { } int seat_save(Seat *s) { - _cleanup_(unlink_and_freep) char *temp_path = NULL; - _cleanup_fclose_ FILE *f = NULL; int r; assert(s); @@ -99,13 +97,16 @@ int seat_save(Seat *s) { r = mkdir_safe_label("/run/systemd/seats", 0755, 0, 0, MKDIR_WARN_MODE); if (r < 0) - goto fail; + return log_error_errno(r, "Failed to create /run/systemd/seats/: %m"); - r = fopen_temporary(s->state_file, &f, &temp_path); + _cleanup_(unlink_and_freep) char *temp_path = NULL; + _cleanup_fclose_ FILE *f = NULL; + r = fopen_tmpfile_linkable(s->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", s->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", s->state_file); fprintf(f, "# This is private data. Do not parse.\n" @@ -144,21 +145,12 @@ int seat_save(Seat *s) { i->sessions_by_seat_next ? ' ' : '\n'); } - r = fflush_and_check(f); + r = flink_tmpfile(f, temp_path, s->state_file, LINK_TMPFILE_REPLACE); if (r < 0) - goto fail; + return log_error_errno(r, "Failed to move '%s' into place: %m", s->state_file); - if (rename(temp_path, s->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 */ return 0; - -fail: - (void) unlink(s->state_file); - return log_error_errno(r, "Failed to save seat data %s: %m", s->state_file); } int seat_load(Seat *s) { diff --git a/src/login/logind-session.c b/src/login/logind-session.c index a540774cca..2ed8c428f5 100644 --- a/src/login/logind-session.c +++ b/src/login/logind-session.c @@ -276,8 +276,6 @@ static void session_save_devices(Session *s, FILE *f) { } int session_save(Session *s) { - _cleanup_(unlink_and_freep) char *temp_path = NULL; - _cleanup_fclose_ FILE *f = NULL; int r; assert(s); @@ -290,13 +288,16 @@ int session_save(Session *s) { r = mkdir_safe_label("/run/systemd/sessions", 0755, 0, 0, MKDIR_WARN_MODE); if (r < 0) - goto fail; + return log_error_errno(r, "Failed to create /run/systemd/sessions/: %m"); - r = fopen_temporary(s->state_file, &f, &temp_path); + _cleanup_(unlink_and_freep) char *temp_path = NULL; + _cleanup_fclose_ FILE *f = NULL; + r = fopen_tmpfile_linkable(s->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", s->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", s->state_file); fprintf(f, "# This is private data. Do not parse.\n" @@ -345,10 +346,8 @@ int session_save(Session *s) { _cleanup_free_ char *escaped = NULL; escaped = cescape(s->remote_host); - if (!escaped) { - r = -ENOMEM; - goto fail; - } + if (!escaped) + return log_oom(); fprintf(f, "REMOTE_HOST=%s\n", escaped); } @@ -357,10 +356,8 @@ int session_save(Session *s) { _cleanup_free_ char *escaped = NULL; escaped = cescape(s->remote_user); - if (!escaped) { - r = -ENOMEM; - goto fail; - } + if (!escaped) + return log_oom(); fprintf(f, "REMOTE_USER=%s\n", escaped); } @@ -369,10 +366,8 @@ int session_save(Session *s) { _cleanup_free_ char *escaped = NULL; escaped = cescape(s->service); - if (!escaped) { - r = -ENOMEM; - goto fail; - } + if (!escaped) + return log_oom(); fprintf(f, "SERVICE=%s\n", escaped); } @@ -381,10 +376,8 @@ int session_save(Session *s) { _cleanup_free_ char *escaped = NULL; escaped = cescape(s->desktop); - if (!escaped) { - r = -ENOMEM; - goto fail; - } + if (!escaped) + return log_oom(); fprintf(f, "DESKTOP=%s\n", escaped); } @@ -413,22 +406,12 @@ int session_save(Session *s) { session_save_devices(s, f); } - r = fflush_and_check(f); + r = flink_tmpfile(f, temp_path, s->state_file, LINK_TMPFILE_REPLACE); if (r < 0) - goto fail; + return log_error_errno(r, "Failed to move '%s' into place: %m", s->state_file); - if (rename(temp_path, s->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 */ return 0; - -fail: - (void) unlink(s->state_file); - - return log_error_errno(r, "Failed to save session data %s: %m", s->state_file); } static int session_load_devices(Session *s, const char *devices) { diff --git a/src/login/logind-user.c b/src/login/logind-user.c index d5795e2a7f..c854b1e5fd 100644 --- a/src/login/logind-user.c +++ b/src/login/logind-user.c @@ -144,8 +144,6 @@ User *user_free(User *u) { } static int user_save_internal(User *u) { - _cleanup_(unlink_and_freep) char *temp_path = NULL; - _cleanup_fclose_ FILE *f = NULL; int r; assert(u); @@ -153,13 +151,16 @@ static int user_save_internal(User *u) { r = mkdir_safe_label("/run/systemd/users", 0755, 0, 0, MKDIR_WARN_MODE); if (r < 0) - goto fail; + return log_error_errno(r, "Failed to create /run/systemd/users/: %m"); - r = fopen_temporary(u->state_file, &f, &temp_path); + _cleanup_(unlink_and_freep) char *temp_path = NULL; + _cleanup_fclose_ FILE *f = NULL; + r = fopen_tmpfile_linkable(u->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", u->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", u->state_file); fprintf(f, "# This is private data. Do not parse.\n" @@ -282,22 +283,12 @@ static int user_save_internal(User *u) { fputc('\n', f); } - r = fflush_and_check(f); + r = flink_tmpfile(f, temp_path, u->state_file, LINK_TMPFILE_REPLACE); if (r < 0) - goto fail; + return log_error_errno(r, "Failed to move '%s' into place: %m", u->state_file); - if (rename(temp_path, u->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 */ return 0; - -fail: - (void) unlink(u->state_file); - - return log_error_errno(r, "Failed to save user data %s: %m", u->state_file); } int user_save(User *u) { From 232f4e12fe526bfbcd5c7bd857bbb6a0db818cbc Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 19 May 2025 11:56:28 +0200 Subject: [PATCH 3/6] logind: fix escaping of various fields in state files parse_env_file() applies a certain kind of unescaping, and we really need to match that when writing out arbitrary strings. (This gets a bit confusing in the inhibit case, since that already applied one level of escaping on its own...) --- src/login/logind-inhibit.c | 31 +++++++----------- src/login/logind-session.c | 66 +++++++------------------------------- src/login/logind-user.c | 14 +++----- 3 files changed, 27 insertions(+), 84 deletions(-) diff --git a/src/login/logind-inhibit.c b/src/login/logind-inhibit.c index c8ba63ddfa..22526ac0bf 100644 --- a/src/login/logind-inhibit.c +++ b/src/login/logind-inhibit.c @@ -117,28 +117,19 @@ static int inhibitor_save(Inhibitor *i) { i->uid, i->pid.pid); - if (i->who) { - _cleanup_free_ char *cc = NULL; + /* For historical reasons there's double escaping applied here: once here via cescape(), and once by + * env_file_fputs_assignment() */ + _cleanup_free_ char *who_escaped = cescape(i->who); + if (!who_escaped) + return log_oom(); + env_file_fputs_assignment(f, "WHO=", who_escaped); - cc = cescape(i->who); - if (!cc) - return log_oom(); + _cleanup_free_ char *why_escaped = cescape(i->why); + if (!why_escaped) + return log_oom(); + env_file_fputs_assignment(f, "WHY=", why_escaped); - fprintf(f, "WHO=%s\n", cc); - } - - if (i->why) { - _cleanup_free_ char *cc = NULL; - - cc = cescape(i->why); - if (!cc) - return log_oom(); - - fprintf(f, "WHY=%s\n", cc); - } - - if (i->fifo_path) - fprintf(f, "FIFO=%s\n", i->fifo_path); + env_file_fputs_assignment(f, "FIFO=", i->fifo_path); r = flink_tmpfile(f, temp_path, i->state_file, LINK_TMPFILE_REPLACE); if (r < 0) diff --git a/src/login/logind-session.c b/src/login/logind-session.c index 2ed8c428f5..c0038c1a62 100644 --- a/src/login/logind-session.c +++ b/src/login/logind-session.c @@ -302,20 +302,20 @@ int session_save(Session *s) { fprintf(f, "# This is private data. Do not parse.\n" "UID="UID_FMT"\n" - "USER=%s\n" "ACTIVE=%s\n" "IS_DISPLAY=%s\n" "STATE=%s\n" "REMOTE=%s\n" "LEADER_FD_SAVED=%s\n", s->user->user_record->uid, - s->user->user_record->user_name, one_zero(session_is_active(s)), one_zero(s->user->display == s), session_state_to_string(session_get_state(s)), one_zero(s->remote), one_zero(s->leader_fd_saved)); + env_file_fputs_assignment(f, "USER=", s->user->user_record->user_name); + if (s->type >= 0) fprintf(f, "TYPE=%s\n", session_type_to_string(s->type)); @@ -325,62 +325,20 @@ int session_save(Session *s) { if (s->class >= 0) fprintf(f, "CLASS=%s\n", session_class_to_string(s->class)); - if (s->scope) - fprintf(f, "SCOPE=%s\n", s->scope); - if (s->scope_job) - fprintf(f, "SCOPE_JOB=%s\n", s->scope_job); - + env_file_fputs_assignment(f, "SCOPE=", s->scope); + env_file_fputs_assignment(f, "SCOPE_JOB=", s->scope_job); if (s->seat) - fprintf(f, "SEAT=%s\n", s->seat->id); - - if (s->tty) - fprintf(f, "TTY=%s\n", s->tty); + env_file_fputs_assignment(f, "SEAT=", s->seat->id); + env_file_fputs_assignment(f, "TTY=", s->tty); if (s->tty_validity >= 0) fprintf(f, "TTY_VALIDITY=%s\n", tty_validity_to_string(s->tty_validity)); - if (s->display) - fprintf(f, "DISPLAY=%s\n", s->display); - - if (s->remote_host) { - _cleanup_free_ char *escaped = NULL; - - escaped = cescape(s->remote_host); - if (!escaped) - return log_oom(); - - fprintf(f, "REMOTE_HOST=%s\n", escaped); - } - - if (s->remote_user) { - _cleanup_free_ char *escaped = NULL; - - escaped = cescape(s->remote_user); - if (!escaped) - return log_oom(); - - fprintf(f, "REMOTE_USER=%s\n", escaped); - } - - if (s->service) { - _cleanup_free_ char *escaped = NULL; - - escaped = cescape(s->service); - if (!escaped) - return log_oom(); - - fprintf(f, "SERVICE=%s\n", escaped); - } - - if (s->desktop) { - _cleanup_free_ char *escaped = NULL; - - escaped = cescape(s->desktop); - if (!escaped) - return log_oom(); - - fprintf(f, "DESKTOP=%s\n", escaped); - } + env_file_fputs_assignment(f, "DISPLAY=", s->display); + env_file_fputs_assignment(f, "REMOTE_HOST=", s->remote_host); + env_file_fputs_assignment(f, "REMOTE_USER=", s->remote_user); + env_file_fputs_assignment(f, "SERVICE=", s->service); + env_file_fputs_assignment(f, "DESKTOP=", s->desktop); if (s->seat && seat_has_vts(s->seat)) fprintf(f, "VTNR=%u\n", s->vtnr); @@ -402,7 +360,7 @@ int session_save(Session *s) { s->timestamp.monotonic); if (s->controller) { - fprintf(f, "CONTROLLER=%s\n", s->controller); + env_file_fputs_assignment(f, "CONTROLLER=", s->controller); session_save_devices(s, f); } diff --git a/src/login/logind-user.c b/src/login/logind-user.c index c854b1e5fd..a1f7c439f2 100644 --- a/src/login/logind-user.c +++ b/src/login/logind-user.c @@ -174,17 +174,11 @@ static int user_save_internal(User *u) { user_gc_mode_to_string(u->gc_mode)); /* LEGACY: no-one reads RUNTIME= anymore, drop it at some point */ - if (u->runtime_path) - fprintf(f, "RUNTIME=%s\n", u->runtime_path); - - if (u->runtime_dir_job) - fprintf(f, "RUNTIME_DIR_JOB=%s\n", u->runtime_dir_job); - - if (u->service_manager_job) - fprintf(f, "SERVICE_JOB=%s\n", u->service_manager_job); - + env_file_fputs_assignment(f, "RUNTIME=", u->runtime_path); + env_file_fputs_assignment(f, "RUNTIME_DIR_JOB=", u->runtime_dir_job); + env_file_fputs_assignment(f, "SERVICE_JOB=", u->service_manager_job); if (u->display) - fprintf(f, "DISPLAY=%s\n", u->display->id); + env_file_fputs_assignment(f, "DISPLAY=%s\n", u->display->id); if (dual_timestamp_is_set(&u->timestamp)) fprintf(f, From b0555149de780f81ec6f741d36bdb41b4d1d8a94 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 19 May 2025 11:57:56 +0200 Subject: [PATCH 4/6] logind: always enclose list fields (that may contain spaces) in quotes --- src/login/logind-seat.c | 21 ++++++++++----------- src/login/logind-session.c | 17 ++++++++++++----- src/login/logind-user.c | 19 ++++++++++++------- 3 files changed, 34 insertions(+), 23 deletions(-) diff --git a/src/login/logind-seat.c b/src/login/logind-seat.c index 22690dcf66..f6eef1c821 100644 --- a/src/login/logind-seat.c +++ b/src/login/logind-seat.c @@ -129,20 +129,19 @@ int seat_save(Seat *s) { } if (s->sessions) { - fputs("SESSIONS=", f); - LIST_FOREACH(sessions_by_seat, i, s->sessions) { - fprintf(f, - "%s%c", - i->id, - i->sessions_by_seat_next ? ' ' : '\n'); - } - - fputs("UIDS=", f); + fputs("SESSIONS=\"", f); LIST_FOREACH(sessions_by_seat, i, s->sessions) fprintf(f, - UID_FMT"%c", + "%s%s", + i->id, + i->sessions_by_seat_next ? " " : "\"\n"); + + fputs("UIDS=\"", f); + LIST_FOREACH(sessions_by_seat, i, s->sessions) + fprintf(f, + UID_FMT"%s", i->user->user_record->uid, - i->sessions_by_seat_next ? ' ' : '\n'); + i->sessions_by_seat_next ? " " : "\"\n"); } r = flink_tmpfile(f, temp_path, s->state_file, LINK_TMPFILE_REPLACE); diff --git a/src/login/logind-session.c b/src/login/logind-session.c index c0038c1a62..150aea887f 100644 --- a/src/login/logind-session.c +++ b/src/login/logind-session.c @@ -267,12 +267,19 @@ int session_set_leader_consume(Session *s, PidRef _leader) { static void session_save_devices(Session *s, FILE *f) { SessionDevice *sd; - if (!hashmap_isempty(s->devices)) { - fprintf(f, "DEVICES="); - HASHMAP_FOREACH(sd, s->devices) - fprintf(f, DEVNUM_FORMAT_STR " ", DEVNUM_FORMAT_VAL(sd->dev)); - fprintf(f, "\n"); + if (hashmap_isempty(s->devices)) + return; + + bool need_space = false; + fputs("DEVICES=\"", f); + HASHMAP_FOREACH(sd, s->devices) { + if (need_space) + fputc(' ', f); + + fprintf(f, DEVNUM_FORMAT_STR, DEVNUM_FORMAT_VAL(sd->dev)); + need_space = true; } + fputs("\"\n", f); } int session_save(Session *s) { diff --git a/src/login/logind-user.c b/src/login/logind-user.c index a1f7c439f2..8da9afb7f8 100644 --- a/src/login/logind-user.c +++ b/src/login/logind-user.c @@ -194,7 +194,7 @@ static int user_save_internal(User *u) { if (u->sessions) { bool first; - fputs("SESSIONS=", f); + fputs("SESSIONS=\"", f); first = true; LIST_FOREACH(sessions_by_user, i, u->sessions) { if (first) @@ -205,7 +205,8 @@ static int user_save_internal(User *u) { fputs(i->id, f); } - fputs("\nSEATS=", f); + fputs("\"\n" + "SEATS=\"", f); first = true; LIST_FOREACH(sessions_by_user, i, u->sessions) { if (!i->seat) @@ -219,7 +220,8 @@ static int user_save_internal(User *u) { fputs(i->seat->id, f); } - fputs("\nACTIVE_SESSIONS=", f); + fputs("\"\n" + "ACTIVE_SESSIONS=\"", f); first = true; LIST_FOREACH(sessions_by_user, i, u->sessions) { if (!session_is_active(i)) @@ -233,7 +235,8 @@ static int user_save_internal(User *u) { fputs(i->id, f); } - fputs("\nONLINE_SESSIONS=", f); + fputs("\"\n" + "ONLINE_SESSIONS=\"", f); first = true; LIST_FOREACH(sessions_by_user, i, u->sessions) { if (session_get_state(i) == SESSION_CLOSING) @@ -247,7 +250,8 @@ static int user_save_internal(User *u) { fputs(i->id, f); } - fputs("\nACTIVE_SEATS=", f); + fputs("\"\n" + "ACTIVE_SEATS=\"", f); first = true; LIST_FOREACH(sessions_by_user, i, u->sessions) { if (!session_is_active(i) || !i->seat) @@ -261,7 +265,8 @@ static int user_save_internal(User *u) { fputs(i->seat->id, f); } - fputs("\nONLINE_SEATS=", f); + fputs("\"\n" + "ONLINE_SEATS=\"", f); first = true; LIST_FOREACH(sessions_by_user, i, u->sessions) { if (session_get_state(i) == SESSION_CLOSING || !i->seat) @@ -274,7 +279,7 @@ static int user_save_internal(User *u) { fputs(i->seat->id, f); } - fputc('\n', f); + fputs("\"\n", f); } r = flink_tmpfile(f, temp_path, u->state_file, LINK_TMPFILE_REPLACE); From 1aa202dd243c815efa5c2017fef996e66546a0ba Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 19 May 2025 12:05:45 +0200 Subject: [PATCH 5/6] logind: save/restore inhibitor process pidfdid --- src/login/logind-inhibit.c | 35 ++++++++++++++++++++++++++++------- 1 file changed, 28 insertions(+), 7 deletions(-) diff --git a/src/login/logind-inhibit.c b/src/login/logind-inhibit.c index 22526ac0bf..5cc748bdc3 100644 --- a/src/login/logind-inhibit.c +++ b/src/login/logind-inhibit.c @@ -110,12 +110,18 @@ static int inhibitor_save(Inhibitor *i) { "# This is private data. Do not parse.\n" "WHAT=%s\n" "MODE=%s\n" - "UID="UID_FMT"\n" - "PID="PID_FMT"\n", + "UID="UID_FMT"\n", inhibit_what_to_string(i->what), inhibit_mode_to_string(i->mode), - i->uid, - i->pid.pid); + i->uid); + + if (pidref_is_set(&i->pid)) { + fprintf(f, "PID="PID_FMT"\n", i->pid.pid); + + (void) pidref_acquire_pidfd_id(&i->pid); + if (i->pid.fd_id != 0) + fprintf(f, "PIDFDID=%" PRIu64 "\n", i->pid.fd_id); + } /* For historical reasons there's double escaping applied here: once here via cescape(), and once by * env_file_fputs_assignment() */ @@ -191,7 +197,7 @@ void inhibitor_stop(Inhibitor *i) { } int inhibitor_load(Inhibitor *i) { - _cleanup_free_ char *what = NULL, *uid = NULL, *pid = NULL, *who = NULL, *why = NULL, *mode = NULL; + _cleanup_free_ char *what = NULL, *uid = NULL, *pid = NULL, *pidfdid = NULL, *who = NULL, *why = NULL, *mode = NULL; InhibitWhat w; InhibitMode mm; char *cc; @@ -202,6 +208,7 @@ int inhibitor_load(Inhibitor *i) { "WHAT", &what, "UID", &uid, "PID", &pid, + "PIDFDID", &pidfdid, "WHO", &who, "WHY", &why, "MODE", &mode, @@ -223,11 +230,25 @@ int inhibitor_load(Inhibitor *i) { log_debug_errno(r, "Failed to parse UID of inhibitor: %s", uid); } + pidref_done(&i->pid); if (pid) { - pidref_done(&i->pid); r = pidref_set_pidstr(&i->pid, pid); if (r < 0) - log_debug_errno(r, "Failed to parse PID of inhibitor: %s", pid); + log_debug_errno(r, "Failed to parse PID of inhibitor, ignoring: %s", pid); + else if (pidfdid) { + uint64_t fd_id; + r = safe_atou64(pidfdid, &fd_id); + if (r < 0) + log_warning_errno(r, "Failed to parse leader pidfd ID, ignoring: %s", pidfdid); + else { + (void) pidref_acquire_pidfd_id(&i->pid); + + if (fd_id != i->pid.fd_id) { + log_debug("PID got recycled, ignoring."); + pidref_done(&i->pid); + } + } + } } if (who) { From f01d8658a3a57d05a5156aefd32d8137c3ee3996 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 19 May 2025 12:14:25 +0200 Subject: [PATCH 6/6] logind: also save pidfdid as part of session state, even if we don't parse it --- src/login/logind-session.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/login/logind-session.c b/src/login/logind-session.c index 150aea887f..7032e95d20 100644 --- a/src/login/logind-session.c +++ b/src/login/logind-session.c @@ -353,8 +353,12 @@ int session_save(Session *s) { if (!s->vtnr) fprintf(f, "POSITION=%u\n", s->position); - if (pidref_is_set(&s->leader)) + if (pidref_is_set(&s->leader)) { fprintf(f, "LEADER="PID_FMT"\n", s->leader.pid); + (void) pidref_acquire_pidfd_id(&s->leader); + if (s->leader.fd_id != 0) + fprintf(f, "LEADER_PIDFDID=%" PRIu64 "\n", s->leader.fd_id); + } if (audit_session_is_valid(s->audit_id)) fprintf(f, "AUDIT=%"PRIu32"\n", s->audit_id);