From 4d38294baf31ac193f4897dccb2645b86f1c390f Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Tue, 10 May 2022 23:05:04 +0900 Subject: [PATCH 1/2] sd-device: always translate sysname to sysfs filename Previously, in sd_device_new_from_subsystem_sysname(), '/' in sysname was replaced '!' for several limited subsystems. This was based on a wrong assumption that no sysname in e.g. driver subsystem does not contain '!'. And the assumption is actually wrong, and trigger issue #23327. In device_set_sysname_and_sysnum() we unconditionally replace '!' in the filename. Hence, the translation in sd_device_new_from_subsystem_sysname() must be also done unconditionally. Fixes #23327. --- src/libsystemd/sd-device/sd-device.c | 22 ++++++++++------------ 1 file changed, 10 insertions(+), 12 deletions(-) diff --git a/src/libsystemd/sd-device/sd-device.c b/src/libsystemd/sd-device/sd-device.c index 0bfcdcdf2d..aaf951ced8 100644 --- a/src/libsystemd/sd-device/sd-device.c +++ b/src/libsystemd/sd-device/sd-device.c @@ -414,17 +414,21 @@ _public_ int sd_device_new_from_subsystem_sysname( assert_return(path_is_normalized(subsystem), -EINVAL); assert_return(path_is_normalized(sysname), -EINVAL); + /* translate sysname back to sysfs filename */ + name = strdupa_safe(sysname); + string_replace_char(name, '/', '!'); + if (streq(subsystem, "subsystem")) { FOREACH_STRING(s, "/sys/bus/", "/sys/class/") { - r = device_strjoin_new(s, sysname, NULL, NULL, ret); + r = device_strjoin_new(s, name, NULL, NULL, ret); if (r < 0) return r; if (r > 0) return 0; } - } else if (streq(subsystem, "module")) { - r = device_strjoin_new("/sys/module/", sysname, NULL, NULL, ret); + } else if (streq(subsystem, "module")) { + r = device_strjoin_new("/sys/module/", name, NULL, NULL, ret); if (r < 0) return r; if (r > 0) @@ -433,10 +437,10 @@ _public_ int sd_device_new_from_subsystem_sysname( } else if (streq(subsystem, "drivers")) { const char *sep; - sep = strchr(sysname, ':'); + sep = strchr(name, ':'); if (sep && sep[1] != '\0') { /* Require ":" and something non-empty after that. */ - const char *subsys = memdupa_suffix0(sysname, sep - sysname); + const char *subsys = memdupa_suffix0(name, sep - name); sep++; if (streq(sep, "drivers")) /* If the sysname is "drivers", then it's the drivers directory itself that is meant. */ @@ -450,12 +454,6 @@ _public_ int sd_device_new_from_subsystem_sysname( } } - /* translate sysname back to sysfs filename */ - name = strdupa_safe(sysname); - for (size_t i = 0; name[i]; i++) - if (name[i] == '/') - name[i] = '!'; - r = device_strjoin_new("/sys/bus/", subsystem, "/devices/", name, ret); if (r < 0) return r; @@ -468,7 +466,7 @@ _public_ int sd_device_new_from_subsystem_sysname( if (r > 0) return 0; - r = device_strjoin_new("/sys/firmware/", subsystem, "/", sysname, ret); + r = device_strjoin_new("/sys/firmware/", subsystem, "/", name, ret); if (r < 0) return r; if (r > 0) From fc0cbed2db860d163d59d04c32fa6ec30bd0606f Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Thu, 12 May 2022 02:00:38 +0900 Subject: [PATCH 2/2] test-sd-device: skip gpio subsystem There exist /sys/class/gpio and /sys/bus/gpio, and both have gpiochip%N device. However, these point to different devpaths. --- src/libsystemd/sd-device/test-sd-device.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/src/libsystemd/sd-device/test-sd-device.c b/src/libsystemd/sd-device/test-sd-device.c index 3bd3761f3b..5e3536be40 100644 --- a/src/libsystemd/sd-device/test-sd-device.c +++ b/src/libsystemd/sd-device/test-sd-device.c @@ -66,7 +66,11 @@ static void test_sd_device_one(sd_device *d) { assert_se(r == -ENOENT); r = sd_device_get_subsystem(d, &subsystem); - if (r >= 0) { + if (r < 0) + assert_se(r == -ENOENT); + else if (!streq(subsystem, "gpio")) { /* Unfortunately, there exist /sys/class/gpio and /sys/bus/gpio. + * Hence, sd_device_new_from_subsystem_sysname() and + * sd_device_new_from_device_id() may not work as expected. */ const char *name, *id; if (streq(subsystem, "drivers")) @@ -102,8 +106,7 @@ static void test_sd_device_one(sd_device *d) { r = sd_device_get_property_value(d, "ID_NET_DRIVER", &val); assert_se(r >= 0 || r == -ENOENT); - } else - assert_se(r == -ENOENT); + } is_block = streq_ptr(subsystem, "block");