Ubuntu
systemd package

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

  1. Git
  2. lp:~enr0n/ubuntu/+source/systemd
  3. ubuntu-kinetic
  4. Merge into ubuntu-kinetic
Proposed by Nick Rosbrook
Status: Merged
Merged at revision: 6b2e008ecdea47dd8cc6fdadddf42a8199c51def
Proposed branch: ~enr0n/ubuntu/+source/systemd:ubuntu-kinetic
Merge into: ~ubuntu-core-dev/ubuntu/+source/systemd:ubuntu-kinetic
Diff against target: 363 lines (+323/-0)
6 files modified
debian/changelog (+12/-0)
debian/patches/lp2002445/core-device-ignore-failed-uevents.patch (+43/-0)
debian/patches/lp2002445/sd-device-introduce-device_get_property_int.patch (+56/-0)
debian/patches/lp2002445/sd-device-make-device_set_syspath-clear-sysname-and-sysnu.patch (+45/-0)
debian/patches/lp2002445/udev-restore-syspath-and-properties-on-failure.patch (+163/-0)
debian/patches/series (+4/-0)
Reviewer Review Type Date Requested Status
Lukas Märdian Approve
Review via email: mp+439251@code.launchpad.net

Description of the change

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.

To post a comment you must log in.
Revision history for this message
Lukas Märdian (slyon) wrote (last edit ):

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
Revision history for this message
Lukas Märdian (slyon) wrote :

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

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
diff --git a/debian/changelog b/debian/changelog
index 2fe2841..4cb9182 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,3 +1,15 @@
1systemd (251.4-1ubuntu7.3) kinetic; urgency=medium
2
3 * udev: gracefully handle rename failures (LP: #2002445)
4 Files:
5 - debian/patches/lp2002445/core-device-ignore-failed-uevents.patch
6 - debian/patches/lp2002445/sd-device-introduce-device_get_property_int.patch
7 - debian/patches/lp2002445/sd-device-make-device_set_syspath-clear-sysname-and-sysnu.patch
8 - debian/patches/lp2002445/udev-restore-syspath-and-properties-on-failure.patch
9 https://git.launchpad.net/~ubuntu-core-dev/ubuntu/+source/systemd/commit/?id=deb435fbb84fde1fd39da47231a7473fc2a412e8
10
11 -- Nick Rosbrook <nick.rosbrook@canonical.com> Mon, 20 Mar 2023 10:25:23 -0400
12
1systemd (251.4-1ubuntu7.2) kinetic; urgency=medium13systemd (251.4-1ubuntu7.2) kinetic; urgency=medium
214
3 * network/dhcp4: accept local subnet routes from DHCP (LP: #2004478)15 * 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
4new file mode 10064416new file mode 100644
index 0000000..2569151
--- /dev/null
+++ b/debian/patches/lp2002445/core-device-ignore-failed-uevents.patch
@@ -0,0 +1,43 @@
1From: Yu Watanabe <watanabe.yu+github@gmail.com>
2Date: Mon, 9 Jan 2023 16:11:52 +0900
3Subject: core/device: ignore failed uevents
4
5Origin: upstream, https://github.com/systemd/systemd/commit/e9336d6ac346df38d96c91ba0447b3c76ee6697b
6Bug-Ubuntu: https://launchpad.net/bugs/2002445
7
8When udevd failed to process the device, SYSTEMD_ALIAS or any other
9properties may contain invalid values. Let's refuse to handle the uevent.
10---
11 src/core/device.c | 19 +++++++++++++++++++
12 1 file changed, 19 insertions(+)
13
14diff --git a/src/core/device.c b/src/core/device.c
15index fcde8a4..379cc8c 100644
16--- a/src/core/device.c
17+++ b/src/core/device.c
18@@ -976,6 +976,25 @@ static int device_dispatch_io(sd_device_monitor *monitor, sd_device *dev, void *
19 if (action == SD_DEVICE_MOVE)
20 device_remove_old_on_move(m, dev);
21
22+ /* When udevd failed to process the device, SYSTEMD_ALIAS or any other properties may contain invalid
23+ * values. Let's refuse to handle the uevent. */
24+ if (sd_device_get_property_value(dev, "UDEV_WORKER_FAILED", NULL) >= 0) {
25+ int v;
26+
27+ if (device_get_property_int(dev, "UDEV_WORKER_ERRNO", &v) >= 0)
28+ log_device_warning_errno(dev, v, "systemd-udevd failed to process the device, ignoring: %m");
29+ else if (device_get_property_int(dev, "UDEV_WORKER_EXIT_STATUS", &v) >= 0)
30+ log_device_warning(dev, "systemd-udevd failed to process the device with exit status %i, ignoring.", v);
31+ else if (device_get_property_int(dev, "UDEV_WORKER_SIGNAL", &v) >= 0) {
32+ const char *s;
33+ (void) sd_device_get_property_value(dev, "UDEV_WORKER_SIGNAL_NAME", &s);
34+ log_device_warning(dev, "systemd-udevd failed to process the device with signal %i(%s), ignoring.", v, strna(s));
35+ } else
36+ log_device_warning(dev, "systemd-udevd failed to process the device with unknown result, ignoring.");
37+
38+ return 0;
39+ }
40+
41 /* A change event can signal that a device is becoming ready, in particular if the device is using
42 * the SYSTEMD_READY logic in udev so we need to reach the else block of the following if, even for
43 * 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
0new file mode 10064444new file mode 100644
index 0000000..26bb25f
--- /dev/null
+++ b/debian/patches/lp2002445/sd-device-introduce-device_get_property_int.patch
@@ -0,0 +1,56 @@
1From: Yu Watanabe <watanabe.yu+github@gmail.com>
2Date: Mon, 9 Jan 2023 16:44:11 +0900
3Subject: sd-device: introduce device_get_property_int()
4
5Origin: upstream, https://github.com/systemd/systemd/commit/eedfef0f0d2654fcde2a3b694e62518d688af827
6Bug-Ubuntu: https://launchpad.net/bugs/2002445
7
8(modified to apply to v251.4)
9---
10 src/libsystemd/sd-device/device-private.h | 1 +
11 src/libsystemd/sd-device/sd-device.c | 20 ++++++++++++++++++++
12 2 files changed, 21 insertions(+)
13
14diff --git a/src/libsystemd/sd-device/device-private.h b/src/libsystemd/sd-device/device-private.h
15index 9602f9e..4621033 100644
16--- a/src/libsystemd/sd-device/device-private.h
17+++ b/src/libsystemd/sd-device/device-private.h
18@@ -18,6 +18,7 @@ static inline int device_new_from_watch_handle(sd_device **ret, int wd) {
19 }
20
21 int device_get_property_bool(sd_device *device, const char *key);
22+int device_get_property_int(sd_device *device, const char *key, int *ret);
23 int device_get_device_id(sd_device *device, const char **ret);
24 int device_get_devlink_priority(sd_device *device, int *ret);
25 int device_get_watch_handle(sd_device *device);
26diff --git a/src/libsystemd/sd-device/sd-device.c b/src/libsystemd/sd-device/sd-device.c
27index 2fa78dc..5665022 100644
28--- a/src/libsystemd/sd-device/sd-device.c
29+++ b/src/libsystemd/sd-device/sd-device.c
30@@ -2026,6 +2026,26 @@ int device_get_property_bool(sd_device *device, const char *key) {
31 return parse_boolean(value);
32 }
33
34+int device_get_property_int(sd_device *device, const char *key, int *ret) {
35+ const char *value;
36+ int r, v;
37+
38+ assert(device);
39+ assert(key);
40+
41+ r = sd_device_get_property_value(device, key, &value);
42+ if (r < 0)
43+ return r;
44+
45+ r = safe_atoi(value, &v);
46+ if (r < 0)
47+ return r;
48+
49+ if (ret)
50+ *ret = v;
51+ return 0;
52+}
53+
54 _public_ int sd_device_get_trigger_uuid(sd_device *device, sd_id128_t *ret) {
55 const char *s;
56 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
0new file mode 10064457new 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 @@
1From: Yu Watanabe <watanabe.yu+github@gmail.com>
2Date: Mon, 9 Jan 2023 14:00:09 +0900
3Subject: sd-device: make device_set_syspath() clear sysname and sysnum
4
5Origin: upstream, https://github.com/systemd/systemd/commit/9a26098e90116fdb5fcffa03485b375ee0c82b6a
6Bug-Ubuntu: https://launchpad.net/bugs/2002445
7
8Otherwise, when a new syspath is assigned to the sd-device object,
9sd_device_get_sysname() or _sysnum() will provide an outdated device
10name or number.
11---
12 src/libsystemd/sd-device/device-private.c | 4 ----
13 src/libsystemd/sd-device/sd-device.c | 4 ++++
14 2 files changed, 4 insertions(+), 4 deletions(-)
15
16diff --git a/src/libsystemd/sd-device/device-private.c b/src/libsystemd/sd-device/device-private.c
17index a453899..ddca336 100644
18--- a/src/libsystemd/sd-device/device-private.c
19+++ b/src/libsystemd/sd-device/device-private.c
20@@ -784,10 +784,6 @@ int device_rename(sd_device *device, const char *name) {
21 if (r < 0)
22 return r;
23
24- /* Here, only clear the sysname and sysnum. They will be set when requested. */
25- device->sysnum = NULL;
26- device->sysname = mfree(device->sysname);
27-
28 r = sd_device_get_property_value(device, "INTERFACE", &interface);
29 if (r == -ENOENT)
30 return 0;
31diff --git a/src/libsystemd/sd-device/sd-device.c b/src/libsystemd/sd-device/sd-device.c
32index 62531b9..2fa78dc 100644
33--- a/src/libsystemd/sd-device/sd-device.c
34+++ b/src/libsystemd/sd-device/sd-device.c
35@@ -247,6 +247,10 @@ int device_set_syspath(sd_device *device, const char *_syspath, bool verify) {
36
37 free_and_replace(device->syspath, syspath);
38 device->devpath = devpath;
39+
40+ /* Unset sysname and sysnum, they will be assigned when requested. */
41+ device->sysnum = NULL;
42+ device->sysname = mfree(device->sysname);
43 return 0;
44 }
45
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
0new file mode 10064446new file mode 100644
index 0000000..543c1ec
--- /dev/null
+++ b/debian/patches/lp2002445/udev-restore-syspath-and-properties-on-failure.patch
@@ -0,0 +1,163 @@
1From: Yu Watanabe <watanabe.yu+github@gmail.com>
2Date: Mon, 9 Jan 2023 14:58:58 +0900
3Subject: udev: restore syspath and properties on failure
4
5Origin: upstream, https://github.com/systemd/systemd/commit/210033847c340c43dd6835520f21f8b23ba29579
6Bug-Ubuntu: https://launchpad.net/bugs/2002445
7
8Otherwise, invalid sysname or properties may be broadcast to udev
9listeners.
10
11(modified to apply to v251.4)
12---
13 src/udev/udev-event.c | 94 +++++++++++++++++++++++++++++++++++----------------
14 1 file changed, 65 insertions(+), 29 deletions(-)
15
16diff --git a/src/udev/udev-event.c b/src/udev/udev-event.c
17index 681aa87..8b8bb4d 100644
18--- a/src/udev/udev-event.c
19+++ b/src/udev/udev-event.c
20@@ -12,6 +12,7 @@
21 #include "sd-event.h"
22
23 #include "alloc-util.h"
24+#include "device-internal.h"
25 #include "device-private.h"
26 #include "device-util.h"
27 #include "fd-util.h"
28@@ -869,7 +870,8 @@ int udev_event_spawn(
29 }
30
31 static int rename_netif(UdevEvent *event) {
32- const char *oldname;
33+ _cleanup_free_ char *old_syspath = NULL, *old_sysname = NULL;
34+ const char *s;
35 sd_device *dev;
36 int ifindex, r;
37
38@@ -880,15 +882,6 @@ static int rename_netif(UdevEvent *event) {
39
40 dev = ASSERT_PTR(event->dev);
41
42- /* Read sysname from cloned sd-device object, otherwise use-after-free is triggered, as the
43- * main object will be renamed and dev->sysname will be freed in device_rename(). */
44- r = sd_device_get_sysname(event->dev_db_clone, &oldname);
45- if (r < 0)
46- return log_device_error_errno(dev, r, "Failed to get sysname: %m");
47-
48- if (streq(event->name, oldname))
49- return 0; /* The interface name is already requested name. */
50-
51 if (!device_for_action(dev, SD_DEVICE_ADD))
52 return 0; /* Rename the interface only when it is added. */
53
54@@ -896,7 +889,7 @@ static int rename_netif(UdevEvent *event) {
55 if (r == -ENOENT)
56 return 0; /* Device is not a network interface. */
57 if (r < 0)
58- return log_device_error_errno(dev, r, "Failed to get ifindex: %m");
59+ return log_device_warning_errno(dev, r, "Failed to get ifindex: %m");
60
61 if (naming_scheme_has(NAMING_REPLACE_STRICTLY) &&
62 !ifname_valid(event->name)) {
63@@ -904,39 +897,82 @@ static int rename_netif(UdevEvent *event) {
64 return 0;
65 }
66
67- /* Set ID_RENAMING boolean property here, and drop it in the corresponding move uevent later. */
68- r = device_add_property(dev, "ID_RENAMING", "1");
69+ r = sd_device_get_sysname(dev, &s);
70 if (r < 0)
71- return log_device_warning_errno(dev, r, "Failed to add 'ID_RENAMING' property: %m");
72+ return log_device_warning_errno(dev, r, "Failed to get sysname: %m");
73
74- r = device_rename(dev, event->name);
75+ if (streq(event->name, s))
76+ return 0; /* The interface name is already requested name. */
77+
78+ old_sysname = strdup(s);
79+ if (!old_sysname)
80+ return -ENOMEM;
81+
82+ r = sd_device_get_syspath(dev, &s);
83 if (r < 0)
84- return log_device_warning_errno(dev, r, "Failed to update properties with new name '%s': %m", event->name);
85+ return log_device_warning_errno(dev, r, "Failed to get syspath: %m");
86+
87+ old_syspath = strdup(s);
88+ if (!old_syspath)
89+ return -ENOMEM;
90+
91+ r = device_rename(dev, event->name);
92+ if (r < 0) {
93+ log_device_warning_errno(dev, r, "Failed to update properties with new name '%s': %m", event->name);
94+ goto revert;
95+ }
96+
97+ /* Set ID_RENAMING boolean property here. It will be dropped when the corresponding move uevent is processed. */
98+ r = device_add_property(dev, "ID_RENAMING", "1");
99+ if (r < 0) {
100+ log_device_warning_errno(dev, r, "Failed to add 'ID_RENAMING' property: %m");
101+ goto revert;
102+ }
103
104 /* Also set ID_RENAMING boolean property to cloned sd_device object and save it to database
105 * before calling rtnl_set_link_name(). Otherwise, clients (e.g., systemd-networkd) may receive
106 * RTM_NEWLINK netlink message before the database is updated. */
107 r = device_add_property(event->dev_db_clone, "ID_RENAMING", "1");
108- if (r < 0)
109- return log_device_warning_errno(event->dev_db_clone, r, "Failed to add 'ID_RENAMING' property: %m");
110+ if (r < 0) {
111+ log_device_warning_errno(event->dev_db_clone, r, "Failed to add 'ID_RENAMING' property: %m");
112+ goto revert;
113+ }
114
115 r = device_update_db(event->dev_db_clone);
116- if (r < 0)
117- return log_device_debug_errno(event->dev_db_clone, r, "Failed to update database under /run/udev/data/: %m");
118+ if (r < 0) {
119+ log_device_debug_errno(event->dev_db_clone, r, "Failed to update database under /run/udev/data/: %m");
120+ goto revert;
121+ }
122
123 r = rtnl_set_link_name(&event->rtnl, ifindex, event->name);
124- if (r == -EBUSY) {
125- log_device_info(dev, "Network interface '%s' is already up, cannot rename to '%s'.",
126- oldname, event->name);
127- return 0;
128+ if (r < 0) {
129+ if (r == -EBUSY) {
130+ log_device_info(dev, "Network interface '%s' is already up, cannot rename to '%s'.",
131+ old_sysname, event->name);
132+ r = 0;
133+ } else
134+ log_device_error_errno(dev, r, "Failed to rename network interface %i from '%s' to '%s': %m",
135+ ifindex, old_sysname, event->name);
136+ goto revert;
137 }
138- if (r < 0)
139- return log_device_error_errno(dev, r, "Failed to rename network interface %i from '%s' to '%s': %m",
140- ifindex, oldname, event->name);
141-
142- log_device_debug(dev, "Network interface %i is renamed from '%s' to '%s'", ifindex, oldname, event->name);
143
144+ log_device_debug(dev, "Network interface %i is renamed from '%s' to '%s'", ifindex, old_sysname, event->name);
145 return 1;
146+
147+revert:
148+ /* Restore 'dev_db_clone' */
149+ (void) device_add_property(event->dev_db_clone, "ID_RENAMING", NULL);
150+ (void) device_update_db(event->dev_db_clone);
151+
152+ /* Restore 'dev' */
153+ (void) device_set_syspath(dev, old_syspath, /* verify = */ false);
154+ if (sd_device_get_property_value(dev, "INTERFACE_OLD", &s) >= 0) {
155+ (void) device_add_property_internal(dev, "INTERFACE", s);
156+ (void) device_add_property_internal(dev, "INTERFACE_OLD", NULL);
157+ }
158+ (void) device_add_property(dev, "ID_RENAMING", NULL);
159+
160+ return r;
161 }
162
163 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
61lp2002445/sd-netlink-restore-altname-on-error-in-rtnl_set_link_name.patch61lp2002445/sd-netlink-restore-altname-on-error-in-rtnl_set_link_name.patch
62lp2002445/udev-attempt-device-rename-even-if-interface-is-up.patch62lp2002445/udev-attempt-device-rename-even-if-interface-is-up.patch
63lp2002445/sd-netlink-add-a-test-for-rtnl_set_link_name.patch63lp2002445/sd-netlink-add-a-test-for-rtnl_set_link_name.patch
64lp2002445/sd-device-make-device_set_syspath-clear-sysname-and-sysnu.patch
65lp2002445/udev-restore-syspath-and-properties-on-failure.patch
66lp2002445/sd-device-introduce-device_get_property_int.patch
67lp2002445/core-device-ignore-failed-uevents.patch

Subscribers

People subscribed via source and target branches