From: Yu Watanabe Date: Sun, 25 Aug 2024 21:24:24 +0000 (+0900) Subject: sd-device: make sd_device_new_from_subsystem_sysname() stricter X-Git-Tag: v257-rc1~612^2~1 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=cd7c71154cd62d3f50c07ce387edd9c20aebd7bc;p=thirdparty%2Fsystemd.git sd-device: make sd_device_new_from_subsystem_sysname() stricter As workarounded by fc0cbed2db860d163d59d04c32fa6ec30bd0606f, the pair of subsystem and sysname is not unique. For examples, - /sys/bus/gpio and /sys/class/gpio, both have gpiochip%N. However, these point to different devpaths. - /sys/bus/mdio_bus and /sys/class/mdio_bus, - /sys/bus/mei and /sys/class/mei, - /sys/bus/typec and /sys/class/typec, and so on. Let's refuse to provide sd_device object in such cases. --- diff --git a/src/libsystemd/sd-device/sd-device.c b/src/libsystemd/sd-device/sd-device.c index 383445edc8f..01fa90b1ff5 100644 --- a/src/libsystemd/sd-device/sd-device.c +++ b/src/libsystemd/sd-device/sd-device.c @@ -386,25 +386,80 @@ _public_ int sd_device_new_from_ifindex(sd_device **ret, int ifindex) { return 0; } -static int device_strjoin_new( +static int device_new_from_path_join( + sd_device **device, + const char *subsystem, + const char *driver_subsystem, + const char *sysname, const char *a, const char *b, const char *c, - const char *d, - sd_device **ret) { + const char *d) { - const char *p; + _cleanup_(sd_device_unrefp) sd_device *new_device = NULL; + _cleanup_free_ char *p = NULL; int r; - p = strjoina(a, b, c, d); - if (access(p, F_OK) < 0) - return IN_SET(errno, ENOENT, ENAMETOOLONG) ? 0 : -errno; /* If this sysfs is too long then it doesn't exist either */ + assert(device); + assert(subsystem); + assert(sysname); + + p = path_join(a, b, c, d); + if (!p) + return -ENOMEM; + + r = sd_device_new_from_syspath(&new_device, p); + if (r == -ENODEV) + return 0; + if (r < 0) + return r; + + /* Check if the found device really has the expected subsystem and sysname, for safety. */ + if (!device_in_subsystem(new_device, subsystem)) + return 0; + + const char *new_driver_subsystem = NULL; + (void) sd_device_get_driver_subsystem(new_device, &new_driver_subsystem); - r = sd_device_new_from_syspath(ret, p); + if (!streq_ptr(driver_subsystem, new_driver_subsystem)) + return 0; + + const char *new_sysname; + r = sd_device_get_sysname(new_device, &new_sysname); if (r < 0) return r; - return 1; + if (!streq(sysname, new_sysname)) + return 0; + + /* If this is the first device we found, then take it. */ + if (!*device) { + *device = TAKE_PTR(new_device); + return 1; + } + + /* Unfortunately, (subsystem, sysname) pair is not unique. For examples, + * - /sys/bus/gpio and /sys/class/gpio, both have gpiochip%N. However, these point to different devpaths. + * - /sys/bus/mdio_bus and /sys/class/mdio_bus, + * - /sys/bus/mei and /sys/class/mei, + * - /sys/bus/typec and /sys/class/typec, and so on. + * Hence, if we already know a device, then we need to check if it is equivalent to the newly found one. */ + + const char *devpath, *new_devpath; + r = sd_device_get_devpath(*device, &devpath); + if (r < 0) + return r; + + r = sd_device_get_devpath(new_device, &new_devpath); + if (r < 0) + return r; + + if (!streq(devpath, new_devpath)) + return log_debug_errno(SYNTHETIC_ERRNO(ETOOMANYREFS), + "sd-device: found multiple devices for subsystem=%s and sysname=%s, refusing: %s, %s", + subsystem, sysname, devpath, new_devpath); + + return 1; /* Fortunately, they are consistent. */ } _public_ int sd_device_new_from_subsystem_sysname( @@ -412,6 +467,7 @@ _public_ int sd_device_new_from_subsystem_sysname( const char *subsystem, const char *sysname) { + _cleanup_(sd_device_unrefp) sd_device *device = NULL; char *name; int r; @@ -430,19 +486,15 @@ _public_ int sd_device_new_from_subsystem_sysname( if (streq(subsystem, "subsystem")) { FOREACH_STRING(s, "/sys/bus/", "/sys/class/") { - r = device_strjoin_new(s, name, NULL, NULL, ret); + r = device_new_from_path_join(&device, subsystem, NULL, sysname, s, name, NULL, NULL); if (r < 0) return r; - if (r > 0) - return 0; } } else if (streq(subsystem, "module")) { - r = device_strjoin_new("/sys/module/", name, NULL, NULL, ret); + r = device_new_from_path_join(&device, subsystem, NULL, sysname, "/sys/module/", name, NULL, NULL); if (r < 0) return r; - if (r > 0) - return 0; } else if (streq(subsystem, "drivers")) { const char *sep; @@ -454,35 +506,31 @@ _public_ int sd_device_new_from_subsystem_sysname( sep++; if (streq(sep, "drivers")) /* If the sysname is "drivers", then it's the drivers directory itself that is meant. */ - r = device_strjoin_new("/sys/bus/", subsys, "/drivers", NULL, ret); + r = device_new_from_path_join(&device, subsystem, subsys, "drivers", "/sys/bus/", subsys, "/drivers", NULL); else - r = device_strjoin_new("/sys/bus/", subsys, "/drivers/", sep, ret); + r = device_new_from_path_join(&device, subsystem, subsys, sep, "/sys/bus/", subsys, "/drivers/", sep); if (r < 0) return r; - if (r > 0) - return 0; } } - r = device_strjoin_new("/sys/bus/", subsystem, "/devices/", name, ret); + r = device_new_from_path_join(&device, subsystem, NULL, sysname, "/sys/bus/", subsystem, "/devices/", name); if (r < 0) return r; - if (r > 0) - return 0; - r = device_strjoin_new("/sys/class/", subsystem, "/", name, ret); + r = device_new_from_path_join(&device, subsystem, NULL, sysname, "/sys/class/", subsystem, name, NULL); if (r < 0) return r; - if (r > 0) - return 0; - r = device_strjoin_new("/sys/firmware/", subsystem, "/", name, ret); + r = device_new_from_path_join(&device, subsystem, NULL, sysname, "/sys/firmware/", subsystem, name, NULL); if (r < 0) return r; - if (r > 0) - return 0; - return -ENODEV; + if (!device) + return -ENODEV; + + *ret = TAKE_PTR(device); + return 0; } _public_ int sd_device_new_from_stat_rdev(sd_device **ret, const struct stat *st) { diff --git a/src/libsystemd/sd-device/test-sd-device.c b/src/libsystemd/sd-device/test-sd-device.c index a048860dbe7..620615b6bb7 100644 --- a/src/libsystemd/sd-device/test-sd-device.c +++ b/src/libsystemd/sd-device/test-sd-device.c @@ -74,9 +74,7 @@ static void test_sd_device_one(sd_device *d) { r = sd_device_get_subsystem(d, &subsystem); 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. */ + else { const char *name, *id; if (streq(subsystem, "drivers")) { @@ -85,10 +83,14 @@ static void test_sd_device_one(sd_device *d) { name = strjoina(driver_subsystem, ":", sysname); } else name = sysname; - assert_se(sd_device_new_from_subsystem_sysname(&dev, subsystem, name) >= 0); - assert_se(sd_device_get_syspath(dev, &val) >= 0); - assert_se(streq(syspath, val)); - dev = sd_device_unref(dev); + + r = sd_device_new_from_subsystem_sysname(&dev, subsystem, name); + if (r >= 0) { + assert_se(sd_device_get_syspath(dev, &val) >= 0); + assert_se(streq(syspath, val)); + dev = sd_device_unref(dev); + } else + ASSERT_ERROR(r, ETOOMANYREFS); /* The device ID depends on subsystem. */ assert_se(sd_device_get_device_id(d, &id) >= 0); @@ -97,12 +99,12 @@ static void test_sd_device_one(sd_device *d) { log_device_warning_errno(d, r, "Failed to create sd-device object from device ID \"%s\". " "Maybe running on a non-host network namespace.", id); - else { - assert_se(r >= 0); + else if (r >= 0) { assert_se(sd_device_get_syspath(dev, &val) >= 0); assert_se(streq(syspath, val)); dev = sd_device_unref(dev); - } + } else + ASSERT_ERROR(r, ETOOMANYREFS); /* These require udev database, and reading database requires device ID. */ r = sd_device_get_is_initialized(d);