core/swap: don't resurrect swap units whose device link disappeared (#43443)
Fixes #43442. A `.swap` unit can end up **active but unloadable, with no device**, and stopping such a unit aborts PID 1. ### What it looks like ``` $ systemctl show 'dev-disk-byx2dpartlabel-primary.swap' -p LoadState,ActiveState,What LoadState=not-found ActiveState=active What= # <-- no device $ systemctl stop 'dev-disk-byx2dpartlabel-primary.swap' systemd[1]: About to execute: /usr/sbin/swapoff # <-- no argument systemd[1]: Caught <ABRT>, from our own process. systemd[1]: Freezing execution. ``` PID 1 stays alive but frozen, so the machine stops answering `systemctl` and never reboots. ### How it happens 1. systemd creates one `.swap` unit per udev device link of every device in `/proc/swaps`. 2. A device link gets repointed at another device while the swap stays on. `/proc/swaps` doesn't change, so nothing re-examines the unit — it stays active. 3. The next `daemon-reload` drops all units and rebuilds them. Enumeration from `/proc/swaps` no longer produces this name, so only the deserialization brings the unit back: by name, with no fragment on disk. 4. `swap_load()` only calls `swap_add_extras()` for units that loaded a fragment or are backed by `/proc/swaps`, so this one gets neither a `What=` nor a patched `ExecContext`. 5. `swap_coldplug()` restores its active state anyway. 6. Shutdown stops it. `unit_stop()` deliberately permits stopping units whose load failed, so it reaches `exec_spawn()` and trips the `PrivateVarTmp=` assertion in `exec_context_serialize()`.
This commit is contained in:
Yu Watanabe
@@ -1724,8 +1724,13 @@ static int exec_context_serialize(const ExecContext *c, FILE *f) {
|
||||
if (r < 0)
|
||||
return r;
|
||||
|
||||
/* This must be set in unit_patch_contexts() before executing a command. */
|
||||
assert(c->private_var_tmp >= 0 && c->private_var_tmp < _PRIVATE_TMP_MAX);
|
||||
/* This must be set in unit_patch_contexts() before executing a command. A unit whose load failed
|
||||
* never goes through that, yet can still be stopped, hence this is reachable. Refuse instead of
|
||||
* asserting, this runs in PID 1. */
|
||||
if (c->private_var_tmp < 0 || c->private_var_tmp >= _PRIVATE_TMP_MAX)
|
||||
return log_debug_errno(SYNTHETIC_ERRNO(EINVAL),
|
||||
"PrivateVarTmp= is not set up, unit was not fully loaded, refusing.");
|
||||
|
||||
r = serialize_item(f, "exec-context-private-var-tmp", private_tmp_to_string(c->private_var_tmp));
|
||||
if (r < 0)
|
||||
return r;
|
||||
|
||||
@@ -566,6 +566,23 @@ static int swap_coldplug(Unit *u) {
|
||||
else if (s->from_proc_swaps)
|
||||
new_state = SWAP_ACTIVE;
|
||||
|
||||
/* If /proc/swaps doesn't know this unit, and we couldn't load a fragment for it either, then it is
|
||||
* a leftover of the serialization describing a swap that doesn't exist anymore — typically because
|
||||
* the device link the unit is named after has been repointed at some other device in the meantime.
|
||||
* Don't adopt such a unit: it has no device we could pass to swapoff, and since active units are
|
||||
* never garbage collected it would stick around until the end of time. Note that units with a
|
||||
* fragment are explicitly *not* covered by this, as those are not backed by /proc/swaps to begin
|
||||
* with when their name doesn't match What=.
|
||||
*
|
||||
* Units with a control process still around are left alone too: we want to re-adopt the process
|
||||
* and re-arm the timeout rather than drop both on the floor. Once it is gone the state machine
|
||||
* ends up in dead anyway, as the unit isn't backed by /proc/swaps. */
|
||||
if (!s->from_proc_swaps && u->load_state != UNIT_LOADED && !SWAP_STATE_WITH_PROCESS(new_state)) {
|
||||
log_unit_debug(u, "Not backed by /proc/swaps and has no fragment, ignoring.");
|
||||
unit_add_to_gc_queue(u);
|
||||
return 0;
|
||||
}
|
||||
|
||||
if (new_state == s->state)
|
||||
return 0;
|
||||
|
||||
@@ -856,6 +873,17 @@ static void swap_enter_deactivating(Swap *s) {
|
||||
|
||||
assert(s);
|
||||
|
||||
/* A swap unit can be active without ever having been loaded, and hence without a device: an
|
||||
* orphaned unit that still has a swapon running is adopted by swap_coldplug() on purpose, and
|
||||
* swap_sigchld_event() moves it to SWAP_ACTIVE once that process exits successfully. There is
|
||||
* nothing to deactivate for such a unit, and calling swapoff without an argument certainly isn't
|
||||
* it — go to dead, like swap_process_proc_swaps() does when a unit disappears from /proc/swaps. */
|
||||
if (isempty(s->what)) {
|
||||
log_unit_warning(UNIT(s), "Swap unit has no device, assuming it is already deactivated.");
|
||||
swap_enter_dead(s, SWAP_SUCCESS);
|
||||
return;
|
||||
}
|
||||
|
||||
s->control_command_id = SWAP_EXEC_DEACTIVATE;
|
||||
s->control_command = s->exec_command + SWAP_EXEC_DEACTIVATE;
|
||||
|
||||
|
||||
Executable
+129
@@ -0,0 +1,129 @@
|
||||
#!/usr/bin/env bash
|
||||
# SPDX-License-Identifier: LGPL-2.1-or-later
|
||||
set -eux
|
||||
set -o pipefail
|
||||
|
||||
# Issue: https://github.com/systemd/systemd/issues/43442
|
||||
#
|
||||
# A daemon-reload must not resurrect a .swap unit whose device link has disappeared.
|
||||
#
|
||||
# systemd creates one .swap unit per udev device link of every device listed in /proc/swaps. When such a
|
||||
# device link goes away (e.g. because it is not unique and udev repoints it at another partition) while the
|
||||
# swap itself stays on, the unit named after the link is left behind: the next daemon-reload drops all units
|
||||
# and rebuilds them, the enumeration from /proc/swaps no longer knows the link, and the unit is only
|
||||
# resurrected by the deserialization, i.e. by name, with no fragment on disk. That used to leave it
|
||||
#
|
||||
# LoadState=not-found, ActiveState=active, What=
|
||||
|
||||
# shellcheck source=test/units/util.sh
|
||||
. "$(dirname "$0")"/util.sh
|
||||
|
||||
LOOPDEV=""
|
||||
UDEV_RULE=/run/udev/rules.d/99-testsuite-orphan-swap.rules
|
||||
DEVLINK=/dev/testsuite-orphan-swap
|
||||
SWAP_UNIT="$(systemd-escape --path --suffix=swap "$DEVLINK")"
|
||||
FRAGMENT="/run/systemd/system/$SWAP_UNIT"
|
||||
# Not /tmp/, that's a tmpfs here: swapping onto a loop device backed by memory that can itself be swapped
|
||||
# out is a deadlock waiting to happen.
|
||||
WORKDIR="$(mktemp -d -p /var/tmp)"
|
||||
|
||||
at_exit() (
|
||||
set +e
|
||||
|
||||
[[ -n "$LOOPDEV" ]] && swapoff "$LOOPDEV"
|
||||
|
||||
rm -f "$FRAGMENT" "$UDEV_RULE"
|
||||
udevadm control --reload
|
||||
|
||||
if [[ -z "$LOOPDEV" ]]; then
|
||||
rm -rf "$WORKDIR"
|
||||
else
|
||||
timeout 30 udevadm trigger --settle --action change "$LOOPDEV"
|
||||
|
||||
# Neither detach the loop device nor unlink its backing file while the swap is still on: the
|
||||
# /proc/swaps entry outlives both, naming a device node that is gone, and leaks into the subtests
|
||||
# that follow in the same boot.
|
||||
if swapon --show=NAME --noheadings | grep -x "$LOOPDEV" >/dev/null; then
|
||||
echo "$LOOPDEV is still swapped on, leaving it and $WORKDIR behind" >&2
|
||||
else
|
||||
losetup -d "$LOOPDEV"
|
||||
rm -rf "$WORKDIR"
|
||||
fi
|
||||
fi
|
||||
)
|
||||
|
||||
trap at_exit EXIT
|
||||
|
||||
truncate -s 64M "$WORKDIR"/swap.img
|
||||
LOOPDEV="$(losetup --show --find "$WORKDIR"/swap.img)"
|
||||
mkswap "$LOOPDEV"
|
||||
|
||||
# Give the loop device an extra device link, so that systemd creates a second .swap unit named after it
|
||||
# once the swap is on.
|
||||
mkdir -p /run/udev/rules.d
|
||||
cat >"$UDEV_RULE" <<EOF
|
||||
SUBSYSTEM=="block", KERNEL=="${LOOPDEV#/dev/}", SYMLINK+="${DEVLINK#/dev/}"
|
||||
EOF
|
||||
udevadm control --reload
|
||||
timeout 30 udevadm trigger --settle --action change "$LOOPDEV"
|
||||
udevadm wait --timeout=30 --settle "$DEVLINK"
|
||||
|
||||
LOOP_SWAP_UNIT="$(systemd-escape --path --suffix=swap "$LOOPDEV")"
|
||||
|
||||
swapon --fixpgsz "$LOOPDEV"
|
||||
swapon --show
|
||||
|
||||
# shellcheck disable=SC2016
|
||||
timeout 30 bash -c 'until systemctl -q is-active "$1"; do sleep .5; done' bash "$SWAP_UNIT"
|
||||
assert_eq "$(systemctl show -P LoadState "$SWAP_UNIT")" "loaded"
|
||||
assert_eq "$(systemctl show -P What "$SWAP_UNIT")" "$DEVLINK"
|
||||
|
||||
# Now drop the device link again, without touching the swap itself, and reload. /proc/swaps still lists the
|
||||
# loop device, but nothing knows the unit name anymore.
|
||||
rm -f "$UDEV_RULE"
|
||||
udevadm control --reload
|
||||
timeout 30 udevadm trigger --settle --action change "$LOOPDEV"
|
||||
udevadm wait --timeout=30 --settle --removed "$DEVLINK"
|
||||
assert_not_in "${DEVLINK#/dev/}" "$(udevadm info --query=symlink "$LOOPDEV")"
|
||||
|
||||
systemctl daemon-reload
|
||||
|
||||
# The unit is orphaned now: /proc/swaps doesn't know it, and there's no fragment on disk either. It must
|
||||
# not have been brought back as active
|
||||
systemctl show "$SWAP_UNIT" --property=Id,LoadState,ActiveState,SubState,What
|
||||
assert_eq "$(systemctl show -P ActiveState "$SWAP_UNIT")" "inactive"
|
||||
|
||||
# The unit named after the loop device itself is backed by /proc/swaps and must have survived, i.e. the
|
||||
# guard must not be over-broad.
|
||||
assert_rc 0 systemctl -q is-active "$LOOP_SWAP_UNIT"
|
||||
|
||||
# Now the same thing once more, except that this time the unit has a fragment on disk. Being loadable is
|
||||
# the other half of what the guard keys on, so this one has to survive losing its device link. Note the
|
||||
# fragment must be named after What=, swap_verify() insists on it.
|
||||
cat >"$FRAGMENT" <<EOF
|
||||
[Swap]
|
||||
What=$DEVLINK
|
||||
EOF
|
||||
cat >"$UDEV_RULE" <<EOF
|
||||
SUBSYSTEM=="block", KERNEL=="${LOOPDEV#/dev/}", SYMLINK+="${DEVLINK#/dev/}"
|
||||
EOF
|
||||
udevadm control --reload
|
||||
timeout 30 udevadm trigger --settle --action change "$LOOPDEV"
|
||||
udevadm wait --timeout=30 --settle "$DEVLINK"
|
||||
|
||||
systemctl daemon-reload
|
||||
assert_eq "$(systemctl show -P LoadState "$SWAP_UNIT")" "loaded"
|
||||
assert_rc 0 systemctl -q is-active "$SWAP_UNIT"
|
||||
|
||||
rm -f "$UDEV_RULE"
|
||||
udevadm control --reload
|
||||
timeout 30 udevadm trigger --settle --action change "$LOOPDEV"
|
||||
udevadm wait --timeout=30 --settle --removed "$DEVLINK"
|
||||
|
||||
systemctl daemon-reload
|
||||
systemctl show "$SWAP_UNIT" --property=Id,LoadState,ActiveState,SubState,What
|
||||
assert_eq "$(systemctl show -P LoadState "$SWAP_UNIT")" "loaded"
|
||||
assert_rc 0 systemctl -q is-active "$SWAP_UNIT"
|
||||
|
||||
# The real swap is untouched
|
||||
swapon --show=NAME --noheadings | grep -x "$LOOPDEV" >/dev/null
|
||||
Reference in New Issue
Block a user