udev: preserve rule-set properties on failed network interface rename

When rename_netif() failed for a reason other than -EBUSY (e.g. -EEXIST
because NAME= conflicts with an existing interface), the revert path
rewrote the on-disk udev database from dev_db_clone (the pre-rules
snapshot) and then returned the error. The caller in
udev_event_execute_rules() bailed out before reaching the final
device_update_db(dev), so every property the rules had attached to the
device during this event (including ENV{}= assignments made alongside
the failing NAME=) was silently dropped from /run/udev/data/.

Persist 'dev' from inside the revert block, after its syspath and
INTERFACE have been restored. The cloned snapshot is still written
first to clear ID_RENAMING/ID_PROCESSING; the subsequent dev write
keeps the rule-applied properties without resurrecting the new name.
Only persist when r < 0, since the -EBUSY path returns success and the
caller will write the DB itself.

Add a TEST-17-UDEV.rename-netif regression test that pre-creates the
rename target, then triggers rules that set three ENV{}= properties
(one on a rule line before the failing NAME=, one on the same line as
NAME=, and one on a rule line after) and asserts all three are still
visible via udevadm info on the source interface. The neighbouring
test_netif_renaming_conflict already exercises the same -EEXIST revert
path but only checks the SYSTEMD_ALIAS broadcast, which is why this
regression went unnoticed.

Verified against unpatched origin/main (b4aff10ac0): the new test
fails, udevadm reports only kernel-supplied fields for the source
interface, and /run/udev/data/n<ifindex> is 0 bytes. With this patch
applied all three properties survive regardless of whether they were
set before, on, or after the failing NAME= rule line.

Downstream impact (NetworkManager managing an interface that rules had
marked NM_UNMANAGED=1): https://redhat.atlassian.net/browse/RHEL-178481

Fixes: https://github.com/systemd/systemd/issues/42331

Co-developed-by: Claude Opus 4.7 <noreply@anthropic.com>
(cherry picked from commit 779a3a13ae)
This commit is contained in:
Chris Patterson
2026-05-26 15:04:28 +00:00
committed by Luca Boccassi
parent 727999719d
commit 44275007bd
2 changed files with 64 additions and 1 deletions

View File

@@ -226,6 +226,15 @@ revert:
}
(void) device_add_property(dev, "ID_RENAMING", NULL);
/* When rename_netif() returns an error, the caller bails out of event_execute_rules() before
* device_update_db(dev) is called. Without persisting 'dev' here we would leave the on-disk
* database holding only the (stale) cloned state, dropping every property that udev rules added
* to the device during this event (e.g. ENV{}= assignments alongside the failing NAME=).
* In the -EBUSY case (r == 0) the caller continues normally and will write the DB itself, so
* only persist here when we're actually about to bail out. */
if (r < 0)
(void) device_update_db(dev);
return r;
}

View File

@@ -100,7 +100,7 @@ timeout 30 bash -c 'while [[ "$(systemctl show --property=ActiveState --value /s
# cleanup
ip link del hoge
# shellcheck disable=SC2317
# shellcheck disable=SC2317,SC2329
teardown_netif_renaming_conflict() {
set +ex
@@ -181,4 +181,58 @@ EOF
test_netif_renaming_conflict
# shellcheck disable=SC2317,SC2329
teardown_netif_renaming_keeps_properties() {
set +ex
rm -f /run/udev/rules.d/50-testsuite.rules
udevadm control --reload --timeout=30
ip link del rename-src 2>/dev/null || :
ip link del rename-dst 2>/dev/null || :
}
test_netif_renaming_keeps_properties() {
trap teardown_netif_renaming_keeps_properties RETURN
cat >/run/udev/rules.d/50-testsuite.rules <<EOF
ACTION!="add", GOTO="end"
SUBSYSTEM!="net", GOTO="end"
OPTIONS="log_level=debug"
# Set properties before, during, and after a rename to a name that is already
# taken. The rename will fail with -EEXIST, but the property assignments must
# persist.
KERNEL=="rename-src", ENV{ID_TEST_PROP_BEFORE}="before"
KERNEL=="rename-src", ENV{ID_TEST_PROP_DURING}="during", NAME="rename-dst"
KERNEL=="rename-src", ENV{ID_TEST_PROP_AFTER}="after"
LABEL="end"
EOF
udevadm control --log-priority=debug --reload --timeout=30
# Pre-create the conflicting target so the rename is guaranteed to fail.
ip link add rename-dst type dummy
udevadm wait --timeout=30 --settle /sys/devices/virtual/net/rename-dst
ip link add rename-src type dummy
udevadm wait --timeout=30 --settle /sys/devices/virtual/net/rename-src
# Sanity check: the rename did fail and the original interface still exists.
[[ -d /sys/devices/virtual/net/rename-src ]]
# The properties set by the rule alongside NAME= must be present in the udev
# database even though the rename itself failed. Check one assigned before,
# one assigned on the same line as the failing NAME= token, and one
# assigned after.
info="$(udevadm info /sys/devices/virtual/net/rename-src)"
assert_in "ID_TEST_PROP_BEFORE=before" "$info"
assert_in "ID_TEST_PROP_DURING=during" "$info"
assert_in "ID_TEST_PROP_AFTER=after" "$info"
}
test_netif_renaming_keeps_properties
exit 0