diff --git a/man/systemd.unit.xml b/man/systemd.unit.xml index b9a346897e..7c1704f160 100644 --- a/man/systemd.unit.xml +++ b/man/systemd.unit.xml @@ -1140,7 +1140,9 @@ semantics. causes the manager to exit following the normal shutdown procedure, and causes it terminate without shutting down services. When or is used by default the exit status of the main - process of the unit (if this applies) is returned from the service manager. However, this may be + process of the unit (if this applies) is returned from the service manager. If there is no such + status, FailureAction= returns 255 instead, while + SuccessAction= leaves the exit status unchanged. However, this may be overridden with FailureActionExitStatus=/SuccessActionExitStatus=, see below. will trigger a userspace reboot operation. diff --git a/src/core/emergency-action.c b/src/core/emergency-action.c index 181da27f7a..8f0dc422ad 100644 --- a/src/core/emergency-action.c +++ b/src/core/emergency-action.c @@ -4,6 +4,7 @@ #include "ansi-color.h" #include "emergency-action.h" +#include "exit-status.h" #include "manager.h" #include "reboot-util.h" #include "special.h" @@ -154,6 +155,10 @@ void emergency_action( if (exit_status >= 0) m->return_value = exit_status; + else if (FLAGS_SET(flags, EMERGENCY_ACTION_IS_FAILURE) && m->return_value == EXIT_SUCCESS) + /* Nothing to propagate, but we act on a failure: don't report success. An exit + * status set explicitly via SetExitCode() stays. */ + m->return_value = EXIT_EXCEPTION; if (MANAGER_IS_USER(m) || detect_container() > 0) { log_and_status(m, action, flags, "Exiting", reason); @@ -173,6 +178,10 @@ void emergency_action( if (exit_status >= 0) m->return_value = exit_status; + else if (FLAGS_SET(flags, EMERGENCY_ACTION_IS_FAILURE) && m->return_value == EXIT_SUCCESS) + /* Nothing to propagate, but we act on a failure: don't report success. An exit + * status set explicitly via SetExitCode() stays. */ + m->return_value = EXIT_EXCEPTION; if (MANAGER_IS_USER(m) || detect_container() > 0) { log_and_status(m, action, flags, "Exiting immediately", reason); diff --git a/src/core/emergency-action.h b/src/core/emergency-action.h index 17f306507f..3d9029414b 100644 --- a/src/core/emergency-action.h +++ b/src/core/emergency-action.h @@ -29,7 +29,8 @@ typedef enum EmergencyActionFlags { EMERGENCY_ACTION_IS_WATCHDOG = 1 << 0, /* this action triggered by a watchdog or other kind of timeout */ EMERGENCY_ACTION_WARN = 1 << 1, /* log at LOG_WARNING + write to system console */ EMERGENCY_ACTION_SLEEP_5S = 1 << 2, /* wait 5s before executing action; only honoured together with EMERGENCY_ACTION_WARN */ - _EMERGENCY_ACTION_FLAGS_MAX = (1 << 3) - 1, + EMERGENCY_ACTION_IS_FAILURE = 1 << 3, /* this action is triggered by a failure, hence must not make us exit 0 */ + _EMERGENCY_ACTION_FLAGS_MAX = (1 << 4) - 1, } EmergencyActionFlags; void emergency_action( diff --git a/src/core/job.c b/src/core/job.c index 2388d1de95..70e14b517f 100644 --- a/src/core/job.c +++ b/src/core/job.c @@ -1141,7 +1141,7 @@ static int job_dispatch_timer(sd_event_source *s, uint64_t monotonic, void *user emergency_action( u->manager, u->job_timeout_action, - EMERGENCY_ACTION_IS_WATCHDOG|EMERGENCY_ACTION_WARN|EMERGENCY_ACTION_SLEEP_5S, + EMERGENCY_ACTION_IS_WATCHDOG|EMERGENCY_ACTION_WARN|EMERGENCY_ACTION_SLEEP_5S|EMERGENCY_ACTION_IS_FAILURE, u->job_timeout_reboot_arg, /* exit_status= */ -1, "job timed out"); diff --git a/src/core/unit.c b/src/core/unit.c index 31d3740d0f..28a4af186b 100644 --- a/src/core/unit.c +++ b/src/core/unit.c @@ -29,6 +29,7 @@ #include "escape.h" #include "exec-credential.h" #include "execute.h" +#include "exit-status.h" #include "fd-util.h" #include "fileio.h" #include "format-util.h" @@ -1887,7 +1888,7 @@ int unit_test_start_limit(Unit *u) { emergency_action( u->manager, u->start_limit_action, - EMERGENCY_ACTION_IS_WATCHDOG|EMERGENCY_ACTION_WARN|EMERGENCY_ACTION_SLEEP_5S, + EMERGENCY_ACTION_IS_WATCHDOG|EMERGENCY_ACTION_WARN|EMERGENCY_ACTION_SLEEP_5S|EMERGENCY_ACTION_IS_FAILURE, u->reboot_arg, /* exit_status= */ -1, reason); @@ -2843,7 +2844,7 @@ void unit_notify(Unit *u, UnitActiveState os, UnitActiveState ns, bool reload_su if (os != UNIT_FAILED && ns == UNIT_FAILED) { reason = strjoina("unit ", u->id, " failed"); - emergency_action(m, u->failure_action, EMERGENCY_ACTION_WARN|EMERGENCY_ACTION_SLEEP_5S, u->reboot_arg, unit_failure_action_exit_status(u), reason); + emergency_action(m, u->failure_action, EMERGENCY_ACTION_WARN|EMERGENCY_ACTION_SLEEP_5S|EMERGENCY_ACTION_IS_FAILURE, u->reboot_arg, unit_failure_action_exit_status(u), reason); } else if (!UNIT_IS_INACTIVE_OR_FAILED(os) && ns == UNIT_INACTIVE) { reason = strjoina("unit ", u->id, " succeeded"); emergency_action(m, u->success_action, /* flags= */ 0, u->reboot_arg, unit_success_action_exit_status(u), reason); @@ -6410,7 +6411,7 @@ int unit_failure_action_exit_status(Unit *u) { r = unit_exit_status(u); if (r == -EBADE) /* Exited, but not cleanly (i.e. by signal or such) */ - return 255; + return EXIT_EXCEPTION; return r; } @@ -6427,7 +6428,7 @@ int unit_success_action_exit_status(Unit *u) { r = unit_exit_status(u); if (r == -EBADE) /* Exited, but not cleanly (i.e. by signal or such) */ - return 255; + return EXIT_EXCEPTION; return r; } diff --git a/test/units/TEST-07-PID1.issue-43645.sh b/test/units/TEST-07-PID1.issue-43645.sh new file mode 100755 index 0000000000..26529f95dc --- /dev/null +++ b/test/units/TEST-07-PID1.issue-43645.sh @@ -0,0 +1,78 @@ +#!/usr/bin/env bash +# SPDX-License-Identifier: LGPL-2.1-or-later + +set -eux +set -o pipefail + +# shellcheck source=test/units/util.sh +. "$(dirname "$0")"/util.sh + +# FailureAction=exit made the manager exit 0 when the unit failed before any of its +# processes could run. +# Issue: https://github.com/systemd/systemd/issues/43645 + +UNITS="$(mktemp -d)" +XDG="$(mktemp -d)" +CG="/sys/fs/cgroup$(cut -d: -f3 /proc/self/cgroup)/issue-43645" + +cleanup() { + rm -rf "$UNITS" "$XDG" || : + rmdir "$CG/app.slice" "$CG" 2>/dev/null || : +} + +trap cleanup EXIT + +cp /usr/lib/systemd/user/{basic,exit,paths,shutdown,sockets,timers}.target \ + /usr/lib/systemd/user/systemd-exit.service "$UNITS/" + +cat >"$UNITS/issue-43645.target" <"$UNITS/issue-43645.service" <"$UNITS/wrapper.sh" <"$CG/cgroup.procs" +exec "\$@" +EOF +chmod +x "$UNITS/wrapper.sh" + +# Runs a nested user manager, so that we can observe what it exits with. +run_manager() { + local rc=0 + + rm -rf "$XDG" || : + mkdir -p "$XDG" + chmod 0700 "$XDG" + + XDG_RUNTIME_DIR="$XDG" SYSTEMD_UNIT_PATH="$UNITS" \ + timeout 60 "$UNITS/wrapper.sh" /usr/lib/systemd/systemd --user --unit=issue-43645.target || rc=$? + + return $rc +} + +# ExecStart= runs and fails: its exit status is propagated. +assert_rc 1 run_manager + +# Now forbid the slice the unit lands in any descendants, so that creating the unit's +# own cgroup fails. An ENOMEM fails the very same mkdir(). +mkdir -p "$CG/app.slice" +echo 0 >"$CG/app.slice/cgroup.max.descendants" + +# The unit now fails before ExecStart= could run: no exit status to propagate, but the +# manager must still not report success. +assert_rc 255 run_manager