From bed4386e2946c483ddd762f01d742f5b45d18612 Mon Sep 17 00:00:00 2001 From: Mike Yuan Date: Sat, 4 Jan 2025 14:43:07 +0100 Subject: [PATCH 1/4] signal-util: generalize sigaction_nop_nocldstop --- src/basic/signal-util.c | 5 +++++ src/basic/signal-util.h | 1 + src/core/crash-handler.c | 7 +------ src/nspawn/nspawn.c | 7 +------ src/tty-ask-password-agent/tty-ask-password-agent.c | 6 +----- 5 files changed, 9 insertions(+), 17 deletions(-) diff --git a/src/basic/signal-util.c b/src/basic/signal-util.c index ad9ed049f1..32d37e68dd 100644 --- a/src/basic/signal-util.c +++ b/src/basic/signal-util.c @@ -302,6 +302,11 @@ const struct sigaction sigaction_default = { .sa_flags = SA_RESTART, }; +const struct sigaction sigaction_nop_nocldstop = { + .sa_handler = nop_signal_handler, + .sa_flags = SA_NOCLDSTOP|SA_RESTART, +}; + int parse_signo(const char *s, int *ret) { int sig, r; diff --git a/src/basic/signal-util.h b/src/basic/signal-util.h index 559ce42949..dc2b9de147 100644 --- a/src/basic/signal-util.h +++ b/src/basic/signal-util.h @@ -68,5 +68,6 @@ void propagate_signal(int sig, siginfo_t *siginfo); extern const struct sigaction sigaction_ignore; extern const struct sigaction sigaction_default; +extern const struct sigaction sigaction_nop_nocldstop; int parse_signo(const char *s, int *ret); diff --git a/src/core/crash-handler.c b/src/core/crash-handler.c index 68bc96e517..e964c897d2 100644 --- a/src/core/crash-handler.c +++ b/src/core/crash-handler.c @@ -69,13 +69,8 @@ _noreturn_ static void crash(int sig, siginfo_t *siginfo, void *context) { LOG_MESSAGE("Caught <%s>, not dumping core.", signal_to_string(sig)), "MESSAGE_ID=" SD_MESSAGE_CRASH_NO_COREDUMP_STR); else { - sa = (struct sigaction) { - .sa_handler = nop_signal_handler, - .sa_flags = SA_NOCLDSTOP|SA_RESTART, - }; - /* We want to wait for the core process, hence let's enable SIGCHLD */ - (void) sigaction(SIGCHLD, &sa, NULL); + (void) sigaction(SIGCHLD, &sigaction_nop_nocldstop, NULL); pid = raw_clone(SIGCHLD); if (pid < 0) diff --git a/src/nspawn/nspawn.c b/src/nspawn/nspawn.c index c2f232ae79..cc037c078f 100644 --- a/src/nspawn/nspawn.c +++ b/src/nspawn/nspawn.c @@ -5175,11 +5175,6 @@ static int run_container( pid_t *pid, int *ret) { - static const struct sigaction sa = { - .sa_handler = nop_signal_handler, - .sa_flags = SA_NOCLDSTOP|SA_RESTART, - }; - _cleanup_(release_lock_file) LockFile uid_shift_lock = LOCK_FILE_INIT; _cleanup_close_ int etc_passwd_lock = -EBADF; _cleanup_close_pair_ int @@ -5240,7 +5235,7 @@ static int run_container( if (r < 0) return log_error_errno(errno, "Failed to change the signal mask: %m"); - r = sigaction(SIGCHLD, &sa, NULL); + r = sigaction(SIGCHLD, &sigaction_nop_nocldstop, NULL); if (r < 0) return log_error_errno(errno, "Failed to install SIGCHLD handler: %m"); diff --git a/src/tty-ask-password-agent/tty-ask-password-agent.c b/src/tty-ask-password-agent/tty-ask-password-agent.c index c6097c4120..9bd7ba452f 100644 --- a/src/tty-ask-password-agent/tty-ask-password-agent.c +++ b/src/tty-ask-password-agent/tty-ask-password-agent.c @@ -571,13 +571,9 @@ static int parse_argv(int argc, char *argv[]) { * terminated. */ static int ask_on_this_console(const char *tty, pid_t *ret_pid, char **arguments) { - static const struct sigaction sigchld = { - .sa_handler = nop_signal_handler, - .sa_flags = SA_NOCLDSTOP | SA_RESTART, - }; int r; - assert_se(sigaction(SIGCHLD, &sigchld, NULL) >= 0); + assert_se(sigaction(SIGCHLD, &sigaction_nop_nocldstop, NULL) >= 0); assert_se(sigaction(SIGHUP, &sigaction_default, NULL) >= 0); assert_se(sigprocmask_many(SIG_UNBLOCK, NULL, SIGHUP, SIGCHLD) >= 0); From 5734893f8291dff62ae8b043e2962856be0eb50e Mon Sep 17 00:00:00 2001 From: Mike Yuan Date: Sat, 4 Jan 2025 14:49:14 +0100 Subject: [PATCH 2/4] tree-wide: make sigactions static const --- src/basic/sigbus.c | 2 +- src/core/crash-handler.c | 15 +++++++-------- src/core/manager.c | 2 +- src/coredump/coredumpctl.c | 9 +++++---- 4 files changed, 14 insertions(+), 14 deletions(-) diff --git a/src/basic/sigbus.c b/src/basic/sigbus.c index e8d2fdb79a..307e879bf5 100644 --- a/src/basic/sigbus.c +++ b/src/basic/sigbus.c @@ -121,7 +121,7 @@ static void sigbus_handler(int sn, siginfo_t *si, void *data) { } void sigbus_install(void) { - struct sigaction sa = { + static const struct sigaction sa = { .sa_sigaction = sigbus_handler, .sa_flags = SA_SIGINFO, }; diff --git a/src/core/crash-handler.c b/src/core/crash-handler.c index e964c897d2..056ac4b347 100644 --- a/src/core/crash-handler.c +++ b/src/core/crash-handler.c @@ -52,8 +52,13 @@ _noreturn_ void freeze_or_exit_or_reboot(void) { } _noreturn_ static void crash(int sig, siginfo_t *siginfo, void *context) { - struct sigaction sa; + static const struct sigaction sa_nocldwait = { + .sa_handler = SIG_IGN, + .sa_flags = SA_NOCLDSTOP|SA_NOCLDWAIT|SA_RESTART, + }; + pid_t pid; + int r; /* NB: 💣 💣 💣 This is a signal handler, most likely executed in a situation where we have corrupted * memory. Thus: please avoid any libc memory allocation here, or any functions that internally use @@ -94,7 +99,6 @@ _noreturn_ static void crash(int sig, siginfo_t *siginfo, void *context) { _exit(EXIT_EXCEPTION); } else { siginfo_t status; - int r; if (siginfo) { if (siginfo->si_pid == 0) @@ -141,13 +145,8 @@ _noreturn_ static void crash(int sig, siginfo_t *siginfo, void *context) { if (arg_crash_chvt >= 0) (void) chvt(arg_crash_chvt); - sa = (struct sigaction) { - .sa_handler = SIG_IGN, - .sa_flags = SA_NOCLDSTOP|SA_NOCLDWAIT|SA_RESTART, - }; - /* Let the kernel reap children for us */ - (void) sigaction(SIGCHLD, &sa, NULL); + (void) sigaction(SIGCHLD, &sa_nocldwait, NULL); if (arg_crash_shell) { log_notice("Executing crash shell..."); diff --git a/src/core/manager.c b/src/core/manager.c index e75c760b6f..f4fbeaa142 100644 --- a/src/core/manager.c +++ b/src/core/manager.c @@ -517,7 +517,7 @@ static int manager_enable_special_signals(Manager *m) { #define RTSIG_IF_AVAILABLE(signum) (signum <= SIGRTMAX ? signum : -1) static int manager_setup_signals(Manager *m) { - struct sigaction sa = { + static const struct sigaction sa = { .sa_handler = SIG_DFL, .sa_flags = SA_NOCLDSTOP|SA_RESTART, }; diff --git a/src/coredump/coredumpctl.c b/src/coredump/coredumpctl.c index e7d0dd34c0..f01db21f88 100644 --- a/src/coredump/coredumpctl.c +++ b/src/coredump/coredumpctl.c @@ -1170,13 +1170,14 @@ static void sigterm_handler(int signal, siginfo_t *info, void *ucontext) { } static int run_debug(int argc, char **argv, void *userdata) { - _cleanup_(sd_journal_closep) sd_journal *j = NULL; - _cleanup_free_ char *exe = NULL, *path = NULL; - _cleanup_strv_free_ char **debugger_call = NULL; - struct sigaction sa = { + static const struct sigaction sa = { .sa_sigaction = sigterm_handler, .sa_flags = SA_SIGINFO, }; + + _cleanup_(sd_journal_closep) sd_journal *j = NULL; + _cleanup_free_ char *exe = NULL, *path = NULL; + _cleanup_strv_free_ char **debugger_call = NULL; bool unlink_path = false; const char *data, *fork_name; size_t len; From c56935a09d408f5cfec481dab6b9a6ab3f727d70 Mon Sep 17 00:00:00 2001 From: Mike Yuan Date: Sat, 4 Jan 2025 15:11:53 +0100 Subject: [PATCH 3/4] tty-ask-password-agent: coding style tweaks --- .../tty-ask-password-agent.c | 28 +++++++++++-------- 1 file changed, 17 insertions(+), 11 deletions(-) diff --git a/src/tty-ask-password-agent/tty-ask-password-agent.c b/src/tty-ask-password-agent/tty-ask-password-agent.c index 9bd7ba452f..68b6d1c70e 100644 --- a/src/tty-ask-password-agent/tty-ask-password-agent.c +++ b/src/tty-ask-password-agent/tty-ask-password-agent.c @@ -567,12 +567,16 @@ static int parse_argv(int argc, char *argv[]) { /* * To be able to ask on all terminal devices of /dev/console the devices are collected. If more than one * device is found, then on each of the terminals an inquiring task is forked. Every task has its own session - * and its own controlling terminal. If one of the tasks does handle a password, the remaining tasks will be + * and its own controlling terminal. If one of the tasks does handle a password, the remaining tasks will be * terminated. */ -static int ask_on_this_console(const char *tty, pid_t *ret_pid, char **arguments) { +static int ask_on_this_console(const char *tty, char **arguments, pid_t *ret_pid) { int r; + assert(tty); + assert(arguments); + assert(ret_pid); + assert_se(sigaction(SIGCHLD, &sigaction_nop_nocldstop, NULL) >= 0); assert_se(sigaction(SIGHUP, &sigaction_default, NULL) >= 0); assert_se(sigprocmask_many(SIG_UNBLOCK, NULL, SIGHUP, SIGCHLD) >= 0); @@ -654,12 +658,12 @@ static void terminate_agents(Set *pids) { } static int ask_on_consoles(char *argv[]) { - _cleanup_set_free_ Set *pids = NULL; _cleanup_strv_free_ char **consoles = NULL, **arguments = NULL; - siginfo_t status = {}; - pid_t pid; + _cleanup_set_free_ Set *pids = NULL; int r; + assert(argv); + r = get_kernel_consoles(&consoles); if (r < 0) return log_error_errno(r, "Failed to determine devices of /dev/console: %m"); @@ -674,7 +678,9 @@ static int ask_on_consoles(char *argv[]) { /* Start an agent on each console. */ STRV_FOREACH(tty, consoles) { - r = ask_on_this_console(*tty, &pid, arguments); + pid_t pid; + + r = ask_on_this_console(*tty, arguments, &pid); if (r < 0) return r; @@ -684,22 +690,22 @@ static int ask_on_consoles(char *argv[]) { /* Wait for an agent to exit. */ for (;;) { - zero(status); + siginfo_t status = {}; if (waitid(P_ALL, 0, &status, WEXITED) < 0) { if (errno == EINTR) continue; - return log_error_errno(errno, "waitid() failed: %m"); + return log_error_errno(errno, "Failed to wait for console ask-password agent: %m"); } + if (!is_clean_exit(status.si_code, status.si_status, EXIT_CLEAN_DAEMON, NULL)) + log_error("Password agent failed with: %d", status.si_status); + set_remove(pids, PID_TO_PTR(status.si_pid)); break; } - if (!is_clean_exit(status.si_code, status.si_status, EXIT_CLEAN_DAEMON, NULL)) - log_error("Password agent failed with: %d", status.si_status); - terminate_agents(pids); return 0; } From 4b59f6e63d8df8196a799974cd47c3b92bf20152 Mon Sep 17 00:00:00 2001 From: Mike Yuan Date: Sat, 4 Jan 2025 13:31:28 +0100 Subject: [PATCH 4/4] tty-ask-password-agent: if we're spawning further agents, grant them notify access Follow-up for 254649d5762540ade590909c70c27af86c7dfdac Otherwise, systemd-ask-password-console.service times out being started since the main process never delivers READY=1 notification. Alternative to #35853 --- src/tty-ask-password-agent/tty-ask-password-agent.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/tty-ask-password-agent/tty-ask-password-agent.c b/src/tty-ask-password-agent/tty-ask-password-agent.c index 68b6d1c70e..5ff892c25d 100644 --- a/src/tty-ask-password-agent/tty-ask-password-agent.c +++ b/src/tty-ask-password-agent/tty-ask-password-agent.c @@ -581,7 +581,7 @@ static int ask_on_this_console(const char *tty, char **arguments, pid_t *ret_pid assert_se(sigaction(SIGHUP, &sigaction_default, NULL) >= 0); assert_se(sigprocmask_many(SIG_UNBLOCK, NULL, SIGHUP, SIGCHLD) >= 0); - r = safe_fork("(sd-passwd)", FORK_RESET_SIGNALS|FORK_LOG, ret_pid); + r = safe_fork("(sd-passwd)", FORK_RESET_SIGNALS|FORK_KEEP_NOTIFY_SOCKET|FORK_LOG, ret_pid); if (r < 0) return r; if (r == 0) { @@ -676,6 +676,13 @@ static int ask_on_consoles(char *argv[]) { if (!arguments) return log_oom(); + /* Grant agents we spawn notify access too, so that once an agent establishes inotify watch + * READY=1 from them is accepted by service manager (see process_and_watch_password_files()). + * + * Note that when any agent exits STOPPING=1 would also be sent, but that's utterly what we want, + * i.e. the password is answered on one console and other agents get killed below. */ + (void) sd_notify(/* unset_environment = */ false, "NOTIFYACCESS=all"); + /* Start an agent on each console. */ STRV_FOREACH(tty, consoles) { pid_t pid;