]> git.ipfire.org Git - thirdparty/systemd.git/commit
core: Prevent corrupting units from stale alias state on daemon-reload 39703/head
authorChris Down <chris@chrisdown.name>
Fri, 20 Mar 2026 09:48:49 +0000 (17:48 +0800)
committerChris Down <chris@chrisdown.name>
Thu, 2 Apr 2026 13:58:19 +0000 (22:58 +0900)
commita77c7a8224447890a304bd857f412c8103f217f1
treec87097081d255977c400edd6edebeced0e5490a4
parent1b7a42178a7c16d187e105a4f9e524d6ffec369d
core: Prevent corrupting units from stale alias state on daemon-reload

During daemon-reload (or daemon-reexec), when a unit becomes an alias to
another unit, deserialising the alias's stale serialised state can
corrupt the canonical unit's live runtime state.

Consider this scenario:

1. Before reload:
   - a.service is running
   - b.service was stopped earlier and is dead
   - Both exist as independent units

2. User creates an alias to migrate from b -> a:

   - `ln -s /run/systemd/system/a.service /etc/systemd/system/b.service`

3. daemon-reload triggers serialisation. State file contains both units:
   - a.service -> state=running, cgroup=/system.slice/a.service,
     PID=1234, ...
   - b.service -> state=dead, cgroup=(empty), no PIDs, ...

4. During deserialisation:
   - Processes a.service: loads Unit A, deserialises -> state=RUNNING
   - Processes b.service: manager_load_unit() detects symlink, returns
     Unit A
   - unit_deserialize_state(Unit A, ...) overwrites with b's dead state

5. The result is that:
   - Unit A incorrectly shows state=dead despite PID 1234 still running
   - If a.service has Upholds= dependents, catch-up logic sees
     a.service should be running but is dead
   - systemd starts a.service again -> PID 5678
   - Two instances run: PID 1234 (left-over) and PID 5678 (new)

This bug is deterministic when serialisation orders a.service before
b.service.

The root cause is that manager_deserialize_one_unit() calls
manager_load_unit(name, &u) which resolves aliases via
unit_follow_merge(), returning the canonical Unit object. However, the
code doesn't distinguish between two cases when u->id differs from the
requested name from the state file. In the corruption case, we're
deserialising an alias entry and unit_deserialize_state() blindly
overwrites the canonical unit's fields with stale data from the old,
independent unit. The serialised b.service then overwrites Unit A's
correct live state.

This commit first scans the serialised unit names, then adds a check
after manager_load_unit():

    if (!streq(u->id, name) && set_contains(serialized_units, u->id))
        ...

This detects when the loaded unit's canonical ID (u->id) differs from
the serialised name, indicating the name is now an alias for a different
unit and the canonical unit also has its own serialised state entry.

If the canonical unit does not have its own serialised state entry, we
keep the state entry. That handles cases where the old name is really
just a rename, and thus the old name is the only serialised state for
the unit. In that case there is no bug, because there is no separate
canonical state entry for the stale alias entry to overwrite.

Skipping is safe because:

1. The canonical unit's own state entry will be correctly deserialised
   regardless of order. This fix only prevents other stale alias entries
   from corrupting it.
2. unit_merge() has already transferred the necessary data. When
   b.service became an alias during unit loading, unit_merge() already
   migrated dependencies and references to the canonical unit.
3. After merging, the alias doesn't have its own runtime state. The
   serialised data represents b.service when it was independent, which
   is now obsolete once the canonical unit also has its own serialised
   entry.
4. All fields are stale. unit_deserialize_state() would overwrite state,
   timestamps, cgroup paths, pids, etc. There's no scenario where we
   want this data applied on top of the canonical unit's own serialised
   state.

This fix also correctly handles unit precedence. For example, imagine
this scenario:

1. `b.service` is a valid, running unit defined in `/run`.
2. The sysadmin creates `ln -s .../a.service /etc/.../b.service`.
3. On reload, the new symlink in `/etc` overrides the unit in `/run`.

The new perspective from the manager side is that `b.service` is now an
alias for `a.service`.

In this case, systemd correctly abandons the old b.service unit, because
that's the intended general semantics of unit file precedence. We also
do that in other cases, like when a unit file in /etc/systemd/system/
masks a vendor-supplied unit file in /lib/systemd/system/, or when an
admin uses systemctl mask to explicitly disable a unit.

In all these scenarios, the configuration with the highest precedence
(in /etc/) is treated as the new source of truth. The old unit's
definition is discarded, and its running processes are (correctly)
abandoned. In that respect we are not doing anything new here.

Some may ask why we shouldn't just ignore the symlink if we think this
case will come up. I think there are multiple very strong reasons not to
do so:

1. It violates unit precedence. The unit design is built on a strict
   precedence list. When an admin puts any file in /etc, they are
   intentionally overriding everything else. If manager_load_unit were
   to "ignore" this file based on runtime state, it would break this
   fundamental precedent.
2. It makes daemon-reload stateful. daemon-reload is supposed to be a
   simple, stateless operation, basically to read the files on disk and
   apply the new configuration. But doing this would make daemon-reload
   stateful, because we'd have to read the files on disk, but
   cross-reference the current runtime state, and...  maybe ignore some
   files. This is complex and unpredictable.
3. It also completely ignores the user intent. The admin clearly has
   tried to replace the old service with an alias. Ignoring their
   instruction is the opposite of what they want.

Fixes: https://github.com/systemd/systemd/issues/38817
Fixes: https://github.com/systemd/systemd/issues/37482
man/systemctl.xml
src/core/manager-serialize.c
test/units/TEST-07-PID1.alias-corruption.sh [new file with mode: 0755]
test/units/TEST-07-PID1.alias-rename.sh [new file with mode: 0755]