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

[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-251
autopkgtest:

systemd 251.4-1ubuntu7.3~ppa1 (amd64) -- Fail: https://autopkgtest.ubuntu.com/results/autopkgtest-kinetic-enr0n-systemd-251/kinetic/amd64/s/systemd/20230318_031454_a5637@/log.gz
systemd 251.4-1ubuntu7.3~ppa1 (arm64) -- Fail: https://autopkgtest.ubuntu.com/results/autopkgtest-kinetic-enr0n-systemd-251/kinetic/arm64/s/systemd/20230317_232929_32459@/log.gz
systemd 251.4-1ubuntu7.3~ppa1 (armhf) -- Pass: https://autopkgtest.ubuntu.com/results/autopkgtest-kinetic-enr0n-systemd-251/kinetic/armhf/s/systemd/20230317_202546_eaf77@/log.gz
systemd 251.4-1ubuntu7.3~ppa1 (ppc64el) -- Fail: https://autopkgtest.ubuntu.com/results/autopkgtest-kinetic-enr0n-systemd-251/kinetic/ppc64el/s/systemd/20230317_203751_951c0@/log.gz

These failures are due to LP: #2009859, and the failing tests will not be considered regressions. The s390x test got lost or wasn't triggered correctly.

[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 -v251.4-1ubuntu7.1`, to keep the previous 251.4-1ubuntu7.2 SRU changelog available.

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

diff --git a/debian/changelog b/debian/changelog
index 2fe2841..4cb9182 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,3 +1,15 @@
+systemd (251.4-1ubuntu7.3) kinetic; 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=deb435fbb84fde1fd39da47231a7473fc2a412e8
+
+ -- Nick Rosbrook <[e-mail]>  Mon, 20 Mar 2023 10:25:23 -0400
+
 systemd (251.4-1ubuntu7.2) kinetic; 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..2569151
--- /dev/null
+++ b/debian/patches/lp2002445/core-device-ignore-failed-uevents.patch
@@ -0,0 +1,43 @@
+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 | 19 +++++++++++++++++++
+ 1 file changed, 19 insertions(+)
+
+diff --git a/src/core/device.c b/src/core/device.c
+index fcde8a4..379cc8c 100644
+--- a/src/core/device.c
++++ b/src/core/device.c
+@@ -976,6 +976,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..26bb25f
--- /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 v251.4)
+---
+ 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 9602f9e..4621033 100644
+--- a/src/libsystemd/sd-device/device-private.h
++++ b/src/libsystemd/sd-device/device-private.h
+@@ -18,6 +18,7 @@ static inline int device_new_from_watch_handle(sd_device **ret, int wd) {
+ }
+ 
+ int device_get_property_bool(sd_device *device, const char *key);
++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 *ret);
+ int device_get_watch_handle(sd_device *device);
+diff --git a/src/libsystemd/sd-device/sd-device.c b/src/libsystemd/sd-device/sd-device.c
+index 2fa78dc..5665022 100644
+--- a/src/libsystemd/sd-device/sd-device.c
++++ b/src/libsystemd/sd-device/sd-device.c
+@@ -2026,6 +2026,26 @@ int device_get_property_bool(sd_device *device, const char *key) {
+         return parse_boolean(value);
+ }
+ 
++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..bc4a098
--- /dev/null
+++ b/debian/patches/lp2002445/sd-device-make-device_set_syspath-clear-sysname-and-sysnu.patch
@@ -0,0 +1,45 @@
+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/device-private.c | 4 ----
+ src/libsystemd/sd-device/sd-device.c      | 4 ++++
+ 2 files changed, 4 insertions(+), 4 deletions(-)
+
+diff --git a/src/libsystemd/sd-device/device-private.c b/src/libsystemd/sd-device/device-private.c
+index a453899..ddca336 100644
+--- a/src/libsystemd/sd-device/device-private.c
++++ b/src/libsystemd/sd-device/device-private.c
+@@ -784,10 +784,6 @@ int device_rename(sd_device *device, const char *name) {
+         if (r < 0)
+                 return r;
+ 
+-        /* Here, only clear the sysname and sysnum. They will be set when requested. */
+-        device->sysnum = NULL;
+-        device->sysname = mfree(device->sysname);
+-
+         r = sd_device_get_property_value(device, "INTERFACE", &interface);
+         if (r == -ENOENT)
+                 return 0;
+diff --git a/src/libsystemd/sd-device/sd-device.c b/src/libsystemd/sd-device/sd-device.c
+index 62531b9..2fa78dc 100644
+--- a/src/libsystemd/sd-device/sd-device.c
++++ b/src/libsystemd/sd-device/sd-device.c
+@@ -247,6 +247,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..543c1ec
--- /dev/null
+++ b/debian/patches/lp2002445/udev-restore-syspath-and-properties-on-failure.patch
@@ -0,0 +1,163 @@
+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 v251.4)
+---
+ src/udev/udev-event.c | 94 +++++++++++++++++++++++++++++++++++----------------
+ 1 file changed, 65 insertions(+), 29 deletions(-)
+
+diff --git a/src/udev/udev-event.c b/src/udev/udev-event.c
+index 681aa87..8b8bb4d 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"
+@@ -869,7 +870,8 @@ int udev_event_spawn(
+ }
+ 
+ static int rename_netif(UdevEvent *event) {
+-        const char *oldname;
++        _cleanup_free_ char *old_syspath = NULL, *old_sysname = NULL;
++        const char *s;
+         sd_device *dev;
+         int ifindex, r;
+ 
+@@ -880,15 +882,6 @@ static int rename_netif(UdevEvent *event) {
+ 
+         dev = ASSERT_PTR(event->dev);
+ 
+-        /* Read sysname from cloned sd-device object, otherwise use-after-free is triggered, as the
+-         * main object will be renamed and dev->sysname will be freed in device_rename(). */
+-        r = sd_device_get_sysname(event->dev_db_clone, &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. */
+ 
+@@ -896,7 +889,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)) {
+@@ -904,39 +897,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 24359ad..3727627 100644
--- a/debian/patches/series
+++ b/debian/patches/series
@@ -61,3 +61,7 @@ lp2002445/sd-netlink-do-not-swap-old-name-and-alternative-name.patch
 lp2002445/sd-netlink-restore-altname-on-error-in-rtnl_set_link_name.patch
 lp2002445/udev-attempt-device-rename-even-if-interface-is-up.patch
 lp2002445/sd-netlink-add-a-test-for-rtnl_set_link_name.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