From 942b2f3b84d6ce66b35c6022a4df0c8b89159863 Mon Sep 17 00:00:00 2001 From: Mike Yuan Date: Wed, 15 May 2024 23:26:35 +0800 Subject: [PATCH 1/5] shutdown: explicitly initialize static variables, make arg_verb const --- src/shutdown/shutdown.c | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/src/shutdown/shutdown.c b/src/shutdown/shutdown.c index 67f44e16e9..b1db28e785 100644 --- a/src/shutdown/shutdown.c +++ b/src/shutdown/shutdown.c @@ -52,8 +52,8 @@ #define SYNC_PROGRESS_ATTEMPTS 3 #define SYNC_TIMEOUT_USEC (10*USEC_PER_SEC) -static char* arg_verb; -static uint8_t arg_exit_code; +static const char *arg_verb = NULL; +static uint8_t arg_exit_code = 0; static usec_t arg_timeout = DEFAULT_TIMEOUT_USEC; static int parse_argv(int argc, char *argv[]) { @@ -146,7 +146,7 @@ static int parse_argv(int argc, char *argv[]) { if (!arg_verb) arg_verb = optarg; else - log_error("Excess arguments, ignoring"); + log_warning("Got extraneous arguments, ignoring."); break; case '?': @@ -157,8 +157,7 @@ static int parse_argv(int argc, char *argv[]) { } if (!arg_verb) - return log_error_errno(SYNTHETIC_ERRNO(EINVAL), - "Verb argument missing."); + return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "Verb argument missing."); return 0; } @@ -565,7 +564,7 @@ int main(int argc, char *argv[]) { watchdog_free_device(); arguments[0] = NULL; /* Filled in by execute_directories(), when needed */ - arguments[1] = arg_verb; + arguments[1] = (char*) arg_verb; arguments[2] = NULL; (void) execute_directories(dirs, DEFAULT_TIMEOUT_USEC, NULL, NULL, arguments, NULL, EXEC_DIR_PARALLEL | EXEC_DIR_IGNORE_ERRORS); From 4fbe2bfc51606b2895127520e5c2a3444c414bc0 Mon Sep 17 00:00:00 2001 From: Mike Yuan Date: Wed, 15 May 2024 23:45:43 +0800 Subject: [PATCH 2/5] shutdown: downgrade log level of ignored errors to warning --- src/shutdown/shutdown.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/shutdown/shutdown.c b/src/shutdown/shutdown.c index b1db28e785..115a4b6eb7 100644 --- a/src/shutdown/shutdown.c +++ b/src/shutdown/shutdown.c @@ -85,14 +85,14 @@ static int parse_argv(int argc, char *argv[]) { case ARG_LOG_LEVEL: r = log_set_max_level_from_string(optarg); if (r < 0) - log_error_errno(r, "Failed to parse log level %s, ignoring: %m", optarg); + log_warning_errno(r, "Failed to parse log level %s, ignoring: %m", optarg); break; case ARG_LOG_TARGET: r = log_set_target_from_string(optarg); if (r < 0) - log_error_errno(r, "Failed to parse log target %s, ignoring: %m", optarg); + log_warning_errno(r, "Failed to parse log target %s, ignoring: %m", optarg); break; @@ -101,7 +101,7 @@ static int parse_argv(int argc, char *argv[]) { if (optarg) { r = log_show_color_from_string(optarg); if (r < 0) - log_error_errno(r, "Failed to parse log color setting %s, ignoring: %m", optarg); + log_warning_errno(r, "Failed to parse log color setting %s, ignoring: %m", optarg); } else log_show_color(true); @@ -111,7 +111,7 @@ static int parse_argv(int argc, char *argv[]) { if (optarg) { r = log_show_location_from_string(optarg); if (r < 0) - log_error_errno(r, "Failed to parse log location setting %s, ignoring: %m", optarg); + log_warning_errno(r, "Failed to parse log location setting %s, ignoring: %m", optarg); } else log_show_location(true); @@ -122,7 +122,7 @@ static int parse_argv(int argc, char *argv[]) { if (optarg) { r = log_show_time_from_string(optarg); if (r < 0) - log_error_errno(r, "Failed to parse log time setting %s, ignoring: %m", optarg); + log_warning_errno(r, "Failed to parse log time setting %s, ignoring: %m", optarg); } else log_show_time(true); @@ -131,14 +131,14 @@ static int parse_argv(int argc, char *argv[]) { case ARG_EXIT_CODE: r = safe_atou8(optarg, &arg_exit_code); if (r < 0) - log_error_errno(r, "Failed to parse exit code %s, ignoring: %m", optarg); + log_warning_errno(r, "Failed to parse exit code %s, ignoring: %m", optarg); break; case ARG_TIMEOUT: r = parse_sec(optarg, &arg_timeout); if (r < 0) - log_error_errno(r, "Failed to parse shutdown timeout %s, ignoring: %m", optarg); + log_warning_errno(r, "Failed to parse shutdown timeout %s, ignoring: %m", optarg); break; From 2e4da5e08cb62a75c1027f8012dfbd5e45ba4ec2 Mon Sep 17 00:00:00 2001 From: Mike Yuan Date: Wed, 15 May 2024 23:30:24 +0800 Subject: [PATCH 3/5] shutdown: use execl where appropriate --- src/shutdown/shutdown.c | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/src/shutdown/shutdown.c b/src/shutdown/shutdown.c index 115a4b6eb7..49f986f8c8 100644 --- a/src/shutdown/shutdown.c +++ b/src/shutdown/shutdown.c @@ -624,13 +624,9 @@ int main(int argc, char *argv[]) { r = safe_fork("(sd-kexec)", FORK_RESET_SIGNALS|FORK_CLOSE_ALL_FDS|FORK_LOG|FORK_WAIT, NULL); if (r == 0) { - const char * const args[] = { - KEXEC, "-e", NULL - }; - /* Child */ - execv(args[0], (char * const *) args); + (void) execl(KEXEC, KEXEC, "-e", NULL); log_debug_errno(errno, "Failed to execute '" KEXEC "' binary, proceeding with reboot(RB_KEXEC): %m"); /* execv failed (kexec binary missing?), so try simply reboot(RB_KEXEC) */ From a2d4451e643886fcaa49ebee2c69f41c9bbea601 Mon Sep 17 00:00:00 2001 From: Mike Yuan Date: Wed, 15 May 2024 23:02:02 +0800 Subject: [PATCH 4/5] shutdown: don't freeze() if not executed by pid1 --- src/shutdown/shutdown.c | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/src/shutdown/shutdown.c b/src/shutdown/shutdown.c index 49f986f8c8..1d41b9f8e4 100644 --- a/src/shutdown/shutdown.c +++ b/src/shutdown/shutdown.c @@ -377,8 +377,12 @@ int main(int argc, char *argv[]) { log_set_prohibit_ipc(true); log_parse_environment(); - if (getpid_cached() == 1) - log_set_always_reopen_console(true); + if (getpid_cached() != 1) { + log_error("Not executed by init (PID 1). Refusing to operate."); + return EXIT_FAILURE; + } + + log_set_always_reopen_console(true); r = parse_argv(argc, argv); if (r < 0) @@ -388,11 +392,6 @@ int main(int argc, char *argv[]) { umask(0022); - if (getpid_cached() != 1) { - r = log_error_errno(SYNTHETIC_ERRNO(EPERM), "Not executed by init (PID 1)."); - goto error; - } - if (streq(arg_verb, "reboot")) cmd = RB_AUTOBOOT; else if (streq(arg_verb, "poweroff")) @@ -667,7 +666,7 @@ int main(int argc, char *argv[]) { r = log_error_errno(errno, "Failed to invoke reboot(): %m"); - error: +error: log_struct_errno(LOG_EMERG, r, LOG_MESSAGE("Critical error while doing system shutdown: %m"), "MESSAGE_ID=" SD_MESSAGE_SHUTDOWN_ERROR_STR); From f2c2fa87b6b7a7b3dcce9eeae8ce2a2ab50b60eb Mon Sep 17 00:00:00 2001 From: Mike Yuan Date: Wed, 15 May 2024 23:10:54 +0800 Subject: [PATCH 5/5] shutdown: rename initrd to exitrd Nowadays the tmpfs where the final shutdown phase is initiated has got its own name. Plus, "Returning to initrd" sounds spurious anyway, as we're not returning to the initial root tmpfs as seen by the kernel. --- src/shutdown/shutdown.c | 25 +++++++++++++------------ 1 file changed, 13 insertions(+), 12 deletions(-) diff --git a/src/shutdown/shutdown.c b/src/shutdown/shutdown.c index 1d41b9f8e4..c2b700aad1 100644 --- a/src/shutdown/shutdown.c +++ b/src/shutdown/shutdown.c @@ -360,7 +360,6 @@ int main(int argc, char *argv[]) { NULL }; _cleanup_free_ char *cgroup = NULL; - char *arguments[3]; int cmd, r; /* Close random fds we might have get passed, just for paranoia, before we open any new fds, for @@ -449,11 +448,11 @@ int main(int argc, char *argv[]) { broadcast_signal(SIGKILL, true, false, arg_timeout); bool need_umount = !in_container, need_swapoff = !in_container, need_loop_detach = !in_container, - need_dm_detach = !in_container, need_md_detach = !in_container, can_initrd, last_try = false; - can_initrd = !in_container && !in_initrd() && access("/run/initramfs/shutdown", X_OK) == 0; + need_dm_detach = !in_container, need_md_detach = !in_container, + can_exitrd = !in_container && !in_initrd() && access("/run/initramfs/shutdown", X_OK) >= 0; /* Unmount all mountpoints, swaps, and loopback devices */ - for (;;) { + for (bool last_try = false;;) { bool changed = false; (void) watchdog_ping(); @@ -530,10 +529,10 @@ int main(int argc, char *argv[]) { break; } - if (!changed && !last_try && !can_initrd) { + if (!changed && !last_try && !can_exitrd) { /* There are things we cannot get rid of. Loop one more time in which we will log * with higher priority to inform the user. Note that we don't need to do this if - * there is an initrd to switch to, because that one is likely to get rid of the + * there is an exitrd to switch to, because that one is likely to get rid of the * remaining mounts. If not, it will log about them. */ last_try = true; continue; @@ -562,14 +561,16 @@ int main(int argc, char *argv[]) { * active to guard against any issues during the rest of the shutdown sequence. */ watchdog_free_device(); - arguments[0] = NULL; /* Filled in by execute_directories(), when needed */ - arguments[1] = (char*) arg_verb; - arguments[2] = NULL; - (void) execute_directories(dirs, DEFAULT_TIMEOUT_USEC, NULL, NULL, arguments, NULL, EXEC_DIR_PARALLEL | EXEC_DIR_IGNORE_ERRORS); + const char *arguments[] = { + NULL, /* Filled in by execute_directories(), when needed */ + arg_verb, + NULL, + }; + (void) execute_directories(dirs, DEFAULT_TIMEOUT_USEC, NULL, NULL, (char**) arguments, NULL, EXEC_DIR_PARALLEL | EXEC_DIR_IGNORE_ERRORS); (void) rlimit_nofile_safe(); - if (can_initrd) { + if (can_exitrd) { r = switch_root_initramfs(); if (r >= 0) { argv[0] = (char*) "/shutdown"; @@ -578,7 +579,7 @@ int main(int argc, char *argv[]) { (void) make_console_stdio(); log_info("Successfully changed into root pivot.\n" - "Returning to initrd..."); + "Entering exitrd..."); execv("/shutdown", argv); log_error_errno(errno, "Failed to execute shutdown binary: %m");