Merge ~enr0n/ubuntu/+source/systemd:ubuntu-jammy into ~ubuntu-core-dev/ubuntu/+source/systemd:ubuntu-jammy

[Nick Rosbrook]

The fix for LP: #2002445 is incomplete without these patches. In particular, udev needs to properly clean up modified device properties after failed renaming attempts. The full upstream PR is here: https://github.com/systemd/systemd/pull/25980.

Only the necessary commits from that PR are backported here.

PPA: https://launchpad.net/~enr0n/+archive/ubuntu/systemd-249
autopkgtest:

systemd 249.11-0ubuntu3.9~ppa1 (amd64) -- Fail: https://autopkgtest.ubuntu.com/results/autopkgtest-jammy-enr0n-systemd-249/jammy/amd64/s/systemd/20230318_022614_2a0e9@/log.gz
systemd 249.11-0ubuntu3.9~ppa1 (arm64) -- Pass: https://autopkgtest.ubuntu.com/results/autopkgtest-jammy-enr0n-systemd-249/jammy/arm64/s/systemd/20230317_220446_9d627@/log.gz
systemd 249.11-0ubuntu3.9~ppa1 (armhf) -- Pass: https://autopkgtest.ubuntu.com/results/autopkgtest-jammy-enr0n-systemd-249/jammy/armhf/s/systemd/20230317_205813_0b841@/log.gz
systemd 249.11-0ubuntu3.9~ppa1 (ppc64el) -- Fail: https://autopkgtest.ubuntu.com/results/autopkgtest-jammy-enr0n-systemd-249/jammy/ppc64el/s/systemd/20230317_211917_e3ab0@/log.gz
systemd 249.11-0ubuntu3.9~ppa1 (s390x) -- Fail: https://autopkgtest.ubuntu.com/results/autopkgtest-jammy-enr0n-systemd-249/jammy/s390x/s/systemd/20230317_215320_04b93@/log.gz

The s390x failure was a test bed failure. The ppc64el and amd64 failures are both familiar flaky tests.

[Lukas Märdian (slyon)]

Thanks, refreshed/backported patches lgtm.

praise: I really like how you provide full context in the commit messages and merge-proposal (test-builds, autopkgtest logs + explanation). Kudos!

review: Approve

[Lukas Märdian (slyon)]

Sponsored as `debuild -S -d -v249.11-0ubuntu3.7`, to keep the previous 249.11-0ubuntu3.8 SRU changelog available.

https://launchpad.net/ubuntu/jammy/+queue?queue_state=1&queue_text=systemd

diff --git a/debian/changelog b/debian/changelog
index 238b809..ec1d8ab 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,3 +1,15 @@
+systemd (249.11-0ubuntu3.9) jammy; urgency=medium
+
+  * udev: gracefully handle rename failures (LP: #2002445)
+    Files:
+    - debian/patches/lp2002445/core-device-ignore-failed-uevents.patch
+    - debian/patches/lp2002445/sd-device-introduce-device_get_property_int.patch
+    - debian/patches/lp2002445/sd-device-make-device_set_syspath-clear-sysname-and-sysnu.patch
+    - debian/patches/lp2002445/udev-restore-syspath-and-properties-on-failure.patch
+    https://git.launchpad.net/~ubuntu-core-dev/ubuntu/+source/systemd/commit/?id=a7ad4a9fc708500c61e3b8127f112d8c90049b2c
+
+ -- Nick Rosbrook <[e-mail]>  Mon, 20 Mar 2023 10:32:08 -0400
+
 systemd (249.11-0ubuntu3.8) jammy; urgency=medium
 
   * network/dhcp4: accept local subnet routes from DHCP (LP: #2004478)
diff --git a/debian/patches/lp2002445/core-device-ignore-failed-uevents.patch b/debian/patches/lp2002445/core-device-ignore-failed-uevents.patch
new file mode 100644
index 0000000..1a2f9d1
--- /dev/null
+++ b/debian/patches/lp2002445/core-device-ignore-failed-uevents.patch
@@ -0,0 +1,51 @@
+From: Yu Watanabe <[e-mail]>
+Date: Mon, 9 Jan 2023 16:11:52 +0900
+Subject: core/device: ignore failed uevents
+
+Origin: upstream, https://github.com/systemd/systemd/commit/e9336d6ac346df38d96c91ba0447b3c76ee6697b
+Bug-Ubuntu: https://launchpad.net/bugs/2002445
+
+When udevd failed to process the device, SYSTEMD_ALIAS or any other
+properties may contain invalid values. Let's refuse to handle the uevent.
+---
+ src/core/device.c | 20 ++++++++++++++++++++
+ 1 file changed, 20 insertions(+)
+
+diff --git a/src/core/device.c b/src/core/device.c
+index 5ed5ceb..e138754 100644
+--- a/src/core/device.c
++++ b/src/core/device.c
+@@ -9,6 +9,7 @@
+ #include "bus-error.h"
+ #include "dbus-device.h"
+ #include "dbus-unit.h"
++#include "device-private.h"
+ #include "device-util.h"
+ #include "device.h"
+ #include "log.h"
+@@ -960,6 +961,25 @@ static int device_dispatch_io(sd_device_monitor *monitor, sd_device *dev, void *
+         if (action == SD_DEVICE_MOVE)
+                 device_remove_old_on_move(m, dev);
+ 
++        /* When udevd failed to process the device, SYSTEMD_ALIAS or any other properties may contain invalid
++         * values. Let's refuse to handle the uevent. */
++        if (sd_device_get_property_value(dev, "UDEV_WORKER_FAILED", NULL) >= 0) {
++                int v;
++
++                if (device_get_property_int(dev, "UDEV_WORKER_ERRNO", &v) >= 0)
++                        log_device_warning_errno(dev, v, "systemd-udevd failed to process the device, ignoring: %m");
++                else if (device_get_property_int(dev, "UDEV_WORKER_EXIT_STATUS", &v) >= 0)
++                        log_device_warning(dev, "systemd-udevd failed to process the device with exit status %i, ignoring.", v);
++                else if (device_get_property_int(dev, "UDEV_WORKER_SIGNAL", &v) >= 0) {
++                        const char *s;
++                        (void) sd_device_get_property_value(dev, "UDEV_WORKER_SIGNAL_NAME", &s);
++                        log_device_warning(dev, "systemd-udevd failed to process the device with signal %i(%s), ignoring.", v, strna(s));
++                } else
++                        log_device_warning(dev, "systemd-udevd failed to process the device with unknown result, ignoring.");
++
++                return 0;
++        }
++
+         /* A change event can signal that a device is becoming ready, in particular if the device is using
+          * the SYSTEMD_READY logic in udev so we need to reach the else block of the following if, even for
+          * change events */
diff --git a/debian/patches/lp2002445/sd-device-introduce-device_get_property_int.patch b/debian/patches/lp2002445/sd-device-introduce-device_get_property_int.patch
new file mode 100644
index 0000000..3dcdf4a
--- /dev/null
+++ b/debian/patches/lp2002445/sd-device-introduce-device_get_property_int.patch
@@ -0,0 +1,56 @@
+From: Yu Watanabe <[e-mail]>
+Date: Mon, 9 Jan 2023 16:44:11 +0900
+Subject: sd-device: introduce device_get_property_int()
+
+Origin: upstream, https://github.com/systemd/systemd/commit/eedfef0f0d2654fcde2a3b694e62518d688af827
+Bug-Ubuntu: https://launchpad.net/bugs/2002445
+
+(modified to apply to v249.11)
+---
+ src/libsystemd/sd-device/device-private.h |  1 +
+ src/libsystemd/sd-device/sd-device.c      | 20 ++++++++++++++++++++
+ 2 files changed, 21 insertions(+)
+
+diff --git a/src/libsystemd/sd-device/device-private.h b/src/libsystemd/sd-device/device-private.h
+index 9bb5eff..0ed7807 100644
+--- a/src/libsystemd/sd-device/device-private.h
++++ b/src/libsystemd/sd-device/device-private.h
+@@ -17,6 +17,7 @@ static inline int device_new_from_watch_handle(sd_device **ret, int wd) {
+         return device_new_from_watch_handle_at(ret, -1, wd);
+ }
+ 
++int device_get_property_int(sd_device *device, const char *key, int *ret);
+ int device_get_device_id(sd_device *device, const char **ret);
+ 
+ int device_get_devlink_priority(sd_device *device, int *priority);
+diff --git a/src/libsystemd/sd-device/sd-device.c b/src/libsystemd/sd-device/sd-device.c
+index 49f56ca..2b9be24 100644
+--- a/src/libsystemd/sd-device/sd-device.c
++++ b/src/libsystemd/sd-device/sd-device.c
+@@ -1904,6 +1904,26 @@ _public_ int sd_device_get_property_value(sd_device *device, const char *key, co
+         return 0;
+ }
+ 
++int device_get_property_int(sd_device *device, const char *key, int *ret) {
++        const char *value;
++        int r, v;
++
++        assert(device);
++        assert(key);
++
++        r = sd_device_get_property_value(device, key, &value);
++        if (r < 0)
++                return r;
++
++        r = safe_atoi(value, &v);
++        if (r < 0)
++                return r;
++
++        if (ret)
++                *ret = v;
++        return 0;
++}
++
+ _public_ int sd_device_get_trigger_uuid(sd_device *device, sd_id128_t *ret) {
+         const char *s;
+         sd_id128_t id;
diff --git a/debian/patches/lp2002445/sd-device-make-device_set_syspath-clear-sysname-and-sysnu.patch b/debian/patches/lp2002445/sd-device-make-device_set_syspath-clear-sysname-and-sysnu.patch
new file mode 100644
index 0000000..82c82e3
--- /dev/null
+++ b/debian/patches/lp2002445/sd-device-make-device_set_syspath-clear-sysname-and-sysnu.patch
@@ -0,0 +1,29 @@
+From: Yu Watanabe <[e-mail]>
+Date: Mon, 9 Jan 2023 14:00:09 +0900
+Subject: sd-device: make device_set_syspath() clear sysname and sysnum
+
+Origin: upstream, https://github.com/systemd/systemd/commit/9a26098e90116fdb5fcffa03485b375ee0c82b6a
+Bug-Ubuntu: https://launchpad.net/bugs/2002445
+
+Otherwise, when a new syspath is assigned to the sd-device object,
+sd_device_get_sysname() or _sysnum() will provide an outdated device
+name or number.
+---
+ src/libsystemd/sd-device/sd-device.c | 4 ++++
+ 1 file changed, 4 insertions(+)
+
+diff --git a/src/libsystemd/sd-device/sd-device.c b/src/libsystemd/sd-device/sd-device.c
+index 418a5b1..49f56ca 100644
+--- a/src/libsystemd/sd-device/sd-device.c
++++ b/src/libsystemd/sd-device/sd-device.c
+@@ -213,6 +213,10 @@ int device_set_syspath(sd_device *device, const char *_syspath, bool verify) {
+ 
+         free_and_replace(device->syspath, syspath);
+         device->devpath = devpath;
++
++        /* Unset sysname and sysnum, they will be assigned when requested. */
++        device->sysnum = NULL;
++        device->sysname = mfree(device->sysname);
+         return 0;
+ }
+ 
diff --git a/debian/patches/lp2002445/udev-restore-syspath-and-properties-on-failure.patch b/debian/patches/lp2002445/udev-restore-syspath-and-properties-on-failure.patch
new file mode 100644
index 0000000..c7f5810
--- /dev/null
+++ b/debian/patches/lp2002445/udev-restore-syspath-and-properties-on-failure.patch
@@ -0,0 +1,159 @@
+From: Yu Watanabe <[e-mail]>
+Date: Mon, 9 Jan 2023 14:58:58 +0900
+Subject: udev: restore syspath and properties on failure
+
+Origin: upstream, https://github.com/systemd/systemd/commit/210033847c340c43dd6835520f21f8b23ba29579
+Bug-Ubuntu: https://launchpad.net/bugs/2002445
+
+Otherwise, invalid sysname or properties may be broadcast to udev
+listeners.
+
+(modified to apply to v249.11)
+---
+ src/udev/udev-event.c | 92 ++++++++++++++++++++++++++++++++++++---------------
+ 1 file changed, 65 insertions(+), 27 deletions(-)
+
+diff --git a/src/udev/udev-event.c b/src/udev/udev-event.c
+index 84822d9..d538377 100644
+--- a/src/udev/udev-event.c
++++ b/src/udev/udev-event.c
+@@ -12,6 +12,7 @@
+ #include "sd-event.h"
+ 
+ #include "alloc-util.h"
++#include "device-internal.h"
+ #include "device-private.h"
+ #include "device-util.h"
+ #include "fd-util.h"
+@@ -827,19 +828,13 @@ int udev_event_spawn(UdevEvent *event,
+ 
+ static int rename_netif(UdevEvent *event) {
+         sd_device *dev = event->dev;
+-        const char *oldname;
++        _cleanup_free_ char *old_syspath = NULL, *old_sysname = NULL;
++        const char *s;
+         int ifindex, r;
+ 
+         if (!event->name)
+                 return 0; /* No new name is requested. */
+ 
+-        r = sd_device_get_sysname(dev, &oldname);
+-        if (r < 0)
+-                return log_device_error_errno(dev, r, "Failed to get sysname: %m");
+-
+-        if (streq(event->name, oldname))
+-                return 0; /* The interface name is already requested name. */
+-
+         if (!device_for_action(dev, SD_DEVICE_ADD))
+                 return 0; /* Rename the interface only when it is added. */
+ 
+@@ -847,7 +842,7 @@ static int rename_netif(UdevEvent *event) {
+         if (r == -ENOENT)
+                 return 0; /* Device is not a network interface. */
+         if (r < 0)
+-                return log_device_error_errno(dev, r, "Failed to get ifindex: %m");
++                return log_device_warning_errno(dev, r, "Failed to get ifindex: %m");
+ 
+         if (naming_scheme_has(NAMING_REPLACE_STRICTLY) &&
+             !ifname_valid(event->name)) {
+@@ -855,39 +850,82 @@ static int rename_netif(UdevEvent *event) {
+                 return 0;
+         }
+ 
+-        /* Set ID_RENAMING boolean property here, and drop it in the corresponding move uevent later. */
+-        r = device_add_property(dev, "ID_RENAMING", "1");
++        r = sd_device_get_sysname(dev, &s);
+         if (r < 0)
+-                return log_device_warning_errno(dev, r, "Failed to add 'ID_RENAMING' property: %m");
++                return log_device_warning_errno(dev, r, "Failed to get sysname: %m");
+ 
+-        r = device_rename(dev, event->name);
++        if (streq(event->name, s))
++                return 0; /* The interface name is already requested name. */
++
++        old_sysname = strdup(s);
++        if (!old_sysname)
++                return -ENOMEM;
++
++        r = sd_device_get_syspath(dev, &s);
+         if (r < 0)
+-                return log_device_warning_errno(dev, r, "Failed to update properties with new name '%s': %m", event->name);
++                return log_device_warning_errno(dev, r, "Failed to get syspath: %m");
++
++        old_syspath = strdup(s);
++        if (!old_syspath)
++                return -ENOMEM;
++
++        r = device_rename(dev, event->name);
++        if (r < 0) {
++                log_device_warning_errno(dev, r, "Failed to update properties with new name '%s': %m", event->name);
++                goto revert;
++        }
++
++        /* Set ID_RENAMING boolean property here. It will be dropped when the corresponding move uevent is processed. */
++        r = device_add_property(dev, "ID_RENAMING", "1");
++        if (r < 0) {
++                log_device_warning_errno(dev, r, "Failed to add 'ID_RENAMING' property: %m");
++                goto revert;
++        }
+ 
+         /* Also set ID_RENAMING boolean property to cloned sd_device object and save it to database
+          * before calling rtnl_set_link_name(). Otherwise, clients (e.g., systemd-networkd) may receive
+          * RTM_NEWLINK netlink message before the database is updated. */
+         r = device_add_property(event->dev_db_clone, "ID_RENAMING", "1");
+-        if (r < 0)
+-                return log_device_warning_errno(event->dev_db_clone, r, "Failed to add 'ID_RENAMING' property: %m");
++        if (r < 0) {
++                log_device_warning_errno(event->dev_db_clone, r, "Failed to add 'ID_RENAMING' property: %m");
++                goto revert;
++        }
+ 
+         r = device_update_db(event->dev_db_clone);
+-        if (r < 0)
+-                return log_device_debug_errno(event->dev_db_clone, r, "Failed to update database under /run/udev/data/: %m");
++        if (r < 0) {
++                log_device_debug_errno(event->dev_db_clone, r, "Failed to update database under /run/udev/data/: %m");
++                goto revert;
++        }
+ 
+         r = rtnl_set_link_name(&event->rtnl, ifindex, event->name);
+-        if (r == -EBUSY) {
+-                log_device_info(dev, "Network interface '%s' is already up, cannot rename to '%s'.",
+-                                oldname, event->name);
+-                return 0;
++        if (r < 0) {
++                if (r == -EBUSY) {
++                        log_device_info(dev, "Network interface '%s' is already up, cannot rename to '%s'.",
++                                        old_sysname, event->name);
++                        r = 0;
++                } else
++                        log_device_error_errno(dev, r, "Failed to rename network interface %i from '%s' to '%s': %m",
++                                               ifindex, old_sysname, event->name);
++                goto revert;
+         }
+-        if (r < 0)
+-                return log_device_error_errno(dev, r, "Failed to rename network interface %i from '%s' to '%s': %m",
+-                                              ifindex, oldname, event->name);
+-
+-        log_device_debug(dev, "Network interface %i is renamed from '%s' to '%s'", ifindex, oldname, event->name);
+ 
++        log_device_debug(dev, "Network interface %i is renamed from '%s' to '%s'", ifindex, old_sysname, event->name);
+         return 1;
++
++revert:
++        /* Restore 'dev_db_clone' */
++        (void) device_add_property(event->dev_db_clone, "ID_RENAMING", NULL);
++        (void) device_update_db(event->dev_db_clone);
++
++        /* Restore 'dev' */
++        (void) device_set_syspath(dev, old_syspath, /* verify = */ false);
++        if (sd_device_get_property_value(dev, "INTERFACE_OLD", &s) >= 0) {
++                (void) device_add_property_internal(dev, "INTERFACE", s);
++                (void) device_add_property_internal(dev, "INTERFACE_OLD", NULL);
++        }
++        (void) device_add_property(dev, "ID_RENAMING", NULL);
++
++        return r;
+ }
+ 
+ static int update_devnode(UdevEvent *event) {
diff --git a/debian/patches/series b/debian/patches/series
index 44b471e..c98336a 100644
--- a/debian/patches/series
+++ b/debian/patches/series
@@ -109,3 +109,7 @@ lp2002445/udev-attempt-device-rename-even-if-interface-is-up.patch
 lp2002445/sd-netlink-add-a-test-for-rtnl_set_link_name.patch
 lp2000880-network-create-stacked-netdevs-after-the-underlying-link-.patch
 lp2009502-Enable-dev-sgx_vepc-access-for-the-group-sgx.patch
+lp2002445/sd-device-make-device_set_syspath-clear-sysname-and-sysnu.patch
+lp2002445/udev-restore-syspath-and-properties-on-failure.patch
+lp2002445/sd-device-introduce-device_get_property_int.patch
+lp2002445/core-device-ignore-failed-uevents.patch