core: Prevent corrupting units from stale alias state on daemon-reload (#39703)

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

@@ -1499,6 +1499,17 @@ Jan 12 10:46:45 example.com bluetoothd[8900]: gatt-time-server: Input/output err
             systemd listens on behalf of user configuration will stay
             accessible.</para>
 
+            <para>When unit aliasing is introduced during reload (e.g., converting
+            <filename>b.service</filename> to a symlink pointing to
+            <filename>a.service</filename>), the running state of the canonical
+            unit (<filename>a.service</filename>) is preserved. The old serialized
+            state of the now-aliased unit is discarded to prevent stale data from
+            corrupting the canonical unit's live state. Dependencies referencing
+            the alias name are automatically resolved to the canonical unit, and
+            the dependency graph is rebuilt from unit files, ensuring consistency.
+            If the now-aliased unit had running processes, they are abandoned and
+            will no longer be tracked by the service manager.</para>
+
             <para>This command should not be confused with the
             <command>reload</command> command.</para>
           </listitem>

src/core/manager-serialize.c

@@ -14,6 +14,7 @@
 #include "manager-serialize.h"
 #include "parse-util.h"
 #include "serialize.h"
+#include "set.h"
 #include "string-util.h"
 #include "strv.h"
 #include "syslog-util.h"
@@ -197,7 +198,50 @@ int manager_serialize(
         return 0;
 }
 
-static int manager_deserialize_one_unit(Manager *m, const char *name, FILE *f, FDSet *fds) {
+static int manager_collect_serialized_unit_names(FILE *f, Set **ret) {
+        _cleanup_set_free_ Set *serialized_units = NULL;
+        off_t offset;
+        int r;
+
+        assert(f);
+        assert(ret);
+
+        offset = ftello(f);
+        if (offset < 0)
+                return log_error_errno(errno, "Failed to determine serialization offset: %m");
+
+        for (;;) {
+                _cleanup_free_ char *line = NULL;
+
+                r = read_stripped_line(f, LONG_LINE_MAX, &line);
+                if (r < 0)
+                        return log_error_errno(r, "Failed to read serialization line: %m");
+                if (r == 0)
+                        break;
+
+                r = set_ensure_consume(&serialized_units, &string_hash_ops_free, TAKE_PTR(line));
+                if (r < 0)
+                        return log_oom();
+
+                r = unit_deserialize_state_skip(f);
+                if (r < 0)
+                        return r;
+        }
+
+        if (fseeko(f, offset, SEEK_SET) < 0)
+                return log_error_errno(errno, "Failed to reset serialization offset: %m");
+
+        *ret = TAKE_PTR(serialized_units);
+        return 0;
+}
+
+static int manager_deserialize_one_unit(
+                Manager *m,
+                const char *name,
+                FILE *f,
+                FDSet *fds,
+                Set *serialized_units) {
+
         Unit *u;
         int r;
 
@@ -207,18 +251,75 @@ static int manager_deserialize_one_unit(Manager *m, const char *name, FILE *f, F
         if (r < 0)
                 return log_notice_errno(r, "Failed to load unit \"%s\", skipping deserialization: %m", name);
 
+        if (!streq(u->id, name) &&
+            set_contains(serialized_units, u->id)) {
+                /*
+                 * The unit from the state file (name) resolved to a different canonical unit (u->id), and
+                 * the canonical unit also has its own state entry.
+                 *
+                 * This means the state entry for the unit name is stale. That is, when the state was
+                 * serialized, the name referred to an independent unit, but it now resolves as an alias to
+                 * the canonical unit. Deserializing it would overwrite the canonical unit's own serialized
+                 * state, and thus corrupt its live runtime state.
+                 *
+                 * It is very important to note that this only affects units that were independent when the
+                 * state file was written, but are now aliases (either because a reload created the symlink,
+                 * or the symlink existed but this is the first reload). Normal aliases that were already
+                 * aliases during the most recent serialization are filtered out in manager_serialize(), so
+                 * they never appear in the state file.
+                 *
+                 * If the canonical unit does not have its own state entry, then this is instead a rename or
+                 * canonical ID change, and this state entry is the only state we have for the unit. In that
+                 * case we must preserve it. After doing so, we insert the canonical unit's ID into the set
+                 * so that any further aliases resolving to the same unit are skipped.
+                 *
+                 * The serialized data represents the old, independent unit. Deserializing this stale state
+                 * would corrupt the canonical unit's live state, so we must discard it.
+                 *
+                 * Take as an example, a.service is running. Someone created symlink b.service -> a.service.
+                 * On first reload, the state file still has b.service as an independent dead unit (from
+                 * before the symlink existed), but b.service now resolves to a.service. We must discard
+                 * b.service's stale dead state to preserve a.service's running state.
+                 *
+                 * Note: This log message is checked in TEST-07-PID1.alias-corruption.sh, so the test case
+                 * may need adjustment if the message is changed.
+                 */
+                log_warning("Unit file for '%s' was overridden by a symlink to '%s', which also has serialized state. Skipping stale state of old unit. Any processes from the overridden unit are now abandoned!",
+                            name,
+                            u->id);
+
+                return unit_deserialize_state_skip(f);
+        }
+
         r = unit_deserialize_state(u, f, fds);
         if (r == -ENOMEM)
                 return log_oom();
         if (r < 0)
                 return log_notice_errno(r, "Failed to deserialize unit \"%s\", skipping: %m", name);
 
+        /* If this unit was deserialized under an alias name (that is, it is a rename), record the canonical
+         * ID so that any further aliases pointing to the same unit are correctly skipped. */
+        if (!streq(u->id, name)) {
+                r = set_put_strdup(&serialized_units, u->id);
+                if (r < 0)
+                        return log_oom();
+        }
+
         return 0;
 }
 
-static int manager_deserialize_units(Manager *m, FILE *f, FDSet *fds) {
+static int manager_deserialize_units(
+                Manager *m,
+                FILE *f,
+                FDSet *fds) {
+
+        _cleanup_set_free_ Set *serialized_units = NULL;
         int r;
 
+        r = manager_collect_serialized_unit_names(f, &serialized_units);
+        if (r < 0)
+                return r;
+
         for (;;) {
                 _cleanup_free_ char *line = NULL;
 
@@ -229,7 +330,7 @@ static int manager_deserialize_units(Manager *m, FILE *f, FDSet *fds) {
                 if (r == 0)
                         break;
 
-                r = manager_deserialize_one_unit(m, line, f, fds);
+                r = manager_deserialize_one_unit(m, line, f, fds, serialized_units);
                 if (r == -ENOMEM)
                         return r;
                 if (r < 0) {

test/units/TEST-07-PID1.alias-corruption.sh

@@ -0,0 +1,229 @@
+#!/usr/bin/env bash
+# SPDX-License-Identifier: LGPL-2.1-or-later
+set -eux
+set -o pipefail
+
+# Verify that stale alias state doesn't overwrite canonical unit state.
+# 1. Legit unit is running (PID A).
+# 2. Sus units are running (PID B, C, D...).
+# 3. We alias sus -> legit.
+# 4. If the bug triggers, legit unit's state is overwritten by a sus unit's state.
+# 5. Legit unit thinks it is now PID B (or C, or D...).
+# 6. We detect this PID change as proof of corruption.
+
+declare -a abandoned_pids=()
+
+reap_abandoned_pids() {
+    local pid attempt
+
+    if (( ${#abandoned_pids[@]} == 0 )); then
+        return 0
+    fi
+
+    echo "Reaping ${#abandoned_pids[@]} abandoned processes..."
+
+    for pid in "${abandoned_pids[@]}"; do
+        kill "$pid" 2>/dev/null || true
+    done
+
+    for pid in "${abandoned_pids[@]}"; do
+        for attempt in $(seq 1 50); do
+            if ! kill -0 "$pid" 2>/dev/null; then
+                break
+            fi
+
+            sleep 0.1
+        done
+
+        if kill -0 "$pid" 2>/dev/null; then
+            kill -KILL "$pid" 2>/dev/null || true
+        fi
+
+        for attempt in $(seq 1 50); do
+            if ! kill -0 "$pid" 2>/dev/null; then
+                break
+            fi
+
+            sleep 0.1
+        done
+
+        if kill -0 "$pid" 2>/dev/null; then
+            echo "ERROR: Failed to reap abandoned process PID $pid"
+            return 1
+        fi
+    done
+
+    abandoned_pids=()
+}
+
+run_test() {
+    local reload_cmd="${1:?}"
+    local current_pid journal_warnings new_pid orig_pid pid reload_start unit warning_count
+
+    echo ""
+    echo "========================================="
+    echo "Testing with: systemctl $reload_cmd"
+    echo "========================================="
+
+    cat >/run/systemd/system/legit.service <<'EOF'
+[Service]
+Type=simple
+ExecStart=/bin/sleep infinity
+EOF
+
+    # Create 20 sus units. They must be Type=simple/running so systemd
+    # CANNOT garbage collect them. If they are dead/stopped, systemd can remove
+    # them from memory before serialization
+    echo "Creating 20 sus units..."
+    for i in $(seq -f "%02g" 1 20); do
+        cat >/run/systemd/system/sus-"${i}".service <<'EOF'
+[Service]
+Type=simple
+ExecStart=/bin/sleep infinity
+EOF
+    done
+
+    systemctl daemon-reload
+
+    echo "Starting legit unit..."
+    systemctl start legit.service
+
+    echo "Starting sus units..."
+    for i in $(seq -f "%02g" 1 20); do
+        systemctl start sus-"${i}".service
+    done
+
+    echo "Setup complete: 1 running legit unit, 20 running sus units"
+
+    orig_pid=$(systemctl show -P MainPID legit.service)
+    echo "Original legit PID: $orig_pid"
+
+    if (( orig_pid == 0 )); then
+        echo "Error: Legit PID is 0, setup failed."
+        return 1
+    fi
+
+    # Since ordering is not deterministic we should loop 3 times to reduce
+    # false negative rate (ordering luck). With this it's roughly 0.01% chance
+    # of falsely passing. Falsely failing does not happen, though.
+    for attempt in 1 2 3; do
+        echo ""
+        echo "--- Attempt $attempt/3 ---"
+
+        unset sus_pids
+        declare -A sus_pids
+        for i in $(seq -f "%02g" 1 20); do
+            pid=$(systemctl show -P MainPID sus-"${i}".service)
+            if (( pid != 0 )); then
+                sus_pids["sus-${i}"]=$pid
+                abandoned_pids+=("$pid")
+                echo "sus-${i}.service PID: $pid"
+            fi
+        done
+
+        echo "Converting sus units to symlinks -> legit.service..."
+        for i in $(seq -f "%02g" 1 20); do
+            rm -f /run/systemd/system/sus-"${i}".service
+            ln -sf /run/systemd/system/legit.service /run/systemd/system/sus-"${i}".service
+        done
+
+        reload_start=$(date '+%Y-%m-%d %H:%M:%S')
+
+        echo "Running $reload_cmd..."
+        systemctl "$reload_cmd"
+
+        # If the bug triggered, legit.service deserialized a sus unit's state
+        # and overwrote its own MainPID with the sus unit's PID.
+        new_pid=$(systemctl show -P MainPID legit.service)
+
+        if [[ "$new_pid" != "$orig_pid" ]]; then
+            echo "legit.service PID changed from $orig_pid to $new_pid!"
+            echo "The stale alias state corrupted the canonical unit."
+            return 1
+        fi
+
+        echo "legit.service PID remains $new_pid. Attempt $attempt passed."
+
+        # Verify that all sus unit processes were abandoned (still running but no longer tracked)
+        echo "Verifying sus unit processes were abandoned..."
+        for unit in "${!sus_pids[@]}"; do
+            pid=${sus_pids[$unit]}
+            # Process should still be running
+            if ! kill -0 "$pid" 2>/dev/null; then
+                echo "ERROR: $unit process (PID $pid) was killed instead of abandoned!"
+                return 1
+            fi
+            # But the alias should now either be inactive (MainPID=0) or resolve to legit's PID.
+            current_pid=$(systemctl show -P MainPID "${unit}.service")
+            if ! (( current_pid == 0 || current_pid == new_pid )); then
+                echo "ERROR: $unit unexpectedly reports MainPID=$current_pid after aliasing!"
+                return 1
+            fi
+            echo "$unit process (PID $pid) was correctly abandoned (still running, no longer tracked)"
+        done
+
+        # Check consistency between journal warnings and abandoned processes
+        echo "Checking journal for stale state warnings..."
+        journal_warnings=$(journalctl --since "$reload_start" --no-pager | grep "Skipping stale state" || true)
+        warning_count=$(echo "$journal_warnings" | grep -c "Skipping stale state" || true)
+
+        echo "Found $warning_count 'Skipping stale state' warnings"
+
+        # Extract unit names from warnings and verify they match our sus units
+        if (( warning_count > 0 )); then
+            echo "Verifying warning consistency..."
+            for unit in "${!sus_pids[@]}"; do
+                if [[ "$journal_warnings" != *"${unit}.service"* ]]; then
+                    echo "WARNING: Expected journal warning for ${unit}.service but didn't find it"
+                fi
+            done
+        fi
+
+        reap_abandoned_pids
+
+        if (( attempt < 3 )); then
+            echo "Resetting sus units..."
+
+            # We must fully reset to get independent running units again
+            for i in $(seq -f "%02g" 1 20); do
+                rm -f /run/systemd/system/sus-"${i}".service
+                cat >/run/systemd/system/sus-"${i}".service <<'EOF'
+[Service]
+Type=simple
+ExecStart=/bin/sleep infinity
+EOF
+            done
+
+            systemctl "$reload_cmd"
+
+            # Ensure they are running again (they might have been
+            # abandoned/killed during the transition)
+            for i in $(seq -f "%02g" 1 20); do
+                systemctl start sus-"${i}".service
+            done
+
+            echo "Reset complete."
+        fi
+    done
+
+    echo "legit.service did not become sus through all 3 $reload_cmd cycles"
+
+    echo "$reload_cmd test passed"
+}
+
+cleanup_test_units() {
+    reap_abandoned_pids || true
+    systemctl stop legit.service 2>/dev/null || true
+    for i in $(seq -f "%02g" 1 20); do
+        systemctl stop sus-"${i}".service 2>/dev/null || true
+        rm -f /run/systemd/system/sus-"${i}".service
+    done
+    rm -f /run/systemd/system/legit.service
+    systemctl daemon-reload
+}
+
+trap cleanup_test_units EXIT
+
+run_test daemon-reload
+cleanup_test_units
+run_test daemon-reexec

test/units/TEST-07-PID1.alias-rename.sh

@@ -0,0 +1,58 @@
+#!/usr/bin/env bash
+# SPDX-License-Identifier: LGPL-2.1-or-later
+set -eux
+set -o pipefail
+
+run_test() {
+    local reload_cmd="${1:?}"
+    local orig_pid new_pid
+
+    echo ""
+    echo "========================================="
+    echo "Testing rename preservation with: systemctl $reload_cmd"
+    echo "========================================="
+
+    cat >/run/systemd/system/rename.service <<'EOF'
+[Service]
+Type=simple
+ExecStart=/bin/sleep infinity
+EOF
+
+    systemctl daemon-reload
+    systemctl start rename.service
+
+    orig_pid=$(systemctl show -P MainPID rename.service)
+    (( orig_pid != 0 ))
+
+    # The old name becomes an alias to the new canonical unit...
+    rm -f /run/systemd/system/rename.service
+    cat >/run/systemd/system/the-unit-formerly-known-as-rename.service <<'EOF'
+[Service]
+Type=simple
+ExecStart=/bin/sleep infinity
+EOF
+    ln -sf /run/systemd/system/the-unit-formerly-known-as-rename.service /run/systemd/system/rename.service
+
+    systemctl "$reload_cmd"
+
+    # ...and the running service must stay tracked across the rename.
+    new_pid=$(systemctl show -P MainPID the-unit-formerly-known-as-rename.service)
+    (( new_pid == orig_pid ))
+    (( $(systemctl show -P MainPID rename.service) == orig_pid ))
+    [[ "$(systemctl show -P ActiveState the-unit-formerly-known-as-rename.service)" == active ]]
+    [[ "$(systemctl show -P ActiveState rename.service)" == active ]]
+}
+
+cleanup_test_units() {
+    systemctl stop the-unit-formerly-known-as-rename.service 2>/dev/null || true
+    systemctl stop rename.service 2>/dev/null || true
+    rm -f /run/systemd/system/rename.service
+    rm -f /run/systemd/system/the-unit-formerly-known-as-rename.service
+    systemctl daemon-reload
+}
+
+trap cleanup_test_units EXIT
+
+run_test daemon-reload
+cleanup_test_units
+run_test daemon-reexec