sd-device: make sd_device_new_from_subsystem_sysname() stricter

As workarounded by fc0cbed2db, 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.
This commit is contained in:
Yu Watanabe
2024-08-26 06:24:24 +09:00
parent 1ff0164be5
commit cd7c71154c
2 changed files with 89 additions and 39 deletions

View File

@@ -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);
r = sd_device_new_from_syspath(ret, p);
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;
return 1;
/* 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);
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;
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) {

View File

@@ -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);