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

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

systemd 252.5-2ubuntu3~ppa3 (amd64) -- Pass: https://autopkgtest.ubuntu.com/results/autopkgtest-lunar-enr0n-systemd/lunar/amd64/s/systemd/20230317_234655_d734e@/log.gz
systemd 252.5-2ubuntu3~ppa3 (arm64) -- Pass: https://autopkgtest.ubuntu.com/results/autopkgtest-lunar-enr0n-systemd/lunar/arm64/s/systemd/20230317_234410_8d883@/log.gz
systemd 252.5-2ubuntu3~ppa3 (armhf) -- Pass: https://autopkgtest.ubuntu.com/results/autopkgtest-lunar-enr0n-systemd/lunar/armhf/s/systemd/20230317_202006_6acc4@/log.gz
systemd 252.5-2ubuntu3~ppa3 (ppc64el) -- Pass: https://autopkgtest.ubuntu.com/results/autopkgtest-lunar-enr0n-systemd/lunar/ppc64el/s/systemd/20230317_214145_957ed@/log.gz
systemd 252.5-2ubuntu3~ppa3 (s390x) -- Fail: https://autopkgtest.ubuntu.com/results/autopkgtest-lunar-enr0n-systemd/lunar/s390x/s/systemd/20230317_221114_4f4cf@/log.gz

The s390x appears to be a test bed failure. All tests up to that point had passed.

[Lukas Märdian (slyon)]

Thanks, lgtm. Patches are matching the upstream commits.

review: Approve

[Lukas Märdian (slyon)]

https://launchpad.net/ubuntu/+source/systemd/252.5-2ubuntu3

diff --git a/debian/changelog b/debian/changelog
index bc515ad..cb44d04 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,3 +1,15 @@
+systemd (252.5-2ubuntu3) lunar; 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=79536dbb165dbcc402629684e0911693626df5b1
+
+ -- Nick Rosbrook <[e-mail]>  Mon, 20 Mar 2023 10:17:24 -0400
+
 systemd (252.5-2ubuntu2) lunar; 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..2044779
--- /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 6e07f27..d3a052d 100644
+--- a/src/core/device.c
++++ b/src/core/device.c
+@@ -1137,6 +1137,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..439f257
--- /dev/null
+++ b/debian/patches/lp2002445/sd-device-introduce-device_get_property_int.patch
@@ -0,0 +1,55 @@
+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
+
+---
+ 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 a59f130..f46abee 100644
+--- a/src/libsystemd/sd-device/device-private.h
++++ b/src/libsystemd/sd-device/device-private.h
+@@ -18,6 +18,7 @@ int device_new_from_strv(sd_device **ret, char **strv);
+ int device_opendir(sd_device *device, const char *subdir, DIR **ret);
+ 
+ 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_sysattr_int(sd_device *device, const char *sysattr, int *ret_value);
+ int device_get_sysattr_unsigned(sd_device *device, const char *sysattr, unsigned *ret_value);
+ int device_get_sysattr_bool(sd_device *device, const char *sysattr);
+diff --git a/src/libsystemd/sd-device/sd-device.c b/src/libsystemd/sd-device/sd-device.c
+index d35726d..4aecf7c 100644
+--- a/src/libsystemd/sd-device/sd-device.c
++++ b/src/libsystemd/sd-device/sd-device.c
+@@ -2186,6 +2186,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..9621622
--- /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 bc7a838..2c1d922 100644
+--- a/src/libsystemd/sd-device/device-private.c
++++ b/src/libsystemd/sd-device/device-private.c
+@@ -646,10 +646,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 ff5d065..d35726d 100644
+--- a/src/libsystemd/sd-device/sd-device.c
++++ b/src/libsystemd/sd-device/sd-device.c
+@@ -250,6 +250,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..ab717da
--- /dev/null
+++ b/debian/patches/lp2002445/udev-restore-syspath-and-properties-on-failure.patch
@@ -0,0 +1,161 @@
+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.
+---
+ 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 08d69cf..f5a10d9 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"
+@@ -860,7 +861,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;
+ 
+@@ -871,15 +873,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. */
+ 
+@@ -887,7 +880,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)) {
+@@ -895,39 +888,82 @@ static int rename_netif(UdevEvent *event) {
+                 return 0;
+         }
+ 
+-        /* 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");
++        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 749ad23..ac0e72b 100644
--- a/debian/patches/series
+++ b/debian/patches/series
@@ -53,3 +53,7 @@ lp2002445-test-network-add-a-test-for-renaming-device-to-current-al.patch
 Deny-list-TEST-74-AUX-UTILS-on-s390x.patch
 lp2004478-network-dhcp4-accept-local-subnet-routes-from-DHCP.patch
 sd-netlink-skip-test_rtnl_set_link_name-when-altnames-are.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