SymbX/systemd
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

core: don't exit 0 when a failure action has nothing to propagate (#43647)

Browse Source 
Fixes #43645.

`FailureAction=exit` propagates the exit status of the unit's main
process. When there is none, `emergency_action()` left `m->return_value`
at its initial 0, so the manager exited 0 for a unit that failed —
indistinguishable from an orderly shutdown to whoever supervises it.
This commit is contained in:
Luca Boccassi
2026-09-05 11:54:29 +01:00
committed by GitHub
parent f789245918 2b42e4f0c9
commit bc17a73107
6 changed files with 98 additions and 7 deletions
Download Patch File Download Diff File Expand all files Collapse all files
man/systemd.unit.xml
+3 -1
View File
@@ -1140,7 +1140,9 @@
semantics. <option>exit</option> causes the manager to exit following the normal shutdown procedure,
and <option>exit-force</option> causes it terminate without shutting down services. When
<option>exit</option> or <option>exit-force</option> 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, <varname>FailureAction=</varname> returns 255 instead, while
<varname>SuccessAction=</varname> leaves the exit status unchanged. However, this may be
overridden with
<varname>FailureActionExitStatus=</varname>/<varname>SuccessActionExitStatus=</varname>, see below.
<option>soft-reboot</option> will trigger a userspace reboot operation.
src/core/emergency-action.c
+9
View File
@@ -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);
src/core/emergency-action.h
+2 -1
View File
@@ -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(
src/core/job.c
+1 -1
View File
@@ -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");
src/core/unit.c
+5 -4
View File
@@ -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;
}
test/units/TEST-07-PID1.issue-43645.sh
Executable
+78
View File
@@ -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" <<EOF
[Unit]
Requires=issue-43645.service
After=issue-43645.service
EOF
cat >"$UNITS/issue-43645.service" <<EOF
[Unit]
FailureAction=exit
[Service]
Type=oneshot
Slice=app.slice
ExecStart=false
EOF
# Move into a cgroup of our own, so that the manager we exec adopts it as its root.
cat >"$UNITS/wrapper.sh" <<EOF
#!/usr/bin/env bash
set -eu
mkdir -p "$CG"
echo \$\$ >"$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