summaryrefslogtreecommitdiffstats
path: root/src/core
diff options
context:
space:
mode:
authorZbigniew Jędrzejewski-Szmek <zbyszek@in.waw.pl>2022-01-27 14:14:24 +0100
committerZbigniew Jędrzejewski-Szmek <zbyszek@in.waw.pl>2022-02-15 11:13:24 +0100
commit5409c6fcc55e6700360546c42edd4a021ee5014e (patch)
treebffbe27adc70736f6b72ecf8dd1a084028826e9a /src/core
parentMerge pull request #22222 from yuwata/dhcp-server-support-non-ethernet-packet (diff)
downloadsystemd-5409c6fcc55e6700360546c42edd4a021ee5014e.tar.xz
systemd-5409c6fcc55e6700360546c42edd4a021ee5014e.zip
manager: do not ignore the return value from the main loop
If manager_loop() fails, we would print an error message, but then actually ignore the error in main(), and potentially execute the shutdown binary. I'm not sure how likely this is to happen in practice, but it seems sloppy. So let's do the cleanup, but actually freeze() if manager_loop() returned an error. invoke_main_loop() is refactored to return the manager objective. This way we don't need to pass a separate parameter to specify whether we are reexecuting. Subsequent patch will make further use of the returned objective.
Diffstat (limited to 'src/core')
-rw-r--r--src/core/main.c75
1 files changed, 39 insertions, 36 deletions
diff --git a/src/core/main.c b/src/core/main.c
index 57aedb9b93..ea2b9b7a52 100644
--- a/src/core/main.c
+++ b/src/core/main.c
@@ -1955,7 +1955,6 @@ static int invoke_main_loop(
Manager *m,
const struct rlimit *saved_rlimit_nofile,
const struct rlimit *saved_rlimit_memlock,
- bool *ret_reexecute,
int *ret_retval, /* Return parameters relevant for shutting down */
const char **ret_shutdown_verb, /* … */
FDSet **ret_fds, /* Return parameters for reexecuting */
@@ -1968,7 +1967,6 @@ static int invoke_main_loop(
assert(m);
assert(saved_rlimit_nofile);
assert(saved_rlimit_memlock);
- assert(ret_reexecute);
assert(ret_retval);
assert(ret_shutdown_verb);
assert(ret_fds);
@@ -1977,13 +1975,13 @@ static int invoke_main_loop(
assert(ret_error_message);
for (;;) {
- r = manager_loop(m);
- if (r < 0) {
+ int objective = manager_loop(m);
+ if (objective < 0) {
*ret_error_message = "Failed to run main loop";
- return log_emergency_errno(r, "Failed to run main loop: %m");
+ return log_emergency_errno(objective, "Failed to run main loop: %m");
}
- switch ((ManagerObjective) r) {
+ switch (objective) {
case MANAGER_RELOAD: {
LogTarget saved_log_target;
@@ -2010,17 +2008,15 @@ static int invoke_main_loop(
if (saved_log_target >= 0)
manager_override_log_target(m, saved_log_target);
- r = manager_reload(m);
- if (r < 0)
+ if (manager_reload(m) < 0)
/* Reloading failed before the point of no return.
* Let's continue running as if nothing happened. */
m->objective = MANAGER_OK;
- break;
+ continue;
}
case MANAGER_REEXECUTE:
-
r = prepare_reexecute(m, &arg_serialization, ret_fds, false);
if (r < 0) {
*ret_error_message = "Failed to prepare for reexecution";
@@ -2029,12 +2025,11 @@ static int invoke_main_loop(
log_notice("Reexecuting.");
- *ret_reexecute = true;
*ret_retval = EXIT_SUCCESS;
*ret_shutdown_verb = NULL;
*ret_switch_root_dir = *ret_switch_root_init = NULL;
- return 0;
+ return objective;
case MANAGER_SWITCH_ROOT:
if (!m->switch_root_init) {
@@ -2048,7 +2043,6 @@ static int invoke_main_loop(
log_notice("Switching root.");
- *ret_reexecute = true;
*ret_retval = EXIT_SUCCESS;
*ret_shutdown_verb = NULL;
@@ -2056,20 +2050,18 @@ static int invoke_main_loop(
*ret_switch_root_dir = TAKE_PTR(m->switch_root);
*ret_switch_root_init = TAKE_PTR(m->switch_root_init);
- return 0;
+ return objective;
case MANAGER_EXIT:
-
if (MANAGER_IS_USER(m)) {
log_debug("Exit.");
- *ret_reexecute = false;
*ret_retval = m->return_value;
*ret_shutdown_verb = NULL;
*ret_fds = NULL;
*ret_switch_root_dir = *ret_switch_root_init = NULL;
- return 0;
+ return objective;
}
_fallthrough_;
@@ -2077,7 +2069,7 @@ static int invoke_main_loop(
case MANAGER_POWEROFF:
case MANAGER_HALT:
case MANAGER_KEXEC: {
- static const char * const table[_MANAGER_OBJECTIVE_MAX] = {
+ static const char* const table[_MANAGER_OBJECTIVE_MAX] = {
[MANAGER_EXIT] = "exit",
[MANAGER_REBOOT] = "reboot",
[MANAGER_POWEROFF] = "poweroff",
@@ -2087,13 +2079,12 @@ static int invoke_main_loop(
log_notice("Shutting down.");
- *ret_reexecute = false;
*ret_retval = m->return_value;
assert_se(*ret_shutdown_verb = table[m->objective]);
*ret_fds = NULL;
*ret_switch_root_dir = *ret_switch_root_init = NULL;
- return 0;
+ return objective;
}
default:
@@ -2709,15 +2700,18 @@ static int save_env(void) {
}
int main(int argc, char *argv[]) {
-
- dual_timestamp initrd_timestamp = DUAL_TIMESTAMP_NULL, userspace_timestamp = DUAL_TIMESTAMP_NULL, kernel_timestamp = DUAL_TIMESTAMP_NULL,
- security_start_timestamp = DUAL_TIMESTAMP_NULL, security_finish_timestamp = DUAL_TIMESTAMP_NULL;
+ dual_timestamp
+ initrd_timestamp = DUAL_TIMESTAMP_NULL,
+ userspace_timestamp = DUAL_TIMESTAMP_NULL,
+ kernel_timestamp = DUAL_TIMESTAMP_NULL,
+ security_start_timestamp = DUAL_TIMESTAMP_NULL,
+ security_finish_timestamp = DUAL_TIMESTAMP_NULL;
struct rlimit saved_rlimit_nofile = RLIMIT_MAKE_CONST(0),
saved_rlimit_memlock = RLIMIT_MAKE_CONST(RLIM_INFINITY); /* The original rlimits we passed
* in. Note we use different values
* for the two that indicate whether
* these fields are initialized! */
- bool skip_setup, loaded_policy = false, queue_default_job = false, first_boot = false, reexecute = false;
+ bool skip_setup, loaded_policy = false, queue_default_job = false, first_boot = false;
char *switch_root_dir = NULL, *switch_root_init = NULL;
usec_t before_startup, after_startup;
static char systemd[] = "systemd";
@@ -3021,16 +3015,23 @@ int main(int argc, char *argv[]) {
goto finish;
}
- (void) invoke_main_loop(m,
- &saved_rlimit_nofile,
- &saved_rlimit_memlock,
- &reexecute,
- &retval,
- &shutdown_verb,
- &fds,
- &switch_root_dir,
- &switch_root_init,
- &error_message);
+ r = invoke_main_loop(m,
+ &saved_rlimit_nofile,
+ &saved_rlimit_memlock,
+ &retval,
+ &shutdown_verb,
+ &fds,
+ &switch_root_dir,
+ &switch_root_init,
+ &error_message);
+ assert(r < 0 || IN_SET(r, MANAGER_EXIT, /* MANAGER_OK is not expected here. */
+ MANAGER_RELOAD,
+ MANAGER_REEXECUTE,
+ MANAGER_REBOOT,
+ MANAGER_POWEROFF,
+ MANAGER_HALT,
+ MANAGER_KEXEC,
+ MANAGER_SWITCH_ROOT));
finish:
pager_close();
@@ -3043,7 +3044,7 @@ finish:
mac_selinux_finish();
- if (reexecute)
+ if (IN_SET(r, MANAGER_REEXECUTE, MANAGER_SWITCH_ROOT))
do_reexecute(argc, argv,
&saved_rlimit_nofile,
&saved_rlimit_memlock,
@@ -3076,7 +3077,9 @@ finish:
__lsan_do_leak_check();
#endif
- if (shutdown_verb) {
+ /* Try to invoke the shutdown binary unless we already failed.
+ * If we failed above, we want to freeze after finishing cleanup. */
+ if (r >= 0 && shutdown_verb) {
r = become_shutdown(shutdown_verb, retval);
log_error_errno(r, "Failed to execute shutdown binary, %s: %m", getpid_cached() == 1 ? "freezing" : "quitting");
error_message = "Failed to execute shutdown binary";