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>

diff --git a/src/udev/udev-event.c b/src/udev/udev-event.c
index 49c692944efe349e598c91a0c343bbe40d24668a..fd79c5fea7511b7350530a229b5685937cc35cc4 100644 (file)
--- a/src/udev/udev-event.c
+++ b/src/udev/udev-event.c
@@ -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;
 }
 

diff --git a/test/units/TEST-17-UDEV.rename-netif.sh b/test/units/TEST-17-UDEV.rename-netif.sh
index 0abfe24bcd82ba18e806a990ff6990ab233077f4..393719efed510e1b3506f9f8ecb48a2a65b1cab3 100755 (executable)
--- a/test/units/TEST-17-UDEV.rename-netif.sh
+++ b/test/units/TEST-17-UDEV.rename-netif.sh
@@ -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