From 1793bb611249b9525f6ed17964347d377d97e494 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 11 Apr 2022 22:02:23 +0200 Subject: [PATCH 01/11] sd-device: use chase_symlinks() O_PATH fd chase_symlinks() can return its last O_PATH fd to us. Let's use that and make the access() check a bit tighter by going via faccessat() on the O_PATH fd. This doesn't really change too much, but is nice in context of the next commit, which uses the O_PATH fd in one other way. --- src/libsystemd/sd-device/sd-device.c | 31 ++++++++++++++++------------ 1 file changed, 18 insertions(+), 13 deletions(-) diff --git a/src/libsystemd/sd-device/sd-device.c b/src/libsystemd/sd-device/sd-device.c index d31526fc22..eb50f592b7 100644 --- a/src/libsystemd/sd-device/sd-device.c +++ b/src/libsystemd/sd-device/sd-device.c @@ -150,7 +150,9 @@ int device_set_syspath(sd_device *device, const char *_syspath, bool verify) { _syspath); if (verify) { - r = chase_symlinks(_syspath, NULL, 0, &syspath, NULL); + _cleanup_close_ int fd = -1; + + r = chase_symlinks(_syspath, NULL, 0, &syspath, &fd); if (r == -ENOENT) /* the device does not exist (any more?) */ return log_debug_errno(SYNTHETIC_ERRNO(ENODEV), @@ -181,25 +183,28 @@ int device_set_syspath(sd_device *device, const char *_syspath, bool verify) { path_simplify(syspath); } - if (path_startswith(syspath, "/sys/devices/")) { - char *path; + if (path_startswith(syspath, "/sys/devices/")) { + /* For proper devices, stricter rules apply: they must have a 'uevent' file, + * otherwise we won't allow them */ - /* all 'devices' require an 'uevent' file */ - path = strjoina(syspath, "/uevent"); - if (access(path, F_OK) < 0) { + if (faccessat(fd, "uevent", F_OK, 0) < 0) { if (errno == ENOENT) - /* This is not a valid device. - * Note, this condition is quite often satisfied when - * enumerating devices or finding a parent device. + /* This is not a valid device. Note, this condition is quite often + * satisfied when enumerating devices or finding a parent device. * Hence, use log_trace_errno() here. */ return log_trace_errno(SYNTHETIC_ERRNO(ENODEV), - "sd-device: the uevent file \"%s\" does not exist.", path); + "sd-device: the uevent file \"%s/uevent\" does not exist.", syspath); - return log_debug_errno(errno, "sd-device: cannot access uevent file for %s: %m", syspath); + return log_debug_errno(errno, "sd-device: cannot find uevent file for %s: %m", syspath); } } else { - /* everything else just needs to be a directory */ - if (!is_dir(syspath, false)) + struct stat st; + + /* For everything else lax rules apply: they just need to be a directory */ + + if (fstat(fd, &st) < 0) + return log_debug_errno(errno, "sd-device: failed to check if syspath \"%s\" is a directory: %m", syspath); + if (!S_ISDIR(st.st_mode)) return log_debug_errno(SYNTHETIC_ERRNO(ENODEV), "sd-device: the syspath \"%s\" is not a directory.", syspath); } From a7910612a5be324d8b6994a2f7e1a2edb63ad03c Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 11 Apr 2022 22:04:06 +0200 Subject: [PATCH 02/11] sd-device: don't accept non-sysfs paths There are some file systems mounted below /sys/ that are not actually sysfs, i.e. are not arranged in a sysfs/kobject style. Let's refuse those early. (Example, /sys/fs/cgroup/ and similar.) (Also, let's add an env var for this, so that it can be turned off for test cases.) --- docs/ENVIRONMENT.md | 6 +++++- src/libsystemd/sd-device/sd-device.c | 17 +++++++++++++++++ test/udev-test.pl | 4 ++++ 3 files changed, 26 insertions(+), 1 deletion(-) diff --git a/docs/ENVIRONMENT.md b/docs/ENVIRONMENT.md index 0cbe5cfb6b..5f02f888a5 100644 --- a/docs/ENVIRONMENT.md +++ b/docs/ENVIRONMENT.md @@ -199,7 +199,7 @@ All tools: or whenever they change if it wants to integrate with `systemd-logind`'s APIs. -`systemd-udevd`: +`systemd-udevd` and sd-device library: * `$NET_NAMING_SCHEME=` — if set, takes a network naming scheme (i.e. one of "v238", "v239", "v240"…, or the special value "latest") as parameter. If @@ -211,6 +211,10 @@ All tools: prefixed with `:` in which case the kernel command line option takes precedence, if it is specified as well. +* `$SYSTEMD_DEVICE_VERIFY_SYSFS` — if set to "0", disables verification that + devices sysfs path are actually backed by sysfs. Relaxing this verification + is useful for testing purposes. + `nss-systemd`: * `$SYSTEMD_NSS_BYPASS_SYNTHETIC=1` — if set, `nss-systemd` won't synthesize diff --git a/src/libsystemd/sd-device/sd-device.c b/src/libsystemd/sd-device/sd-device.c index eb50f592b7..0435beca16 100644 --- a/src/libsystemd/sd-device/sd-device.c +++ b/src/libsystemd/sd-device/sd-device.c @@ -13,6 +13,7 @@ #include "device-private.h" #include "device-util.h" #include "dirent-util.h" +#include "env-util.h" #include "fd-util.h" #include "fileio.h" #include "format-util.h" @@ -20,6 +21,7 @@ #include "hashmap.h" #include "id128-util.h" #include "macro.h" +#include "missing_magic.h" #include "netlink-util.h" #include "parse-util.h" #include "path-util.h" @@ -208,6 +210,21 @@ int device_set_syspath(sd_device *device, const char *_syspath, bool verify) { return log_debug_errno(SYNTHETIC_ERRNO(ENODEV), "sd-device: the syspath \"%s\" is not a directory.", syspath); } + + /* Only operate on sysfs, i.e. refuse going down into /sys/fs/cgroup/ or similar places where + * things are not arranged as kobjects in kernel, and hence don't necessarily have + * kobject/attribute structure. */ + r = getenv_bool_secure("SYSTEMD_DEVICE_VERIFY_SYSFS"); + if (r < 0 && r != -ENXIO) + log_debug_errno(r, "Failed to parse $SYSTEMD_DEVICE_VERIFY_SYSFS value: %m"); + if (r != 0) { + r = fd_is_fs_type(fd, SYSFS_MAGIC); + if (r < 0) + return log_debug_errno(r, "sd-device: failed to check if syspath \"%s\" is backed by sysfs.", syspath); + if (r == 0) + return log_debug_errno(SYNTHETIC_ERRNO(ENODEV), + "sd-device: the syspath \"%s\" is outside of sysfs, refusing.", syspath); + } } else { syspath = strdup(_syspath); if (!syspath) diff --git a/test/udev-test.pl b/test/udev-test.pl index 3aeac65578..802bc1f3de 100755 --- a/test/udev-test.pl +++ b/test/udev-test.pl @@ -33,6 +33,10 @@ BEGIN { } } +# Relax sd-device's sysfs verification, since we want to provide a fake sysfs +# here that actually is a tmpfs. +$ENV{"SYSTEMD_DEVICE_VERIFY_SYSFS"}="0"; + my $udev_bin = "./test-udev"; my $valgrind = 0; my $gdb = 0; From 5cf0ee31eca931c706e1f316099b449a12cadb6d Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 12 Apr 2022 11:42:30 +0200 Subject: [PATCH 03/11] sd-device: generate e better error code when trying to allocate sd_device for non-dir Currently, for sysfs paths outside of /sys/devices/ we do better checking if something is a suitable path: we check if it's actually a directory, and if not return ENODEV. Let's make the codepath for nodes *inside* of /sys/device/ similar: let's also return ENODEV if the path supplied is not a directory. Previously, we'd return ENOTDIR in that case, which is quite confusing I think. --- src/libsystemd/sd-device/sd-device.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/libsystemd/sd-device/sd-device.c b/src/libsystemd/sd-device/sd-device.c index 0435beca16..8c1b1194cd 100644 --- a/src/libsystemd/sd-device/sd-device.c +++ b/src/libsystemd/sd-device/sd-device.c @@ -196,6 +196,10 @@ int device_set_syspath(sd_device *device, const char *_syspath, bool verify) { * Hence, use log_trace_errno() here. */ return log_trace_errno(SYNTHETIC_ERRNO(ENODEV), "sd-device: the uevent file \"%s/uevent\" does not exist.", syspath); + if (errno == ENOTDIR) + /* Not actually a directory. */ + return log_debug_errno(SYNTHETIC_ERRNO(ENODEV), + "sd-device: the syspath \"%s\" is not a directory.", syspath); return log_debug_errno(errno, "sd-device: cannot find uevent file for %s: %m", syspath); } From e7c7d79d52643656cb97d60003a6a3f44fecf916 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 11 Apr 2022 23:00:52 +0200 Subject: [PATCH 04/11] sd-device: split out checking of matches from enumerator_scan_dir_and_add_devices() No change in behaviour, just some splitting out of code. --- src/libsystemd/sd-device/device-enumerator.c | 51 ++++++++++++++------ 1 file changed, 37 insertions(+), 14 deletions(-) diff --git a/src/libsystemd/sd-device/device-enumerator.c b/src/libsystemd/sd-device/device-enumerator.c index 61645f44d8..55b8aba53a 100644 --- a/src/libsystemd/sd-device/device-enumerator.c +++ b/src/libsystemd/sd-device/device-enumerator.c @@ -547,7 +547,42 @@ static int match_initialized(sd_device_enumerator *enumerator, sd_device *device return (enumerator->match_initialized == MATCH_INITIALIZED_NO) == (r == 0); } -static int enumerator_scan_dir_and_add_devices(sd_device_enumerator *enumerator, const char *basedir, const char *subdir1, const char *subdir2) { +static int test_matches( + sd_device_enumerator *enumerator, + sd_device *device) { + + int r; + + assert(enumerator); + assert(device); + + /* Checks all matches, except for the sysname match (which the caller should check beforehand) */ + + r = match_initialized(enumerator, device); + if (r <= 0) + return r; + + if (!device_match_parent(device, enumerator->match_parent, NULL)) + return false; + + if (!match_tag(enumerator, device)) + return false; + + if (!match_property(enumerator, device)) + return false; + + if (!device_match_sysattr(device, enumerator->match_sysattr, enumerator->nomatch_sysattr)) + return false; + + return true; +} + +static int enumerator_scan_dir_and_add_devices( + sd_device_enumerator *enumerator, + const char *basedir, + const char *subdir1, + const char *subdir2) { + _cleanup_closedir_ DIR *dir = NULL; char *path; int k, r = 0; @@ -589,25 +624,13 @@ static int enumerator_scan_dir_and_add_devices(sd_device_enumerator *enumerator, continue; } - k = match_initialized(enumerator, device); + k = test_matches(enumerator, device); if (k <= 0) { if (k < 0) r = k; continue; } - if (!device_match_parent(device, enumerator->match_parent, NULL)) - continue; - - if (!match_tag(enumerator, device)) - continue; - - if (!match_property(enumerator, device)) - continue; - - if (!device_match_sysattr(device, enumerator->match_sysattr, enumerator->nomatch_sysattr)) - continue; - k = device_enumerator_add_device(enumerator, device); if (k < 0) r = k; From 29a5428f1c7154c23e13939160e47be6e420a863 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 12 Apr 2022 11:25:00 +0200 Subject: [PATCH 05/11] sd-device: filter regular files when enumerating Currently if enumerating with debug logging you'll likely see something like this: sd-device: the syspath "/sys/class/devcoredump/disabled" is not a directory. sd-device: the syspath "/sys/class/firmware/timeout" is not a directory. sd-device: the syspath "/sys/class/zram-control/hot_remove" is not a directory. sd-device: the syspath "/sys/class/zram-control/hot_add" is not a directory. sd-device: the syspath "/sys/class/drm/version" is not a directory. This is because these sysfs classes place regular files in these directories, which we so far didn't expect. Let's filter them early, and only bother with enumerated inodes if they actually are dirs or symlinks, i.e. can be kobject dirs. Regular file definitely never are kobject dirs... --- src/libsystemd/sd-device/device-enumerator.c | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/src/libsystemd/sd-device/device-enumerator.c b/src/libsystemd/sd-device/device-enumerator.c index 55b8aba53a..9345cbea6c 100644 --- a/src/libsystemd/sd-device/device-enumerator.c +++ b/src/libsystemd/sd-device/device-enumerator.c @@ -577,6 +577,17 @@ static int test_matches( return true; } +static bool relevant_sysfs_subdir(const struct dirent *de) { + assert(de); + + if (de->d_name[0] == '.') + return false; + + /* Also filter out regular files and such, i.e. stuff that definitely isn't a kobject path. (Note + * that we rely on the fact that sysfs fills in d_type here, i.e. doesn't do DT_UNKNOWN) */ + return IN_SET(de->d_type, DT_DIR, DT_LNK); +} + static int enumerator_scan_dir_and_add_devices( sd_device_enumerator *enumerator, const char *basedir, @@ -607,7 +618,7 @@ static int enumerator_scan_dir_and_add_devices( _cleanup_(sd_device_unrefp) sd_device *device = NULL; char syspath[strlen(path) + 1 + strlen(de->d_name) + 1]; - if (de->d_name[0] == '.') + if (!relevant_sysfs_subdir(de)) continue; if (!match_sysname(enumerator, de->d_name)) @@ -682,7 +693,7 @@ static int enumerator_scan_dir( FOREACH_DIRENT_ALL(de, dir, return -errno) { int k; - if (de->d_name[0] == '.') + if (!relevant_sysfs_subdir(de)) continue; if (!match_subsystem(enumerator, subsystem ? : de->d_name)) From be247835c7189e49ae93728c6f638bd5cad1fd9f Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 12 Apr 2022 15:45:07 +0200 Subject: [PATCH 06/11] sd-device: add some comments --- src/libsystemd/sd-device/sd-device.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/libsystemd/sd-device/sd-device.c b/src/libsystemd/sd-device/sd-device.c index 8c1b1194cd..ac4a538afa 100644 --- a/src/libsystemd/sd-device/sd-device.c +++ b/src/libsystemd/sd-device/sd-device.c @@ -261,7 +261,7 @@ _public_ int sd_device_new_from_syspath(sd_device **ret, const char *syspath) { if (r < 0) return r; - r = device_set_syspath(device, syspath, true); + r = device_set_syspath(device, syspath, /* verify= */ true); if (r < 0) return r; @@ -943,7 +943,8 @@ int device_set_drivers_subsystem(sd_device *device) { if (!drivers) return -EINVAL; - r = path_find_last_component(devpath, false, &drivers, &p); + /* Find the path component immediately before the "/drivers/" string */ + r = path_find_last_component(devpath, /* accept_dot_dot= */ false, &drivers, &p); if (r < 0) return r; if (r == 0) From 0b859c9773a14f12d512447c27e3caf274456498 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 12 Apr 2022 15:45:48 +0200 Subject: [PATCH 07/11] sd-device: properly support some corner case syspath --- src/libsystemd/sd-device/sd-device.c | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/src/libsystemd/sd-device/sd-device.c b/src/libsystemd/sd-device/sd-device.c index ac4a538afa..fb50a688b0 100644 --- a/src/libsystemd/sd-device/sd-device.c +++ b/src/libsystemd/sd-device/sd-device.c @@ -439,7 +439,10 @@ _public_ int sd_device_new_from_subsystem_sysname( const char *subsys = memdupa_suffix0(sysname, sep - sysname); sep++; - r = device_strjoin_new("/sys/bus/", subsys, "/drivers/", sep, ret); + 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); + else + r = device_strjoin_new("/sys/bus/", subsys, "/drivers/", sep, ret); if (r < 0) return r; if (r > 0) @@ -940,6 +943,8 @@ int device_set_drivers_subsystem(sd_device *device) { return r; drivers = strstr(devpath, "/drivers/"); + if (!drivers) + drivers = endswith(devpath, "/drivers"); if (!drivers) return -EINVAL; @@ -986,11 +991,11 @@ _public_ int sd_device_get_subsystem(sd_device *device, const char **ret) { if (subsystem) r = device_set_subsystem(device, subsystem); /* use implicit names */ - else if (path_startswith(device->devpath, "/module/")) + else if (!isempty(path_startswith(device->devpath, "/module/"))) r = device_set_subsystem(device, "module"); - else if (strstr(syspath, "/drivers/")) + else if (strstr(syspath, "/drivers/") || endswith(syspath, "/drivers")) r = device_set_drivers_subsystem(device); - else if (PATH_STARTSWITH_SET(device->devpath, "/class/", "/bus/")) + else if (!isempty(PATH_STARTSWITH_SET(device->devpath, "/class/", "/bus/"))) r = device_set_subsystem(device, "subsystem"); else { device->subsystem_set = true; From 00dfbf35fde43a8106bd0132109ac25ec2033b09 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 11 Apr 2022 23:13:40 +0200 Subject: [PATCH 08/11] sd-device: include parent devices in enumeration --- src/libsystemd/sd-device/device-enumerator.c | 93 +++++++++++++++----- 1 file changed, 71 insertions(+), 22 deletions(-) diff --git a/src/libsystemd/sd-device/device-enumerator.c b/src/libsystemd/sd-device/device-enumerator.c index 9345cbea6c..97f4ee356c 100644 --- a/src/libsystemd/sd-device/device-enumerator.c +++ b/src/libsystemd/sd-device/device-enumerator.c @@ -577,6 +577,28 @@ static int test_matches( return true; } +static bool match_subsystem(sd_device_enumerator *enumerator, const char *subsystem) { + const char *subsystem_match; + + assert(enumerator); + + if (!subsystem) + return false; + + SET_FOREACH(subsystem_match, enumerator->nomatch_subsystem) + if (fnmatch(subsystem_match, subsystem, 0) == 0) + return false; + + if (set_isempty(enumerator->match_subsystem)) + return true; + + SET_FOREACH(subsystem_match, enumerator->match_subsystem) + if (fnmatch(subsystem_match, subsystem, 0) == 0) + return true; + + return false; +} + static bool relevant_sysfs_subdir(const struct dirent *de) { assert(de); @@ -617,6 +639,7 @@ static int enumerator_scan_dir_and_add_devices( FOREACH_DIRENT_ALL(de, dir, return -errno) { _cleanup_(sd_device_unrefp) sd_device *device = NULL; char syspath[strlen(path) + 1 + strlen(de->d_name) + 1]; + sd_device *upwards; if (!relevant_sysfs_subdir(de)) continue; @@ -645,33 +668,59 @@ static int enumerator_scan_dir_and_add_devices( k = device_enumerator_add_device(enumerator, device); if (k < 0) r = k; + + /* Also include all potentially matching parent devices in the enumeration. These are things + * like root busses — e.g. /sys/devices/pci0000:00/ or /sys/devices/pnp0/, which ar not + * linked from /sys/class/ or /sys/bus/, hence pick them up explicitly here. */ + upwards = device; + for (;;) { + const char *ss, *usn; + + k = sd_device_get_parent(upwards, &upwards); + if (k == -ENOENT) /* Reached the top? */ + break; + if (k < 0) { + r = k; + break; + } + + k = sd_device_get_subsystem(upwards, &ss); + if (k == -ENOENT) /* Has no subsystem? */ + continue; + if (k < 0) { + r = k; + break; + } + + if (!match_subsystem(enumerator, ss)) + continue; + + k = sd_device_get_sysname(upwards, &usn); + if (k < 0) { + r = k; + break; + } + + if (!match_sysname(enumerator, usn)) + continue; + + k = test_matches(enumerator, upwards); + if (k < 0) + break; + if (k == 0) + continue; + + k = device_enumerator_add_device(enumerator, upwards); + if (k < 0) + r = k; + else if (k == 0) /* Exists already? Then no need to go further up. */ + break; + } } return r; } -static bool match_subsystem(sd_device_enumerator *enumerator, const char *subsystem) { - const char *subsystem_match; - - assert(enumerator); - - if (!subsystem) - return false; - - SET_FOREACH(subsystem_match, enumerator->nomatch_subsystem) - if (fnmatch(subsystem_match, subsystem, 0) == 0) - return false; - - if (set_isempty(enumerator->match_subsystem)) - return true; - - SET_FOREACH(subsystem_match, enumerator->match_subsystem) - if (fnmatch(subsystem_match, subsystem, 0) == 0) - return true; - - return false; -} - static int enumerator_scan_dir( sd_device_enumerator *enumerator, const char *basedir, From 9117d94b9a38a2d47500551fe3c6251f3bf285c0 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 5 Apr 2022 10:20:18 +0200 Subject: [PATCH 09/11] udevadm: add new --tree mode to "udevadm info" sysfs is a tree, hence let's a mode that allows showing it as such. --- man/udevadm.xml | 9 ++ src/basic/glyph-util.c | 2 + src/basic/glyph-util.h | 1 + src/test/test-locale-util.c | 1 + src/udev/udevadm-info.c | 215 ++++++++++++++++++++++++++++++++---- 5 files changed, 207 insertions(+), 21 deletions(-) diff --git a/man/udevadm.xml b/man/udevadm.xml index b327a4ef7e..edb1fccc0c 100644 --- a/man/udevadm.xml +++ b/man/udevadm.xml @@ -160,6 +160,15 @@ along the chain, up to the root of sysfs that can be used in udev rules. + + + + + Display a sysfs tree. This recursively iterates through the sysfs hierarchy and displays it + in a tree structure. If a path is specified only the subtree below that directory is + shown. This will show both device and subsystem items. + + diff --git a/src/basic/glyph-util.c b/src/basic/glyph-util.c index cfa7d041e7..1bba139bfc 100644 --- a/src/basic/glyph-util.c +++ b/src/basic/glyph-util.c @@ -40,6 +40,7 @@ const char *special_glyph(SpecialGlyph code) { [SPECIAL_GLYPH_TREE_RIGHT] = "`-", [SPECIAL_GLYPH_TREE_SPACE] = " ", [SPECIAL_GLYPH_TREE_TOP] = ",-", + [SPECIAL_GLYPH_VERTICAL_DOTTED] = ":", [SPECIAL_GLYPH_TRIANGULAR_BULLET] = ">", [SPECIAL_GLYPH_BLACK_CIRCLE] = "*", [SPECIAL_GLYPH_WHITE_CIRCLE] = "*", @@ -81,6 +82,7 @@ const char *special_glyph(SpecialGlyph code) { [SPECIAL_GLYPH_TREE_TOP] = u8"┌─", /* Single glyphs in both cases */ + [SPECIAL_GLYPH_VERTICAL_DOTTED] = u8"┆", [SPECIAL_GLYPH_TRIANGULAR_BULLET] = u8"‣", [SPECIAL_GLYPH_BLACK_CIRCLE] = u8"●", [SPECIAL_GLYPH_WHITE_CIRCLE] = u8"○", diff --git a/src/basic/glyph-util.h b/src/basic/glyph-util.h index 7e0a73842a..065dde8a62 100644 --- a/src/basic/glyph-util.h +++ b/src/basic/glyph-util.h @@ -12,6 +12,7 @@ typedef enum SpecialGlyph { SPECIAL_GLYPH_TREE_RIGHT, SPECIAL_GLYPH_TREE_SPACE, SPECIAL_GLYPH_TREE_TOP, + SPECIAL_GLYPH_VERTICAL_DOTTED, SPECIAL_GLYPH_TRIANGULAR_BULLET, SPECIAL_GLYPH_BLACK_CIRCLE, SPECIAL_GLYPH_WHITE_CIRCLE, diff --git a/src/test/test-locale-util.c b/src/test/test-locale-util.c index 6ec3f7f00b..e58e1638e7 100644 --- a/src/test/test-locale-util.c +++ b/src/test/test-locale-util.c @@ -92,6 +92,7 @@ TEST(dump_special_glyphs) { dump_glyph(SPECIAL_GLYPH_TREE_RIGHT); dump_glyph(SPECIAL_GLYPH_TREE_SPACE); dump_glyph(SPECIAL_GLYPH_TREE_TOP); + dump_glyph(SPECIAL_GLYPH_VERTICAL_DOTTED); dump_glyph(SPECIAL_GLYPH_TRIANGULAR_BULLET); dump_glyph(SPECIAL_GLYPH_BLACK_CIRCLE); dump_glyph(SPECIAL_GLYPH_WHITE_CIRCLE); diff --git a/src/udev/udevadm-info.c b/src/udev/udevadm-info.c index 12a1769bd8..fc5f0bbae4 100644 --- a/src/udev/udevadm-info.c +++ b/src/udev/udevadm-info.c @@ -18,6 +18,8 @@ #include "dirent-util.h" #include "errno-util.h" #include "fd-util.h" +#include "glyph-util.h" +#include "pager.h" #include "sort-util.h" #include "static-destruct.h" #include "string-table.h" @@ -31,6 +33,7 @@ typedef enum ActionType { ACTION_QUERY, ACTION_ATTRIBUTE_WALK, ACTION_DEVICE_ID_FILE, + ACTION_TREE, } ActionType; typedef enum QueryType { @@ -48,6 +51,9 @@ static bool arg_value = false; static const char *arg_export_prefix = NULL; static usec_t arg_wait_for_initialization_timeout = 0; +/* Put a limit on --tree descent level to not exhaust our stack */ +#define TREE_DEPTH_MAX 64 + static bool skip_attribute(const char *name) { assert(name); @@ -171,7 +177,7 @@ static int print_device_chain(sd_device *device) { return 0; } -static int print_record(sd_device *device) { +static int print_record(sd_device *device, const char *prefix) { const char *str, *val, *subsys; dev_t devnum; uint64_t q; @@ -179,6 +185,8 @@ static int print_record(sd_device *device) { assert(device); + prefix = strempty(prefix); + /* We don't show syspath here, because it's identical to devpath (modulo the "/sys" prefix). * * We don't show action/seqnum here because that only makes sense for records synthesized from @@ -197,52 +205,54 @@ static int print_record(sd_device *device) { * • no color for regular properties */ assert_se(sd_device_get_devpath(device, &str) >= 0); - printf("P: %s%s%s\n", ansi_highlight_white(), str, ansi_normal()); + printf("%sP: %s%s%s\n", prefix, ansi_highlight_white(), str, ansi_normal()); if (sd_device_get_sysname(device, &str) >= 0) - printf("M: %s%s%s\n", ansi_highlight_white(), str, ansi_normal()); + printf("%sM: %s%s%s\n", prefix, ansi_highlight_white(), str, ansi_normal()); if (sd_device_get_sysnum(device, &str) >= 0) - printf("R: %s%s%s\n", ansi_highlight_white(), str, ansi_normal()); + printf("%sR: %s%s%s\n", prefix, ansi_highlight_white(), str, ansi_normal()); if (sd_device_get_subsystem(device, &subsys) >= 0) - printf("U: %s%s%s\n", ansi_highlight_green(), subsys, ansi_normal()); + printf("%sU: %s%s%s\n", prefix, ansi_highlight_green(), subsys, ansi_normal()); if (sd_device_get_devtype(device, &str) >= 0) - printf("T: %s%s%s\n", ansi_highlight_green(), str, ansi_normal()); + printf("%sT: %s%s%s\n", prefix, ansi_highlight_green(), str, ansi_normal()); if (sd_device_get_devnum(device, &devnum) >= 0) - printf("D: %s%c %u:%u%s\n", + printf("%sD: %s%c %u:%u%s\n", + prefix, ansi_highlight_cyan(), streq_ptr(subsys, "block") ? 'b' : 'c', major(devnum), minor(devnum), ansi_normal()); if (sd_device_get_ifindex(device, &ifi) >= 0) - printf("I: %s%i%s\n", ansi_highlight_cyan(), ifi, ansi_normal()); + printf("%sI: %s%i%s\n", prefix, ansi_highlight_cyan(), ifi, ansi_normal()); if (sd_device_get_devname(device, &str) >= 0) { assert_se(val = path_startswith(str, "/dev/")); - printf("N: %s%s%s\n", ansi_highlight_cyan(), val, ansi_normal()); + printf("%sN: %s%s%s\n", prefix, ansi_highlight_cyan(), val, ansi_normal()); if (device_get_devlink_priority(device, &i) >= 0) - printf("L: %s%i%s\n", ansi_highlight_cyan(), i, ansi_normal()); + printf("%sL: %s%i%s\n", prefix, ansi_highlight_cyan(), i, ansi_normal()); FOREACH_DEVICE_DEVLINK(device, str) { assert_se(val = path_startswith(str, "/dev/")); - printf("S: %s%s%s\n", ansi_highlight_cyan(), val, ansi_normal()); + printf("%sS: %s%s%s\n", prefix, ansi_highlight_cyan(), val, ansi_normal()); } } if (sd_device_get_diskseq(device, &q) >= 0) - printf("Q: %s%" PRIu64 "%s\n", ansi_highlight_magenta(), q, ansi_normal()); + printf("%sQ: %s%" PRIu64 "%s\n", prefix, ansi_highlight_magenta(), q, ansi_normal()); if (sd_device_get_driver(device, &str) >= 0) - printf("V: %s%s%s\n", ansi_highlight_yellow4(), str, ansi_normal()); + printf("%sV: %s%s%s\n", prefix, ansi_highlight_yellow4(), str, ansi_normal()); FOREACH_DEVICE_PROPERTY(device, str, val) - printf("E: %s=%s\n", str, val); + printf("%sE: %s=%s\n", prefix, str, val); - puts(""); + if (isempty(prefix)) + puts(""); return 0; } @@ -284,7 +294,7 @@ static int export_devices(void) { return log_error_errno(r, "Failed to scan devices: %m"); FOREACH_DEVICE_AND_SUBSYSTEM(e, d) - (void) print_record(d); + (void) print_record(d, NULL); return 0; } @@ -449,7 +459,7 @@ static int query_device(QueryType query, sd_device* device) { } case QUERY_ALL: - return print_record(device); + return print_record(device, NULL); default: assert_not_reached(); @@ -474,6 +484,7 @@ static int help(void) { " -r --root Prepend dev directory to path names\n" " -a --attribute-walk Print all key matches walking along the chain\n" " of parent devices\n" + " -t --tree Show tree of devices\n" " -d --device-id-of-file=FILE Print major:minor of device containing this file\n" " -x --export Export key/value pairs\n" " -P --export-prefix Export the key name with a prefix\n" @@ -486,6 +497,156 @@ static int help(void) { return 0; } +static int draw_tree( + sd_device *parent, + sd_device *const array[], size_t n, + const char *prefix, + unsigned level); + +static int output_tree_device( + sd_device *device, + const char *str, + const char *prefix, + bool more, + sd_device *const array[], size_t n, + unsigned level) { + + _cleanup_free_ char *subprefix = NULL, *subsubprefix = NULL; + + assert(device); + assert(str); + + prefix = strempty(prefix); + + printf("%s%s%s\n", prefix, special_glyph(more ? SPECIAL_GLYPH_TREE_BRANCH : SPECIAL_GLYPH_TREE_RIGHT), str); + + subprefix = strjoin(prefix, special_glyph(more ? SPECIAL_GLYPH_TREE_VERTICAL : SPECIAL_GLYPH_TREE_SPACE)); + if (!subprefix) + return log_oom(); + + subsubprefix = strjoin(subprefix, special_glyph(SPECIAL_GLYPH_VERTICAL_DOTTED), " "); + if (!subsubprefix) + return log_oom(); + + (void) print_record(device, subsubprefix); + + return draw_tree(device, array, n, subprefix, level + 1); +} + +static int draw_tree( + sd_device *parent, + sd_device *const array[], size_t n, + const char *prefix, + unsigned level) { + + const char *parent_path; + size_t i = 0; + int r; + + if (n == 0) + return 0; + + assert(array); + + if (parent) { + r = sd_device_get_devpath(parent, &parent_path); + if (r < 0) + return log_error_errno(r, "Failed to get sysfs path of parent device: %m"); + } else + parent_path = NULL; + + if (level > TREE_DEPTH_MAX) { + log_warning("Eliding tree below '%s', too deep.", strna(parent_path)); + return 0; + } + + while (i < n) { + sd_device *device = array[i]; + const char *device_path, *str; + bool more = false; + size_t j; + + r = sd_device_get_devpath(device, &device_path); + if (r < 0) + return log_error_errno(r, "Failed to get sysfs path of enumerated device: %m"); + + /* Scan through the subsequent devices looking children of the device we are looking at. */ + for (j = i + 1; j < n; j++) { + sd_device *next = array[j]; + const char *next_path; + + r = sd_device_get_devpath(next, &next_path); + if (r < 0) + return log_error_errno(r, "Failed to get sysfs of child device: %m"); + + if (!path_startswith(next_path, device_path)) { + more = !parent_path || path_startswith(next_path, parent_path); + break; + } + } + + /* Determine the string to display for this node. If we are at the top of the tree, the full + * device path so far, otherwise just the part suffixing the parent's device path. */ + str = parent ? ASSERT_PTR(path_startswith(device_path, parent_path)) : device_path; + + r = output_tree_device(device, str, prefix, more, array + i + 1, j - i - 1, level); + if (r < 0) + return r; + + i = j; + } + + return 0; +} + +static int print_tree(sd_device* below) { + _cleanup_(sd_device_enumerator_unrefp) sd_device_enumerator *e = NULL; + const char *below_path; + sd_device **array; + size_t n = 0; + int r; + + if (below) { + r = sd_device_get_devpath(below, &below_path); + if (r < 0) + return log_error_errno(r, "Failed to get sysfs path of device: %m"); + + } else + below_path = NULL; + + r = sd_device_enumerator_new(&e); + if (r < 0) + return log_error_errno(r, "Failed to allocate device enumerator: %m"); + + if (below) { + r = sd_device_enumerator_add_match_parent(e, below); + if (r < 0) + return log_error_errno(r, "Failed to install parent enumerator match: %m"); + } + + r = sd_device_enumerator_allow_uninitialized(e); + if (r < 0) + return log_error_errno(r, "Failed to enable enumeration of uninitialized devices: %m"); + + r = device_enumerator_scan_devices_and_subsystems(e); + if (r < 0) + return log_error_errno(r, "Failed to scan for devices and subsystems: %m"); + + assert_se(array = device_enumerator_get_devices(e, &n)); + + if (n == 0) { + log_info("No items."); + return 0; + } + + r = draw_tree(NULL, array, n, NULL, 0); + if (r < 0) + return r; + + printf("\n%zu items shown.\n", n); + return 0; +} + int info_main(int argc, char *argv[], void *userdata) { _cleanup_strv_free_ char **devices = NULL; _cleanup_free_ char *name = NULL; @@ -498,6 +659,7 @@ int info_main(int argc, char *argv[], void *userdata) { static const struct option options[] = { { "attribute-walk", no_argument, NULL, 'a' }, + { "tree", no_argument, NULL, 't' }, { "cleanup-db", no_argument, NULL, 'c' }, { "device-id-of-file", required_argument, NULL, 'd' }, { "export", no_argument, NULL, 'x' }, @@ -518,7 +680,7 @@ int info_main(int argc, char *argv[], void *userdata) { ActionType action = ACTION_QUERY; QueryType query = QUERY_ALL; - while ((c = getopt_long(argc, argv, "aced:n:p:q:rxP:w::Vh", options, NULL)) >= 0) + while ((c = getopt_long(argc, argv, "atced:n:p:q:rxP:w::Vh", options, NULL)) >= 0) switch (c) { case ARG_PROPERTY: /* Make sure that if the empty property list was specified, we won't show any @@ -578,6 +740,9 @@ int info_main(int argc, char *argv[], void *userdata) { case 'a': action = ACTION_ATTRIBUTE_WALK; break; + case 't': + action = ACTION_TREE; + break; case 'e': return export_devices(); case 'c': @@ -620,17 +785,23 @@ int info_main(int argc, char *argv[], void *userdata) { if (r < 0) return log_error_errno(r, "Failed to build argument list: %m"); - if (strv_isempty(devices)) + if (action != ACTION_TREE && strv_isempty(devices)) return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "A device name or path is required"); - if (action == ACTION_ATTRIBUTE_WALK && strv_length(devices) > 1) + if (IN_SET(action, ACTION_ATTRIBUTE_WALK, ACTION_TREE) && strv_length(devices) > 1) return log_error_errno(SYNTHETIC_ERRNO(EINVAL), - "Only one device may be specified with -a/--attribute-walk"); + "Only one device may be specified with -a/--attribute-walk and -t/--tree"); if (arg_export && arg_value) return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "-x/--export or -P/--export-prefix cannot be used with --value"); + if (strv_isempty(devices)) { + assert(action == ACTION_TREE); + pager_open(0); + return print_tree(NULL); + } + ret = 0; STRV_FOREACH(p, devices) { _cleanup_(sd_device_unrefp) sd_device *device = NULL; @@ -666,6 +837,8 @@ int info_main(int argc, char *argv[], void *userdata) { r = query_device(query, device); else if (action == ACTION_ATTRIBUTE_WALK) r = print_device_chain(device); + else if (action == ACTION_TREE) + r = print_tree(device); else assert_not_reached(); if (r < 0) From 7b6d16837634eccfab6855af2be593d63f4a644b Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 12 Apr 2022 18:30:49 +0200 Subject: [PATCH 10/11] udevadm: use xopendirat() where appropriate And while we are at it, let's use more appropriate open flags. O_NONBLOCk is pointless in combination with O_NOFOLLOW. O_NOFOLLOW makes a ton of sense otoh, since the inode is supposed to be a dir, we just checked. THe other flags are implied by xopendirat() --- src/udev/udevadm-info.c | 21 +++++++++++++-------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/src/udev/udevadm-info.c b/src/udev/udevadm-info.c index fc5f0bbae4..4ee15592cb 100644 --- a/src/udev/udevadm-info.c +++ b/src/udev/udevadm-info.c @@ -18,6 +18,7 @@ #include "dirent-util.h" #include "errno-util.h" #include "fd-util.h" +#include "fileio.h" #include "glyph-util.h" #include "pager.h" #include "sort-util.h" @@ -315,11 +316,13 @@ static void cleanup_dir(DIR *dir, mode_t mask, int depth) { if ((stats.st_mode & mask) != 0) continue; if (S_ISDIR(stats.st_mode)) { - _cleanup_closedir_ DIR *dir2 = NULL; + _cleanup_closedir_ DIR *subdir = NULL; - dir2 = fdopendir(openat(dirfd(dir), dent->d_name, O_RDONLY|O_NONBLOCK|O_DIRECTORY|O_CLOEXEC)); - if (dir2) - cleanup_dir(dir2, mask, depth-1); + subdir = xopendirat(dirfd(dir), dent->d_name, O_NOFOLLOW); + if (!subdir) + log_debug_errno(errno, "Failed to open subdirectory '%s', ignoring: %m", dent->d_name); + else + cleanup_dir(subdir, mask, depth-1); (void) unlinkat(dirfd(dir), dent->d_name, AT_REMOVEDIR); } else @@ -362,11 +365,13 @@ static void cleanup_dirs_after_db_cleanup(DIR *dir, DIR *datadir) { if (fstatat(dirfd(dir), dent->d_name, &stats, AT_SYMLINK_NOFOLLOW) < 0) continue; if (S_ISDIR(stats.st_mode)) { - _cleanup_closedir_ DIR *dir2 = NULL; + _cleanup_closedir_ DIR *subdir = NULL; - dir2 = fdopendir(openat(dirfd(dir), dent->d_name, O_RDONLY|O_NONBLOCK|O_DIRECTORY|O_CLOEXEC)); - if (dir2) - cleanup_dir_after_db_cleanup(dir2, datadir); + subdir = xopendirat(dirfd(dir), dent->d_name, O_NOFOLLOW); + if (!subdir) + log_debug_errno(errno, "Failed to open subdirectory '%s', ignoring: %m", dent->d_name); + else + cleanup_dir_after_db_cleanup(subdir, datadir); (void) unlinkat(dirfd(dir), dent->d_name, AT_REMOVEDIR); } else From 2f048ad0fe9d7ee6e492e192b11c2c618728bbe6 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 5 Apr 2022 10:21:14 +0200 Subject: [PATCH 11/11] update TODO --- TODO | 3 --- 1 file changed, 3 deletions(-) diff --git a/TODO b/TODO index 9099c2e626..678146c3e9 100644 --- a/TODO +++ b/TODO @@ -164,9 +164,6 @@ Features: sd_device object, so that data passed into sd_device_new_from_devnum() can also be queried. -* udevadm: a new "tree" verb that shows tree of devices as syspath hierarchy, - along with their properties. uninitialized devices should be greyed out. - * bootctl: show whether UEFI audit mode is available * sd-event: optionally, if per-event source rate limit is hit, downgrade