From 25151ca2b8ec091f046ab4d4328e5b8d767b6eed Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Tue, 13 May 2025 23:59:50 +0900 Subject: [PATCH 1/9] backlight: use device_get_sysattr_unsigned() at one more place --- src/backlight/backlight.c | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/src/backlight/backlight.c b/src/backlight/backlight.c index a39398a638..665d0c17e4 100644 --- a/src/backlight/backlight.c +++ b/src/backlight/backlight.c @@ -8,6 +8,7 @@ #include "alloc-util.h" #include "argv-util.h" +#include "device-private.h" #include "device-util.h" #include "escape.h" #include "fileio.h" @@ -298,19 +299,14 @@ static int validate_device(sd_device *device) { static int read_max_brightness(sd_device *device, unsigned *ret) { unsigned max_brightness; - const char *s; int r; assert(device); assert(ret); - r = sd_device_get_sysattr_value(device, "max_brightness", &s); + r = device_get_sysattr_unsigned(device, "max_brightness", &max_brightness); if (r < 0) - return log_device_warning_errno(device, r, "Failed to read 'max_brightness' attribute: %m"); - - r = safe_atou(s, &max_brightness); - if (r < 0) - return log_device_warning_errno(device, r, "Failed to parse 'max_brightness' \"%s\": %m", s); + return log_device_warning_errno(device, r, "Failed to read/parse 'max_brightness' attribute: %m"); /* If max_brightness is 0, then there is no actual backlight device. This happens on desktops * with Asus mainboards that load the eeepc-wmi module. */ From b7d60b2966c538f582f7fadb679756445c7d85bf Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Wed, 14 May 2025 00:02:07 +0900 Subject: [PATCH 2/9] backlight: replace recursion with for loop --- src/backlight/backlight.c | 69 +++++++++++++++++++++------------------ 1 file changed, 37 insertions(+), 32 deletions(-) diff --git a/src/backlight/backlight.c b/src/backlight/backlight.c index 665d0c17e4..e43ed74ae2 100644 --- a/src/backlight/backlight.c +++ b/src/backlight/backlight.c @@ -93,53 +93,58 @@ static int has_multiple_graphics_cards(void) { } static int find_pci_or_platform_parent(sd_device *device, sd_device **ret) { - sd_device *parent; - const char *s; int r; assert(device); assert(ret); - r = sd_device_get_parent(device, &parent); - if (r < 0) - return r; - - if (device_in_subsystem(parent, "drm")) { - - r = sd_device_get_sysname(parent, &s); + for (;;) { + r = sd_device_get_parent(device, &device); if (r < 0) return r; - s = startswith(s, "card"); - if (!s) - return -ENODATA; + if (device_in_subsystem(device, "drm")) { + const char *s; - s += strspn(s, DIGITS); - if (*s == '-' && !STARTSWITH_SET(s, "-LVDS-", "-Embedded DisplayPort-", "-eDP-")) - /* A connector DRM device, let's ignore all but LVDS and eDP! */ - return -EOPNOTSUPP; + r = sd_device_get_sysname(device, &s); + if (r < 0) + return r; - } else if (device_in_subsystem(parent, "pci") && - sd_device_get_sysattr_value(parent, "class", &s)) { + s = startswith(s, "card"); + if (!s) + return -ENODATA; - unsigned long class; + s += strspn(s, DIGITS); + if (*s == '-' && !STARTSWITH_SET(s, "-LVDS-", "-Embedded DisplayPort-", "-eDP-")) + /* A connector DRM device, let's ignore all but LVDS and eDP! */ + return -EOPNOTSUPP; - r = safe_atolu(s, &class); - if (r < 0) - return log_device_warning_errno(parent, r, "Cannot parse PCI class '%s': %m", s); - - /* Graphics card */ - if (class == PCI_CLASS_GRAPHICS_CARD) { - *ret = parent; - return 0; + continue; } - } else if (device_in_subsystem(parent, "platform")) { - *ret = parent; - return 0; - } + if (device_in_subsystem(device, "pci")) { + uint32_t class; - return find_pci_or_platform_parent(parent, ret); + r = device_get_sysattr_u32(device, "class", &class); + if (r == -ENOENT) + continue; + if (r < 0) + return r; + + /* Graphics card */ + if (class == PCI_CLASS_GRAPHICS_CARD) { + *ret = device; + return 0; + } + + continue; + } + + if (device_in_subsystem(device, "platform")) { + *ret = device; + return 0; + } + } } static int same_device(sd_device *a, sd_device *b) { From ab1bd9daede454ac7cd5e5987bd345e7169894f8 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Thu, 27 Feb 2025 13:19:03 +0900 Subject: [PATCH 3/9] device-util: introduce several more helper functions This also makes device_in_subsystem() and device_is_devtype() return negative error on critical error --- src/analyze/analyze-chid.c | 5 +- src/backlight/backlight.c | 48 ++-- src/dissect/dissect.c | 7 +- src/libsystemd/sd-device/device-enumerator.c | 5 +- src/libsystemd/sd-device/device-monitor.c | 11 +- src/libsystemd/sd-device/device-private.c | 5 +- src/libsystemd/sd-device/device-util.c | 55 ++++- src/libsystemd/sd-device/device-util.h | 13 +- src/libsystemd/sd-device/sd-device.c | 51 ++-- src/libsystemd/sd-device/test-device-util.c | 238 ++++++++++++++----- src/libsystemd/sd-device/test-sd-device.c | 7 +- src/login/logind-core.c | 31 ++- src/login/logind-session-device.c | 23 +- src/login/logind.c | 14 +- src/mount/mount-tool.c | 11 +- src/network/networkctl-link-info.c | 2 +- src/network/networkd-manager.c | 6 +- src/network/networkd-wiphy.c | 5 +- src/shared/blockdev-list.c | 9 +- src/shared/blockdev-util.c | 12 +- src/udev/test-udev-rule-runner.c | 5 +- src/udev/udev-builtin-blkid.c | 6 +- src/udev/udev-builtin-dissect_image.c | 18 +- src/udev/udev-builtin-hwdb.c | 14 +- src/udev/udev-builtin-input_id.c | 8 +- src/udev/udev-builtin-net_id.c | 62 ++--- src/udev/udev-builtin-path_id.c | 48 ++-- src/udev/udev-node.c | 6 +- src/udev/udev-watch.c | 10 +- src/udev/udev-worker.c | 35 +-- 30 files changed, 533 insertions(+), 237 deletions(-) diff --git a/src/analyze/analyze-chid.c b/src/analyze/analyze-chid.c index 6d2c74345f..e691a7d9a5 100644 --- a/src/analyze/analyze-chid.c +++ b/src/analyze/analyze-chid.c @@ -361,7 +361,10 @@ int verb_chid(int argc, char *argv[], void *userdata) { if (r < 0) return log_error_errno(r, "Failed to open device %s: %m", arg_drm_device_path); - if (!device_in_subsystem(drm_dev, "drm")) + r = device_in_subsystem(drm_dev, "drm"); + if (r < 0) + return log_error_errno(r, "Failed to check if the device is a DRM device: %m"); + if (r == 0) return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "Cannot read EDID from a non-DRM device '%s'", arg_drm_device_path); r = edid_parse(drm_dev, &smbios_fields[CHID_EDID_PANEL]); diff --git a/src/backlight/backlight.c b/src/backlight/backlight.c index e43ed74ae2..0fa5209305 100644 --- a/src/backlight/backlight.c +++ b/src/backlight/backlight.c @@ -103,15 +103,16 @@ static int find_pci_or_platform_parent(sd_device *device, sd_device **ret) { if (r < 0) return r; - if (device_in_subsystem(device, "drm")) { + r = device_in_subsystem(device, "drm"); + if (r < 0) + return r; + if (r > 0) { const char *s; - r = sd_device_get_sysname(device, &s); + r = device_sysname_startswith_strv(device, STRV_MAKE("card"), &s); if (r < 0) return r; - - s = startswith(s, "card"); - if (!s) + if (r == 0) return -ENODATA; s += strspn(s, DIGITS); @@ -122,7 +123,10 @@ static int find_pci_or_platform_parent(sd_device *device, sd_device **ret) { continue; } - if (device_in_subsystem(device, "pci")) { + r = device_in_subsystem(device, "pci"); + if (r < 0) + return r; + if (r > 0) { uint32_t class; r = device_get_sysattr_u32(device, "class", &class); @@ -140,7 +144,10 @@ static int find_pci_or_platform_parent(sd_device *device, sd_device **ret) { continue; } - if (device_in_subsystem(device, "platform")) { + r = device_in_subsystem(device, "platform"); + if (r < 0) + return r; + if (r > 0) { *ret = device; return 0; } @@ -197,7 +204,10 @@ static int validate_device(sd_device *device) { if (r < 0) return log_device_debug_errno(device, r, "Failed to get sysname: %m"); - if (!device_in_subsystem(device, "backlight")) + r = device_in_subsystem(device, "leds"); + if (r < 0) + return log_device_debug_errno(device, r, "Failed to check if device is in backlight subsystem: %m"); + if (r > 0) return true; /* We assume LED device is always valid. */ r = sd_device_get_sysattr_value(device, "type", &v); @@ -242,7 +252,7 @@ static int validate_device(sd_device *device) { if (r < 0) return log_debug_errno(r, "Failed to add sysattr match: %m"); - if (device_in_subsystem(parent, "pci")) { + if (device_in_subsystem(parent, "pci") > 0) { r = has_multiple_graphics_cards(); if (r < 0) return log_debug_errno(r, "Failed to check if the system has multiple graphics cards: %m"); @@ -284,7 +294,7 @@ static int validate_device(sd_device *device) { return false; } - if (device_in_subsystem(other_parent, "platform") && device_in_subsystem(parent, "pci")) { + if (device_in_subsystem(other_parent, "platform") > 0 && device_in_subsystem(parent, "pci") > 0) { /* The other is connected to the platform bus and we are a PCI device, that also means we are out. */ if (DEBUG_LOGGING) { const char *other_sysname = NULL, *other_type = NULL; @@ -335,6 +345,7 @@ static int clamp_brightness( unsigned *brightness) { unsigned new_brightness, min_brightness; + int r; assert(device); assert(brightness); @@ -345,7 +356,10 @@ static int clamp_brightness( * state restoration. */ min_brightness = (unsigned) ((double) max_brightness * percent / 100); - if (device_in_subsystem(device, "backlight")) + r = device_in_subsystem(device, "backlight"); + if (r < 0) + return r; + if (r > 0) min_brightness = MAX(1U, min_brightness); new_brightness = CLAMP(*brightness, min_brightness, max_brightness); @@ -361,7 +375,7 @@ static int clamp_brightness( return 0; } -static bool shall_clamp(sd_device *device, unsigned *ret) { +static int shall_clamp(sd_device *device, unsigned *ret) { const char *property, *s; unsigned default_percent; int r; @@ -369,7 +383,10 @@ static bool shall_clamp(sd_device *device, unsigned *ret) { assert(device); assert(ret); - if (device_in_subsystem(device, "backlight")) { + r = device_in_subsystem(device, "backlight"); + if (r < 0) + return r; + if (r > 0) { property = "ID_BACKLIGHT_CLAMP"; default_percent = 5; } else { @@ -561,7 +578,10 @@ static int verb_load(int argc, char *argv[], void *userdata) { if (validate_device(device) == 0) return 0; - clamp = shall_clamp(device, &percent); + r = shall_clamp(device, &percent); + if (r < 0) + return r; + clamp = r; r = read_saved_brightness(device, &brightness); if (r < 0) { diff --git a/src/dissect/dissect.c b/src/dissect/dissect.c index 09cc90cf02..5db462f4ff 100644 --- a/src/dissect/dissect.c +++ b/src/dissect/dissect.c @@ -2084,8 +2084,13 @@ static int action_detach(const char *path) { FOREACH_DEVICE(e, d) { _cleanup_(loop_device_unrefp) LoopDevice *entry_loop = NULL; - if (!device_is_devtype(d, "disk")) /* Filter out partition block devices */ + r = device_is_devtype(d, "disk"); + if (r < 0) { + log_device_warning_errno(d, r, "Failed to check if device is a whole disk, skipping: %m"); continue; + } + if (r == 0) + continue; /* Filter out partition block devices */ r = loop_device_open(d, O_RDONLY, LOCK_SH, &entry_loop); if (r < 0) { diff --git a/src/libsystemd/sd-device/device-enumerator.c b/src/libsystemd/sd-device/device-enumerator.c index d2faff4ae3..8230763ddd 100644 --- a/src/libsystemd/sd-device/device-enumerator.c +++ b/src/libsystemd/sd-device/device-enumerator.c @@ -394,7 +394,10 @@ static int enumerator_sort_devices(sd_device_enumerator *enumerator) { HASHMAP_FOREACH_KEY(device, syspath, enumerator->devices_by_syspath) { _cleanup_free_ char *p = NULL; - if (!device_in_subsystem(device, *prioritized_subsystem)) + r = device_in_subsystem(device, *prioritized_subsystem); + if (r < 0) + return r; + if (r == 0) continue; devices[n++] = sd_device_ref(device); diff --git a/src/libsystemd/sd-device/device-monitor.c b/src/libsystemd/sd-device/device-monitor.c index 992f531be2..99bdc7f342 100644 --- a/src/libsystemd/sd-device/device-monitor.c +++ b/src/libsystemd/sd-device/device-monitor.c @@ -471,6 +471,7 @@ DEFINE_PUBLIC_TRIVIAL_REF_UNREF_FUNC(sd_device_monitor, sd_device_monitor, devic static int check_subsystem_filter(sd_device_monitor *m, sd_device *device) { const char *s, *d; + int r; assert(m); assert(device); @@ -479,13 +480,9 @@ static int check_subsystem_filter(sd_device_monitor *m, sd_device *device) { return true; HASHMAP_FOREACH_KEY(d, s, m->subsystem_filter) { - if (!device_in_subsystem(device, s)) - continue; - - if (d && !device_is_devtype(device, d)) - continue; - - return true; + r = device_is_subsystem_devtype(device, s, d); + if (r != 0) + return r; } return false; diff --git a/src/libsystemd/sd-device/device-private.c b/src/libsystemd/sd-device/device-private.c index 3ce87690b3..270e0293e1 100644 --- a/src/libsystemd/sd-device/device-private.c +++ b/src/libsystemd/sd-device/device-private.c @@ -441,7 +441,10 @@ static int device_verify(sd_device *device) { return log_device_debug_errno(device, SYNTHETIC_ERRNO(EINVAL), "sd-device: Device created from strv or nulstr lacks devpath, subsystem, action or seqnum."); - if (device_in_subsystem(device, "drivers")) { + r = device_in_subsystem(device, "drivers"); + if (r < 0) + return log_device_debug_errno(device, r, "sd-device: Failed to check if the device is a driver: %m"); + if (r > 0) { r = device_set_drivers_subsystem(device); if (r < 0) return log_device_debug_errno(device, r, diff --git a/src/libsystemd/sd-device/device-util.c b/src/libsystemd/sd-device/device-util.c index b0475c5ca3..67bcbf8af5 100644 --- a/src/libsystemd/sd-device/device-util.c +++ b/src/libsystemd/sd-device/device-util.c @@ -132,24 +132,65 @@ char** device_make_log_fields(sd_device *device) { return TAKE_PTR(strv); } -bool device_in_subsystem(sd_device *device, const char *subsystem) { - const char *s = NULL; +int device_in_subsystem_strv(sd_device *device, char * const *subsystems) { + const char *s; + int r; assert(device); - (void) sd_device_get_subsystem(device, &s); - return streq_ptr(s, subsystem); + r = sd_device_get_subsystem(device, &s); + if (r == -ENOENT) + return strv_isempty(subsystems); + if (r < 0) + return r; + return strv_contains(subsystems, s); } -bool device_is_devtype(sd_device *device, const char *devtype) { - const char *s = NULL; +int device_is_devtype(sd_device *device, const char *devtype) { + const char *s; + int r; assert(device); - (void) sd_device_get_devtype(device, &s); + r = sd_device_get_devtype(device, &s); + if (r == -ENOENT) + return !devtype; + if (r < 0) + return r; return streq_ptr(s, devtype); } +int device_is_subsystem_devtype(sd_device *device, const char *subsystem, const char *devtype) { + int r; + + assert(device); + + r = device_in_subsystem(device, subsystem); + if (r <= 0) + return r; + + if (!devtype) + return true; + + return device_is_devtype(device, devtype); +} + +int device_sysname_startswith_strv(sd_device *device, char * const *prefixes, const char **ret_suffix) { + const char *sysname; + int r; + + assert(device); + + r = sd_device_get_sysname(device, &sysname); + if (r < 0) + return r; + + const char *suffix = startswith_strv(sysname, prefixes); + if (ret_suffix) + *ret_suffix = suffix; + return !!suffix; +} + bool device_property_can_set(const char *property) { return property && !STR_IN_SET(property, diff --git a/src/libsystemd/sd-device/device-util.h b/src/libsystemd/sd-device/device-util.h index 0a740ede32..008946f4de 100644 --- a/src/libsystemd/sd-device/device-util.h +++ b/src/libsystemd/sd-device/device-util.h @@ -103,7 +103,16 @@ int device_open_from_devnum(mode_t mode, dev_t devnum, int flags, char **ret_dev char** device_make_log_fields(sd_device *device); -bool device_in_subsystem(sd_device *device, const char *subsystem); -bool device_is_devtype(sd_device *device, const char *devtype); +int device_in_subsystem_strv(sd_device *device, char * const *subsystems); +#define device_in_subsystem(device, ...) \ + device_in_subsystem_strv(device, STRV_MAKE(__VA_ARGS__)) + +int device_is_devtype(sd_device *device, const char *devtype); + +int device_is_subsystem_devtype(sd_device *device, const char *subsystem, const char *devtype); + +int device_sysname_startswith_strv(sd_device *device, char * const *prefixes, const char **ret_suffix); +#define device_sysname_startswith(device, ...) \ + device_sysname_startswith_strv(device, STRV_MAKE(__VA_ARGS__), NULL) bool device_property_can_set(const char *property) _pure_; diff --git a/src/libsystemd/sd-device/sd-device.c b/src/libsystemd/sd-device/sd-device.c index 12763f53b5..37de2b9e60 100644 --- a/src/libsystemd/sd-device/sd-device.c +++ b/src/libsystemd/sd-device/sd-device.c @@ -314,7 +314,10 @@ int device_new_from_mode_and_devnum(sd_device **ret, mode_t mode, dev_t devnum) if (n != devnum) return -ENXIO; - if (device_in_subsystem(dev, "block") != !!S_ISBLK(mode)) + r = device_in_subsystem(dev, "block"); + if (r < 0) + return r; + if (r > 0 ? !S_ISBLK(mode) : !S_ISCHR(mode)) return -ENXIO; *ret = TAKE_PTR(dev); @@ -414,8 +417,9 @@ static int device_new_from_path_join( 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; + r = device_in_subsystem(new_device, subsystem); + if (r <= 0) + return r; const char *new_driver_subsystem = NULL; (void) sd_device_get_driver_subsystem(new_device, &new_driver_subsystem); @@ -843,7 +847,10 @@ int device_read_uevent_file(sd_device *device) { major, strna(minor)); } - if (device_in_subsystem(device, "drivers")) { + r = device_in_subsystem(device, "drivers"); + if (r < 0) + log_device_debug_errno(device, r, "Failed to check if the device is a driver, ignoring: %m"); + if (r > 0) { r = device_set_drivers_subsystem(device); if (r < 0) log_device_debug_errno(device, r, @@ -1245,9 +1252,14 @@ _public_ int sd_device_get_subsystem(sd_device *device, const char **ret) { } _public_ int sd_device_get_driver_subsystem(sd_device *device, const char **ret) { + int r; + assert_return(device, -EINVAL); - if (!device_in_subsystem(device, "drivers")) + r = device_in_subsystem(device, "drivers"); + if (r < 0) + return r; + if (r == 0) return -ENOENT; assert(device->driver_subsystem); @@ -1287,10 +1299,10 @@ _public_ int sd_device_get_parent_with_subsystem_devtype(sd_device *device, cons if (r < 0) return r; - if (!device_in_subsystem(device, subsystem)) - continue; - - if (devtype && !device_is_devtype(device, devtype)) + r = device_is_subsystem_devtype(device, subsystem, devtype); + if (r < 0) + return r; + if (r == 0) continue; if (ret) @@ -1710,15 +1722,20 @@ _public_ int sd_device_get_device_id(sd_device *device, const char **ret) { int ifindex, r; if (sd_device_get_devnum(device, &devnum) >= 0) { + r = device_in_subsystem(device, "block"); + if (r < 0) + return r; + char t = r > 0 ? 'b' : 'c'; + /* use dev_t — b259:131072, c254:0 */ - if (asprintf(&id, "%c" DEVNUM_FORMAT_STR, - device_in_subsystem(device, "block") ? 'b' : 'c', - DEVNUM_FORMAT_VAL(devnum)) < 0) + if (asprintf(&id, "%c" DEVNUM_FORMAT_STR, t, DEVNUM_FORMAT_VAL(devnum)) < 0) return -ENOMEM; + } else if (sd_device_get_ifindex(device, &ifindex) >= 0) { /* use netdev ifindex — n3 */ if (asprintf(&id, "n%u", (unsigned) ifindex) < 0) return -ENOMEM; + } else { _cleanup_free_ char *sysname = NULL; @@ -1730,7 +1747,10 @@ _public_ int sd_device_get_device_id(sd_device *device, const char **ret) { if (r == O_DIRECTORY) return -EINVAL; - if (device_in_subsystem(device, "drivers")) + r = device_in_subsystem(device, "drivers"); + if (r < 0) + return r; + if (r > 0) /* the 'drivers' pseudo-subsystem is special, and needs the real * subsystem encoded as well */ id = strjoin("+drivers:", ASSERT_PTR(device->driver_subsystem), ":", sysname); @@ -2797,7 +2817,10 @@ _public_ int sd_device_open(sd_device *device, int flags) { if (st.st_rdev != devnum) return -ENXIO; - if (device_in_subsystem(device, "block") ? !S_ISBLK(st.st_mode) : !S_ISCHR(st.st_mode)) + r = device_in_subsystem(device, "block"); + if (r < 0) + return r; + if (r > 0 ? !S_ISBLK(st.st_mode) : !S_ISCHR(st.st_mode)) return -ENXIO; /* If flags has O_PATH, then we cannot check diskseq. Let's return earlier. */ diff --git a/src/libsystemd/sd-device/test-device-util.c b/src/libsystemd/sd-device/test-device-util.c index f7c9deb45c..aa3fb8215f 100644 --- a/src/libsystemd/sd-device/test-device-util.c +++ b/src/libsystemd/sd-device/test-device-util.c @@ -6,79 +6,203 @@ TEST(log_device_full) { _cleanup_(sd_device_unrefp) sd_device *dev = NULL; - int r; (void) sd_device_new_from_subsystem_sysname(&dev, "net", "lo"); for (int level = LOG_ERR; level <= LOG_DEBUG; level++) { log_device_full(dev, level, "test level=%d: %m", level); - r = log_device_full_errno(dev, level, EUCLEAN, "test level=%d errno=EUCLEAN: %m", level); - assert_se(r == -EUCLEAN); - - r = log_device_full_errno(dev, level, 0, "test level=%d errno=0: %m", level); - assert_se(r == 0); - - r = log_device_full_errno(dev, level, SYNTHETIC_ERRNO(ENODATA), "test level=%d errno=S(ENODATA).", level); - assert_se(r == -ENODATA); + ASSERT_EQ(log_device_full_errno(dev, level, EUCLEAN, "test level=%d errno=EUCLEAN: %m", level), -EUCLEAN); + ASSERT_EQ(log_device_full_errno(dev, level, 0, "test level=%d errno=0: %m", level), 0); + ASSERT_EQ(log_device_full_errno(dev, level, SYNTHETIC_ERRNO(ENODATA), "test level=%d errno=S(ENODATA).", level), -ENODATA); } } -TEST(device_in_subsystem) { - _cleanup_(sd_device_unrefp) sd_device *dev = NULL; - int r; - - r = sd_device_new_from_subsystem_sysname(&dev, "net", "lo"); - if (r == -ENODEV) - return (void) log_tests_skipped("net/lo does not exist"); - assert_se(r >= 0); - - assert_se(device_in_subsystem(dev, "net")); - assert_se(!device_in_subsystem(dev, "disk")); - assert_se(!device_in_subsystem(dev, "subsystem")); - assert_se(!device_in_subsystem(dev, "")); - assert_se(!device_in_subsystem(dev, NULL)); - - dev = sd_device_unref(dev); - - assert_se(sd_device_new_from_syspath(&dev, "/sys/class/net") >= 0); - assert_se(!device_in_subsystem(dev, "net")); - assert_se(!device_in_subsystem(dev, "disk")); - assert_se(device_in_subsystem(dev, "subsystem")); - assert_se(!device_in_subsystem(dev, "")); - assert_se(!device_in_subsystem(dev, NULL)); - - dev = sd_device_unref(dev); - - assert_se(sd_device_new_from_syspath(&dev, "/sys/class") >= 0); - assert_se(!device_in_subsystem(dev, "net")); - assert_se(!device_in_subsystem(dev, "disk")); - assert_se(!device_in_subsystem(dev, "subsystem")); - assert_se(!device_in_subsystem(dev, "")); - assert_se(device_in_subsystem(dev, NULL)); -} - -TEST(device_is_devtype) { +TEST(device_in_subsystem_devtype_sysname_startswith) { _cleanup_(sd_device_enumerator_unrefp) sd_device_enumerator *e = NULL; - _cleanup_(sd_device_unrefp) sd_device *dev = NULL; - assert_se(sd_device_enumerator_new(&e) >= 0); - assert_se(sd_device_enumerator_add_match_subsystem(e, "disk", true) >= 0); + ASSERT_OK(sd_device_enumerator_new(&e)); + ASSERT_OK(sd_device_enumerator_allow_uninitialized(e)); + ASSERT_OK(sd_device_enumerator_add_match_subsystem(e, "block", true)); FOREACH_DEVICE(e, d) { - const char *t; + ASSERT_OK_ZERO(device_in_subsystem(d, "net")); + ASSERT_OK_POSITIVE(device_in_subsystem(d, "block")); + ASSERT_OK_ZERO(device_in_subsystem(d, "subsystem")); + ASSERT_OK_ZERO(device_in_subsystem(d, "", "net")); + ASSERT_OK_POSITIVE(device_in_subsystem(d, "net", "block")); + ASSERT_OK_POSITIVE(device_in_subsystem(d, "block", "subsystem")); + ASSERT_OK_POSITIVE(device_in_subsystem(d, "block", "")); + ASSERT_OK_ZERO(device_in_subsystem(d, "")); + ASSERT_OK_ZERO(device_in_subsystem(d, NULL)); - assert_se(sd_device_get_devtype(d, &t) >= 0); - assert_se(device_is_devtype(d, t)); - assert_se(!device_is_devtype(d, "hoge")); - assert_se(!device_is_devtype(d, "")); - assert_se(!device_is_devtype(d, NULL)); + ASSERT_OK_ZERO(device_in_subsystem_strv(d, STRV_MAKE("net"))); + ASSERT_OK_ZERO(device_in_subsystem_strv(d, STRV_MAKE("", "net"))); + ASSERT_OK_POSITIVE(device_in_subsystem_strv(d, STRV_MAKE("net", "block"))); + ASSERT_OK_POSITIVE(device_in_subsystem_strv(d, STRV_MAKE("block", "subsystem"))); + ASSERT_OK_POSITIVE(device_in_subsystem_strv(d, STRV_MAKE("block", ""))); + ASSERT_OK_ZERO(device_in_subsystem_strv(d, STRV_MAKE(""))); + ASSERT_OK_ZERO(device_in_subsystem_strv(d, STRV_MAKE(NULL))); + ASSERT_OK_ZERO(device_in_subsystem_strv(d, NULL)); + + const char *t; + ASSERT_OK(sd_device_get_devtype(d, &t)); + ASSERT_OK_POSITIVE(device_is_devtype(d, t)); + ASSERT_OK_ZERO(device_is_devtype(d, "hoge")); + ASSERT_OK_ZERO(device_is_devtype(d, "")); + ASSERT_OK_ZERO(device_is_devtype(d, NULL)); + + ASSERT_OK_POSITIVE(device_is_subsystem_devtype(d, "block", t)); + ASSERT_OK_ZERO(device_is_subsystem_devtype(d, "block", "hoge")); + ASSERT_OK_ZERO(device_is_subsystem_devtype(d, "block", "")); + ASSERT_OK_POSITIVE(device_is_subsystem_devtype(d, "block", NULL)); + ASSERT_OK_ZERO(device_is_subsystem_devtype(d, "net", t)); + ASSERT_OK_ZERO(device_is_subsystem_devtype(d, "net", "hoge")); + ASSERT_OK_ZERO(device_is_subsystem_devtype(d, "net", "")); + ASSERT_OK_ZERO(device_is_subsystem_devtype(d, "net", NULL)); + ASSERT_OK_ZERO(device_is_subsystem_devtype(d, "subsystem", t)); + ASSERT_OK_ZERO(device_is_subsystem_devtype(d, "subsystem", "hoge")); + ASSERT_OK_ZERO(device_is_subsystem_devtype(d, "subsystem", "")); + ASSERT_OK_ZERO(device_is_subsystem_devtype(d, "subsystem", NULL)); + ASSERT_OK_ZERO(device_is_subsystem_devtype(d, NULL, t)); + ASSERT_OK_ZERO(device_is_subsystem_devtype(d, NULL, "hoge")); + ASSERT_OK_ZERO(device_is_subsystem_devtype(d, NULL, "")); + ASSERT_OK_ZERO(device_is_subsystem_devtype(d, NULL, NULL)); + + const char *s; + ASSERT_OK(sd_device_get_sysname(d, &s)); + ASSERT_OK_POSITIVE(device_sysname_startswith(d, s)); + ASSERT_OK_POSITIVE(device_sysname_startswith(d, CHAR_TO_STR(s[0]))); + ASSERT_OK_POSITIVE(device_sysname_startswith(d, "")); + ASSERT_OK_ZERO(device_sysname_startswith(d, "00")); } - assert_se(sd_device_new_from_syspath(&dev, "/sys/class/net") >= 0); - assert_se(!device_is_devtype(dev, "hoge")); - assert_se(!device_is_devtype(dev, "")); - assert_se(device_is_devtype(dev, NULL)); + _cleanup_(sd_device_unrefp) sd_device *dev = NULL; + + if (sd_device_new_from_subsystem_sysname(&dev, "net", "lo") >= 0) { + ASSERT_OK_POSITIVE(device_in_subsystem(dev, "net")); + ASSERT_OK_ZERO(device_in_subsystem(dev, "block")); + ASSERT_OK_ZERO(device_in_subsystem(dev, "subsystem")); + ASSERT_OK_POSITIVE(device_in_subsystem(dev, "", "net")); + ASSERT_OK_POSITIVE(device_in_subsystem(dev, "net", "block")); + ASSERT_OK_ZERO(device_in_subsystem(dev, "block", "subsystem")); + ASSERT_OK_ZERO(device_in_subsystem(dev, "block", "")); + ASSERT_OK_ZERO(device_in_subsystem(dev, "")); + ASSERT_OK_ZERO(device_in_subsystem(dev, NULL)); + + ASSERT_OK_POSITIVE(device_in_subsystem_strv(dev, STRV_MAKE("net"))); + ASSERT_OK_POSITIVE(device_in_subsystem_strv(dev, STRV_MAKE("", "net"))); + ASSERT_OK_POSITIVE(device_in_subsystem_strv(dev, STRV_MAKE("net", "block"))); + ASSERT_OK_ZERO(device_in_subsystem_strv(dev, STRV_MAKE("block", "subsystem"))); + ASSERT_OK_ZERO(device_in_subsystem_strv(dev, STRV_MAKE("block", ""))); + ASSERT_OK_ZERO(device_in_subsystem_strv(dev, STRV_MAKE(""))); + ASSERT_OK_ZERO(device_in_subsystem_strv(dev, STRV_MAKE(NULL))); + ASSERT_OK_ZERO(device_in_subsystem_strv(dev, NULL)); + + ASSERT_OK_ZERO(device_is_devtype(dev, "hoge")); + ASSERT_OK_ZERO(device_is_devtype(dev, "")); + ASSERT_OK_POSITIVE(device_is_devtype(dev, NULL)); + + ASSERT_OK_ZERO(device_is_subsystem_devtype(dev, "block", "hoge")); + ASSERT_OK_ZERO(device_is_subsystem_devtype(dev, "block", "")); + ASSERT_OK_ZERO(device_is_subsystem_devtype(dev, "block", NULL)); + ASSERT_OK_ZERO(device_is_subsystem_devtype(dev, "net", "hoge")); + ASSERT_OK_ZERO(device_is_subsystem_devtype(dev, "net", "")); + ASSERT_OK_POSITIVE(device_is_subsystem_devtype(dev, "net", NULL)); + ASSERT_OK_ZERO(device_is_subsystem_devtype(dev, "subsystem", "hoge")); + ASSERT_OK_ZERO(device_is_subsystem_devtype(dev, "subsystem", "")); + ASSERT_OK_ZERO(device_is_subsystem_devtype(dev, "subsystem", NULL)); + ASSERT_OK_ZERO(device_is_subsystem_devtype(dev, NULL, "hoge")); + ASSERT_OK_ZERO(device_is_subsystem_devtype(dev, NULL, "")); + ASSERT_OK_ZERO(device_is_subsystem_devtype(dev, NULL, NULL)); + + ASSERT_OK_POSITIVE(device_sysname_startswith(dev, "lo")); + ASSERT_OK_POSITIVE(device_sysname_startswith(dev, "l")); + ASSERT_OK_POSITIVE(device_sysname_startswith(dev, "")); + ASSERT_OK_ZERO(device_sysname_startswith(dev, "00")); + + dev = sd_device_unref(dev); + } + + ASSERT_OK(sd_device_new_from_syspath(&dev, "/sys/class/net")); + ASSERT_OK_ZERO(device_in_subsystem(dev, "net")); + ASSERT_OK_ZERO(device_in_subsystem(dev, "block")); + ASSERT_OK_POSITIVE(device_in_subsystem(dev, "subsystem")); + ASSERT_OK_ZERO(device_in_subsystem(dev, "", "net")); + ASSERT_OK_ZERO(device_in_subsystem(dev, "net", "block")); + ASSERT_OK_POSITIVE(device_in_subsystem(dev, "block", "subsystem")); + ASSERT_OK_ZERO(device_in_subsystem(dev, "block", "")); + ASSERT_OK_ZERO(device_in_subsystem(dev, "")); + ASSERT_OK_ZERO(device_in_subsystem(dev, NULL)); + + ASSERT_OK_ZERO(device_in_subsystem_strv(dev, STRV_MAKE("net"))); + ASSERT_OK_ZERO(device_in_subsystem_strv(dev, STRV_MAKE("", "net"))); + ASSERT_OK_ZERO(device_in_subsystem_strv(dev, STRV_MAKE("net", "block"))); + ASSERT_OK_POSITIVE(device_in_subsystem_strv(dev, STRV_MAKE("block", "subsystem"))); + ASSERT_OK_ZERO(device_in_subsystem_strv(dev, STRV_MAKE("block", ""))); + ASSERT_OK_ZERO(device_in_subsystem_strv(dev, STRV_MAKE(""))); + ASSERT_OK_ZERO(device_in_subsystem_strv(dev, STRV_MAKE(NULL))); + ASSERT_OK_ZERO(device_in_subsystem_strv(dev, NULL)); + + ASSERT_OK_ZERO(device_is_devtype(dev, "hoge")); + ASSERT_OK_ZERO(device_is_devtype(dev, "")); + ASSERT_OK_POSITIVE(device_is_devtype(dev, NULL)); + + ASSERT_OK_ZERO(device_is_subsystem_devtype(dev, "block", "hoge")); + ASSERT_OK_ZERO(device_is_subsystem_devtype(dev, "block", "")); + ASSERT_OK_ZERO(device_is_subsystem_devtype(dev, "block", NULL)); + ASSERT_OK_ZERO(device_is_subsystem_devtype(dev, "subsystem", "hoge")); + ASSERT_OK_ZERO(device_is_subsystem_devtype(dev, "subsystem", "")); + ASSERT_OK_POSITIVE(device_is_subsystem_devtype(dev, "subsystem", NULL)); + ASSERT_OK_ZERO(device_is_subsystem_devtype(dev, NULL, "hoge")); + ASSERT_OK_ZERO(device_is_subsystem_devtype(dev, NULL, "")); + ASSERT_OK_ZERO(device_is_subsystem_devtype(dev, NULL, NULL)); + + ASSERT_OK_POSITIVE(device_sysname_startswith(dev, "net")); + ASSERT_OK_POSITIVE(device_sysname_startswith(dev, "n")); + ASSERT_OK_POSITIVE(device_sysname_startswith(dev, "")); + ASSERT_OK_ZERO(device_sysname_startswith(dev, "00")); + + dev = sd_device_unref(dev); + + ASSERT_OK(sd_device_new_from_syspath(&dev, "/sys/class")); + ASSERT_OK_ZERO(device_in_subsystem(dev, "net")); + ASSERT_OK_ZERO(device_in_subsystem(dev, "block")); + ASSERT_OK_ZERO(device_in_subsystem(dev, "subsystem")); + ASSERT_OK_ZERO(device_in_subsystem(dev, "", "net")); + ASSERT_OK_ZERO(device_in_subsystem(dev, "net", "block")); + ASSERT_OK_ZERO(device_in_subsystem(dev, "block", "subsystem")); + ASSERT_OK_ZERO(device_in_subsystem(dev, "block", "")); + ASSERT_OK_ZERO(device_in_subsystem(dev, "")); + ASSERT_OK_POSITIVE(device_in_subsystem(dev, NULL)); + + ASSERT_OK_ZERO(device_in_subsystem_strv(dev, STRV_MAKE("net"))); + ASSERT_OK_ZERO(device_in_subsystem_strv(dev, STRV_MAKE("", "net"))); + ASSERT_OK_ZERO(device_in_subsystem_strv(dev, STRV_MAKE("net", "block"))); + ASSERT_OK_ZERO(device_in_subsystem_strv(dev, STRV_MAKE("block", "subsystem"))); + ASSERT_OK_ZERO(device_in_subsystem_strv(dev, STRV_MAKE("block", ""))); + ASSERT_OK_ZERO(device_in_subsystem_strv(dev, STRV_MAKE(""))); + ASSERT_OK_POSITIVE(device_in_subsystem_strv(dev, STRV_MAKE(NULL))); + ASSERT_OK_POSITIVE(device_in_subsystem_strv(dev, NULL)); + + ASSERT_OK_ZERO(device_is_devtype(dev, "hoge")); + ASSERT_OK_ZERO(device_is_devtype(dev, "")); + ASSERT_OK_POSITIVE(device_is_devtype(dev, NULL)); + + ASSERT_OK_ZERO(device_is_subsystem_devtype(dev, "block", "hoge")); + ASSERT_OK_ZERO(device_is_subsystem_devtype(dev, "block", "")); + ASSERT_OK_ZERO(device_is_subsystem_devtype(dev, "block", NULL)); + ASSERT_OK_ZERO(device_is_subsystem_devtype(dev, "subsystem", "hoge")); + ASSERT_OK_ZERO(device_is_subsystem_devtype(dev, "subsystem", "")); + ASSERT_OK_ZERO(device_is_subsystem_devtype(dev, "subsystem", NULL)); + ASSERT_OK_ZERO(device_is_subsystem_devtype(dev, NULL, "hoge")); + ASSERT_OK_ZERO(device_is_subsystem_devtype(dev, NULL, "")); + ASSERT_OK_POSITIVE(device_is_subsystem_devtype(dev, NULL, NULL)); + + ASSERT_OK_POSITIVE(device_sysname_startswith(dev, "class")); + ASSERT_OK_POSITIVE(device_sysname_startswith(dev, "c")); + ASSERT_OK_POSITIVE(device_sysname_startswith(dev, "")); + ASSERT_OK_ZERO(device_sysname_startswith(dev, "00")); } static int intro(void) { diff --git a/src/libsystemd/sd-device/test-sd-device.c b/src/libsystemd/sd-device/test-sd-device.c index 2db4f42c74..ff59d3bd50 100644 --- a/src/libsystemd/sd-device/test-sd-device.c +++ b/src/libsystemd/sd-device/test-sd-device.c @@ -539,6 +539,7 @@ TEST(sd_device_enumerator_add_match_parent) { TEST(sd_device_enumerator_add_all_parents) { _cleanup_(sd_device_enumerator_unrefp) sd_device_enumerator *e = NULL; + int r; /* STEP 1: enumerate all block devices without all_parents() */ ASSERT_OK(sd_device_enumerator_new(&e)); @@ -552,8 +553,7 @@ TEST(sd_device_enumerator_add_all_parents) { unsigned devices_count_with_parents = 0; unsigned devices_count_without_parents = 0; FOREACH_DEVICE(e, dev) { - ASSERT_TRUE(device_in_subsystem(dev, "block")); - ASSERT_TRUE(device_is_devtype(dev, "partition")); + ASSERT_OK_POSITIVE(device_is_subsystem_devtype(dev, "block", "partition")); devices_count_without_parents++; } @@ -564,7 +564,8 @@ TEST(sd_device_enumerator_add_all_parents) { unsigned not_filtered_parent_count = 0; FOREACH_DEVICE(e, dev) { - if (!device_in_subsystem(dev, "block") || !device_is_devtype(dev, "partition")) + ASSERT_OK(r = device_is_subsystem_devtype(dev, "block", "partition")); + if (r == 0) not_filtered_parent_count++; devices_count_with_parents++; } diff --git a/src/login/logind-core.c b/src/login/logind-core.c index 75fd0a2d87..6d435c0ebe 100644 --- a/src/login/logind-core.c +++ b/src/login/logind-core.c @@ -608,7 +608,6 @@ static int manager_count_external_displays(Manager *m) { return r; FOREACH_DEVICE(e, d) { - const char *status, *enabled, *dash, *nn; sd_device *p; if (sd_device_get_parent(d, &p) < 0) @@ -617,17 +616,22 @@ static int manager_count_external_displays(Manager *m) { /* If the parent shares the same subsystem as the * device we are looking at then it is a connector, * which is what we are interested in. */ - if (!device_in_subsystem(p, "drm")) + r = device_in_subsystem(p, "drm"); + if (r < 0) + return r; + if (r == 0) continue; - if (sd_device_get_sysname(d, &nn) < 0) - continue; + const char *nn; + r = sd_device_get_sysname(d, &nn); + if (r < 0) + return r; /* Ignore internal displays: the type is encoded in the sysfs name, as the second dash * separated item (the first is the card name, the last the connector number). We implement a * deny list of external displays here, rather than an allow list of internal ones, to ensure * we don't block suspends too eagerly. */ - dash = strchr(nn, '-'); + const char *dash = strchr(nn, '-'); if (!dash) continue; @@ -639,12 +643,21 @@ static int manager_count_external_displays(Manager *m) { continue; /* Ignore ports that are not enabled */ - if (sd_device_get_sysattr_value(d, "enabled", &enabled) < 0 || !streq(enabled, "enabled")) + const char *enabled; + r = sd_device_get_sysattr_value(d, "enabled", &enabled); + if (r == -ENOENT) + continue; + if (r < 0) + return r; + if (!streq(enabled, "enabled")) continue; - /* We count any connector which is not explicitly - * "disconnected" as connected. */ - if (sd_device_get_sysattr_value(d, "status", &status) < 0 || !streq(status, "disconnected")) + /* We count any connector which is not explicitly "disconnected" as connected. */ + const char *status = NULL; + r = sd_device_get_sysattr_value(d, "status", &status); + if (r < 0 && r != -ENOENT) + return r; + if (!streq_ptr(status, "disconnected")) n++; } diff --git a/src/login/logind-session-device.c b/src/login/logind-session-device.c index c51660540d..9e592740e8 100644 --- a/src/login/logind-session-device.c +++ b/src/login/logind-session-device.c @@ -273,21 +273,22 @@ static void session_device_stop(SessionDevice *sd) { } static DeviceType detect_device_type(sd_device *dev) { - const char *sysname; - - if (sd_device_get_sysname(dev, &sysname) < 0) - return DEVICE_TYPE_UNKNOWN; - - if (device_in_subsystem(dev, "drm")) { - if (startswith(sysname, "card")) + if (device_in_subsystem(dev, "drm") > 0) { + if (device_sysname_startswith(dev, "card") > 0) return DEVICE_TYPE_DRM; + return DEVICE_TYPE_UNKNOWN; + } - } else if (device_in_subsystem(dev, "input")) { - if (startswith(sysname, "event")) + if (device_in_subsystem(dev, "input") > 0) { + if (device_sysname_startswith(dev, "event") > 0) return DEVICE_TYPE_EVDEV; - } else if (device_in_subsystem(dev, "hidraw")) { - if (startswith(sysname, "hidraw")) + return DEVICE_TYPE_UNKNOWN; + } + + if (device_in_subsystem(dev, "hidraw") > 0) { + if (device_sysname_startswith(dev, "hidraw") > 0) return DEVICE_TYPE_HIDRAW; + return DEVICE_TYPE_UNKNOWN; } return DEVICE_TYPE_UNKNOWN; diff --git a/src/login/logind.c b/src/login/logind.c index aeace67315..877f30f937 100644 --- a/src/login/logind.c +++ b/src/login/logind.c @@ -628,16 +628,22 @@ static int manager_dispatch_device_udev(sd_device_monitor *monitor, sd_device *d static int manager_dispatch_vcsa_udev(sd_device_monitor *monitor, sd_device *device, void *userdata) { Manager *m = ASSERT_PTR(userdata); - const char *name; + int r; assert(device); /* Whenever a VCSA device is removed try to reallocate our * VTs, to make sure our auto VTs never go away. */ - if (sd_device_get_sysname(device, &name) >= 0 && - startswith(name, "vcsa") && - device_for_action(device, SD_DEVICE_REMOVE)) + r = device_sysname_startswith(device, "vcsa"); + if (r < 0) { + log_device_debug_errno(device, r, "Failed to check device sysname, ignoring: %m"); + return 0; + } + if (r == 0) + return 0; + + if (device_for_action(device, SD_DEVICE_REMOVE)) seat_preallocate_vts(m->seat0); return 0; diff --git a/src/mount/mount-tool.c b/src/mount/mount-tool.c index 2c64ccdf30..71fe112702 100644 --- a/src/mount/mount-tool.c +++ b/src/mount/mount-tool.c @@ -1302,6 +1302,7 @@ static int acquire_description(sd_device *d) { static int acquire_removable(sd_device *d) { const char *v; + int r; assert(d); @@ -1313,10 +1314,16 @@ static int acquire_removable(sd_device *d) { if (sd_device_get_sysattr_value(d, "removable", &v) >= 0) break; - if (sd_device_get_parent(d, &d) < 0) + r = sd_device_get_parent(d, &d); + if (r == -ENODEV) return 0; + if (r < 0) + return r; - if (!device_in_subsystem(d, "block")) + r = device_in_subsystem(d, "block"); + if (r < 0) + return r; + if (r == 0) return 0; } diff --git a/src/network/networkctl-link-info.c b/src/network/networkctl-link-info.c index 1768688980..c687da9e0f 100644 --- a/src/network/networkctl-link-info.c +++ b/src/network/networkctl-link-info.c @@ -327,7 +327,7 @@ static void acquire_wlan_link_info(LinkInfo *link) { if (!link->sd_device) return; - if (!device_is_devtype(link->sd_device, "wlan")) + if (device_is_devtype(link->sd_device, "wlan") <= 0) return; r = sd_genl_socket_open(&genl); diff --git a/src/network/networkd-manager.c b/src/network/networkd-manager.c index cf713f3ffe..7d29298d83 100644 --- a/src/network/networkd-manager.c +++ b/src/network/networkd-manager.c @@ -170,11 +170,11 @@ static int manager_process_uevent(sd_device_monitor *monitor, sd_device *device, if (r < 0) return log_device_warning_errno(device, r, "Failed to get udev action, ignoring: %m"); - if (device_in_subsystem(device, "net")) + if (device_in_subsystem(device, "net") > 0) r = manager_udev_process_link(m, device, action); - else if (device_in_subsystem(device, "ieee80211")) + else if (device_in_subsystem(device, "ieee80211") > 0) r = manager_udev_process_wiphy(m, device, action); - else if (device_in_subsystem(device, "rfkill")) + else if (device_in_subsystem(device, "rfkill") > 0) r = manager_udev_process_rfkill(m, device, action); if (r < 0) log_device_warning_errno(device, r, "Failed to process \"%s\" uevent, ignoring: %m", diff --git a/src/network/networkd-wiphy.c b/src/network/networkd-wiphy.c index 0641fea370..33f7fe680f 100644 --- a/src/network/networkd-wiphy.c +++ b/src/network/networkd-wiphy.c @@ -126,7 +126,10 @@ static int link_get_wiphy(Link *link, Wiphy **ret) { if (!link->dev) return -ENODEV; - if (!device_is_devtype(link->dev, "wlan")) + r = device_is_devtype(link->dev, "wlan"); + if (r < 0) + return r; + if (r == 0) return -EOPNOTSUPP; r = sd_device_new_child(&phy, link->dev, "phy80211"); diff --git a/src/shared/blockdev-list.c b/src/shared/blockdev-list.c index d38f1e4b2c..4c43fb6762 100644 --- a/src/shared/blockdev-list.c +++ b/src/shared/blockdev-list.c @@ -37,15 +37,12 @@ int blockdev_list(BlockDevListFlags flags) { } if (FLAGS_SET(flags, BLOCKDEV_LIST_IGNORE_ZRAM)) { - const char *dn; - - r = sd_device_get_sysname(dev, &dn); + r = device_sysname_startswith(dev, "zram"); if (r < 0) { - log_warning_errno(r, "Failed to get device name of discovered block device '%s', ignoring: %m", node); + log_warning_errno(r, "Failed to check device name of discovered block device '%s', ignoring: %m", node); continue; } - - if (startswith(dn, "zram")) + if (r > 0) continue; } diff --git a/src/shared/blockdev-util.c b/src/shared/blockdev-util.c index 968ff6adc3..fc3c3e7d14 100644 --- a/src/shared/blockdev-util.c +++ b/src/shared/blockdev-util.c @@ -57,9 +57,14 @@ static int fd_get_devnum(int fd, BlockDeviceLookupFlag flags, dev_t *ret) { } int block_device_is_whole_disk(sd_device *dev) { + int r; + assert(dev); - if (!device_in_subsystem(dev, "block")) + r = device_in_subsystem(dev, "block"); + if (r < 0) + return r; + if (r == 0) return -ENOTBLK; return device_is_devtype(dev, "disk"); @@ -414,7 +419,10 @@ int blockdev_partscan_enabled(sd_device *dev) { /* Partition block devices never have partition scanning on, there's no concept of sub-partitions for * partitions. */ - if (device_is_devtype(dev, "partition")) + r = device_is_devtype(dev, "partition"); + if (r < 0) + return r; + if (r > 0) return false; /* For loopback block device, especially for v5.19 or newer. Even if this is enabled, we also need to diff --git a/src/udev/test-udev-rule-runner.c b/src/udev/test-udev-rule-runner.c index 9ad446aa2d..3371892ec4 100644 --- a/src/udev/test-udev-rule-runner.c +++ b/src/udev/test-udev-rule-runner.c @@ -151,7 +151,10 @@ static int run(int argc, char *argv[]) { if (sd_device_get_devname(dev, &devname) >= 0) { mode_t mode = 0600; - if (device_in_subsystem(dev, "block")) + r = device_in_subsystem(dev, "block"); + if (r < 0) + return r; + if (r > 0) mode |= S_IFBLK; else mode |= S_IFCHR; diff --git a/src/udev/udev-builtin-blkid.c b/src/udev/udev-builtin-blkid.c index 1448fa3f04..6cd738c3a5 100644 --- a/src/udev/udev-builtin-blkid.c +++ b/src/udev/udev-builtin-blkid.c @@ -364,7 +364,6 @@ static int read_loopback_backing_inode( _cleanup_free_ char *fn = NULL; struct loop_info64 info; - const char *name; int r; assert(dev); @@ -381,11 +380,10 @@ static int read_loopback_backing_inode( * sometimes, depending on context, it's a good thing to return the string userspace can freely pick * over the string automatically generated by the kernel. */ - r = sd_device_get_sysname(dev, &name); + r = device_sysname_startswith(dev, "loop"); if (r < 0) return r; - - if (!startswith(name, "loop")) + if (r == 0) goto notloop; if (ioctl(fd, LOOP_GET_STATUS64, &info) < 0) { diff --git a/src/udev/udev-builtin-dissect_image.c b/src/udev/udev-builtin-dissect_image.c index 8bc8df990c..5d6b0d8fce 100644 --- a/src/udev/udev-builtin-dissect_image.c +++ b/src/udev/udev-builtin-dissect_image.c @@ -318,20 +318,22 @@ static int verb_copy(UdevEvent *event, sd_device *dev) { if (r < 0) return log_device_debug_errno(dev, r, "Failed to get device node: %m"); - if (!device_in_subsystem(dev, "block")) - return log_device_debug_errno(dev, SYNTHETIC_ERRNO(EINVAL), "Invoked on non-block device '%s', refusing: %m", devnode); - if (!device_is_devtype(dev, "partition")) - return log_device_debug_errno(dev, SYNTHETIC_ERRNO(EINVAL), "Invoked on non-partition block device '%s', refusing: %m", devnode); + r = device_is_subsystem_devtype(dev, "block", "partition"); + if (r < 0) + return log_device_debug_errno(dev, r, "Failed to check if the device '%s' is a partition, refusing: %m", devnode); + if (r == 0) + return log_device_debug_errno(dev, SYNTHETIC_ERRNO(EINVAL), "Invoked on non-partition device '%s', refusing.", devnode); sd_device *parent; r = sd_device_get_parent(dev, &parent); if (r < 0) return log_error_errno(r, "Failed to get parent of device '%s': %m", devnode); - if (!device_in_subsystem(parent, "block")) - return log_device_debug_errno(dev, SYNTHETIC_ERRNO(EINVAL), "Parent of block device '%s' is not a block device, refusing: %m", devnode); - if (!device_is_devtype(parent, "disk")) - return log_device_debug_errno(dev, SYNTHETIC_ERRNO(EINVAL), "Parent of block device '%s' is not a whole block device, refusing: %m", devnode); + r = device_is_subsystem_devtype(parent, "block", "disk"); + if (r < 0) + return log_device_debug_errno(dev, r, "Failed to check if the parent of block device '%s' is a whole block device, refusing: %m", devnode); + if (r == 0) + return log_device_debug_errno(dev, SYNTHETIC_ERRNO(EINVAL), "Parent of block device '%s' is not a whole block device, refusing.", devnode); const char *partn; r = sd_device_get_property_value(dev, "PARTN", &partn); diff --git a/src/udev/udev-builtin-hwdb.c b/src/udev/udev-builtin-hwdb.c index 458ce6968a..6e82ad8c75 100644 --- a/src/udev/udev-builtin-hwdb.c +++ b/src/udev/udev-builtin-hwdb.c @@ -88,12 +88,20 @@ static int udev_builtin_hwdb_search( const char *modalias = NULL; /* look only at devices of a specific subsystem */ - if (subsystem && !device_in_subsystem(d, subsystem)) - goto next; + if (subsystem) { + r = device_in_subsystem(d, subsystem); + if (r < 0) + return r; + if (r == 0) + goto next; + } (void) sd_device_get_property_value(d, "MODALIAS", &modalias); - if (device_in_subsystem(d, "usb") && device_is_devtype(d, "usb_device")) { + r = device_is_subsystem_devtype(d, "usb", "usb_device"); + if (r < 0) + return r; + if (r > 0) { /* if the usb_device does not have a modalias, compose one */ if (!modalias) modalias = modalias_usb(d, s, sizeof(s)); diff --git a/src/udev/udev-builtin-input_id.c b/src/udev/udev-builtin-input_id.c index 60fc95e0d2..f8ac6d5517 100644 --- a/src/udev/udev-builtin-input_id.c +++ b/src/udev/udev-builtin-input_id.c @@ -384,8 +384,8 @@ static int builtin_input_id(UdevEvent *event, int argc, char *argv[]) { bitmask_key[NBITS(KEY_MAX)], bitmask_rel[NBITS(REL_MAX)], bitmask_props[NBITS(INPUT_PROP_MAX)]; - const char *sysname; bool is_pointer, is_key; + int r; /* walk up the parental chain until we find the real input device; the * argument is very likely a subdevice of this, like eventN */ @@ -425,8 +425,10 @@ static int builtin_input_id(UdevEvent *event, int argc, char *argv[]) { udev_builtin_add_property(event, "ID_INPUT_SWITCH", "1"); } - if (sd_device_get_sysname(dev, &sysname) >= 0 && - startswith(sysname, "event")) + r = device_sysname_startswith(dev, "event"); + if (r < 0) + return r; + if (r > 0) extract_info(event); return 0; diff --git a/src/udev/udev-builtin-net_id.c b/src/udev/udev-builtin-net_id.c index e20faae8df..a5ace58db6 100644 --- a/src/udev/udev-builtin-net_id.c +++ b/src/udev/udev-builtin-net_id.c @@ -43,20 +43,29 @@ #define ONBOARD_14BIT_INDEX_MAX ((1U << 14) - 1) #define ONBOARD_16BIT_INDEX_MAX ((1U << 16) - 1) -/* skip intermediate virtio devices */ -static sd_device *device_skip_virtio(sd_device *dev) { - /* there can only ever be one virtio bus per parent device, so we can - * safely ignore any virtio buses. see +static int device_get_parent_skip_virtio(sd_device *dev, sd_device **ret) { + int r; + + assert(dev); + assert(ret); + + /* This provides the parent device, but skips intermediate virtio devices. There can only ever be one + * virtio bus per parent device, so we can safely ignore any virtio buses. See * https://lore.kernel.org/virtualization/CAPXgP137A=CdmggtVPUZXbnpTbU9Tewq-sOjg9T8ohYktct1kQ@mail.gmail.com/ */ - while (dev) { - if (!device_in_subsystem(dev, "virtio")) - break; - if (sd_device_get_parent(dev, &dev) < 0) - return NULL; + for (;;) { + r = sd_device_get_parent(dev, &dev); + if (r < 0) + return r; + + r = device_in_subsystem(dev, "virtio"); + if (r < 0) + return r; + if (r == 0) { + *ret = dev; + return 0; + } } - - return dev; } static int get_matching_parent( @@ -70,26 +79,23 @@ static int get_matching_parent( assert(dev); - r = sd_device_get_parent(dev, &parent); + if (skip_virtio) + r = device_get_parent_skip_virtio(dev, &parent); + else + r = sd_device_get_parent(dev, &parent); if (r < 0) return r; - if (skip_virtio) { - /* skip virtio subsystem if present */ - parent = device_skip_virtio(parent); - if (!parent) - return -ENODEV; - } - /* check if our direct parent is in an expected subsystem. */ - STRV_FOREACH(s, parent_subsystems) - if (device_in_subsystem(parent, *s)) { - if (ret) - *ret = parent; - return 0; - } + r = device_in_subsystem_strv(parent, parent_subsystems); + if (r < 0) + return r; + if (r == 0) + return -ENODEV; - return -ENODEV; + if (ret) + *ret = parent; + return 0; } static int get_first_syspath_component(sd_device *dev, const char *prefix, char **ret) { @@ -1284,9 +1290,9 @@ static int get_ifname_prefix(sd_device *dev, const char **ret) { /* handle only ARPHRD_ETHER, ARPHRD_SLIP and ARPHRD_INFINIBAND devices */ switch (iftype) { case ARPHRD_ETHER: { - if (device_is_devtype(dev, "wlan")) + if (device_is_devtype(dev, "wlan") > 0) *ret = "wl"; - else if (device_is_devtype(dev, "wwan")) + else if (device_is_devtype(dev, "wwan") > 0) *ret = "ww"; else *ret = "en"; diff --git a/src/udev/udev-builtin-path_id.c b/src/udev/udev-builtin-path_id.c index 5237ae3090..27897b461f 100644 --- a/src/udev/udev-builtin-path_id.c +++ b/src/udev/udev-builtin-path_id.c @@ -89,7 +89,7 @@ static sd_device* skip_subsystem(sd_device *dev, const char *subsys) { */ for (parent = dev; ; ) { - if (!device_in_subsystem(parent, subsys)) + if (device_in_subsystem(parent, subsys) <= 0) break; dev = parent; @@ -404,7 +404,7 @@ static sd_device* handle_scsi_hyperv(sd_device *parent, char **path, size_t guid static sd_device* handle_scsi(sd_device *parent, char **path, char **compat_path, bool *supported_parent) { const char *id, *name; - if (!device_is_devtype(parent, "scsi_device")) + if (device_is_devtype(parent, "scsi_device") <= 0) return parent; /* firewire */ @@ -519,7 +519,7 @@ static sd_device* handle_usb(sd_device *parent, char **path) { const char *str, *port; int r; - if (!device_is_devtype(parent, "usb_interface") && !device_is_devtype(parent, "usb_device")) + if (device_is_devtype(parent, "usb_interface") <= 0 && device_is_devtype(parent, "usb_device") <= 0) return parent; if (sd_device_get_sysname(parent, &str) < 0) @@ -699,95 +699,95 @@ static int builtin_path_id(UdevEvent *event, int argc, char *argv[]) { if (sd_device_get_sysname(parent, &sysname) < 0) { ; - } else if (device_in_subsystem(parent, "scsi_tape")) { + } else if (device_in_subsystem(parent, "scsi_tape") > 0) { handle_scsi_tape(parent, &path); - } else if (device_in_subsystem(parent, "scsi")) { + } else if (device_in_subsystem(parent, "scsi") > 0) { parent = handle_scsi(parent, &path, &compat_path, &supported_parent); supported_transport = true; - } else if (device_in_subsystem(parent, "cciss")) { + } else if (device_in_subsystem(parent, "cciss") > 0) { parent = handle_cciss(parent, &path); supported_transport = true; - } else if (device_in_subsystem(parent, "usb")) { + } else if (device_in_subsystem(parent, "usb") > 0) { parent = handle_usb(parent, &path); supported_transport = true; - } else if (device_in_subsystem(parent, "bcma")) { + } else if (device_in_subsystem(parent, "bcma") > 0) { parent = handle_bcma(parent, &path); supported_transport = true; - } else if (device_in_subsystem(parent, "serio")) { + } else if (device_in_subsystem(parent, "serio") > 0) { const char *sysnum; if (sd_device_get_sysnum(parent, &sysnum) >= 0) { path_prepend(&path, "serio-%s", sysnum); parent = skip_subsystem(parent, "serio"); } - } else if (device_in_subsystem(parent, "pci")) { + } else if (device_in_subsystem(parent, "pci") > 0) { path_prepend(&path, "pci-%s", sysname); if (compat_path) path_prepend(&compat_path, "pci-%s", sysname); parent = skip_subsystem(parent, "pci"); supported_parent = true; - } else if (device_in_subsystem(parent, "platform")) { + } else if (device_in_subsystem(parent, "platform") > 0) { path_prepend(&path, "platform-%s", sysname); if (compat_path) path_prepend(&compat_path, "platform-%s", sysname); parent = skip_subsystem(parent, "platform"); supported_transport = true; supported_parent = true; - } else if (device_in_subsystem(parent, "amba")) { + } else if (device_in_subsystem(parent, "amba") > 0) { path_prepend(&path, "amba-%s", sysname); if (compat_path) path_prepend(&compat_path, "amba-%s", sysname); parent = skip_subsystem(parent, "amba"); supported_transport = true; supported_parent = true; - } else if (device_in_subsystem(parent, "acpi")) { + } else if (device_in_subsystem(parent, "acpi") > 0) { path_prepend(&path, "acpi-%s", sysname); if (compat_path) path_prepend(&compat_path, "acpi-%s", sysname); parent = skip_subsystem(parent, "acpi"); supported_parent = true; - } else if (device_in_subsystem(parent, "xen")) { + } else if (device_in_subsystem(parent, "xen") > 0) { path_prepend(&path, "xen-%s", sysname); if (compat_path) path_prepend(&compat_path, "xen-%s", sysname); parent = skip_subsystem(parent, "xen"); supported_parent = true; - } else if (device_in_subsystem(parent, "virtio")) { + } else if (device_in_subsystem(parent, "virtio") > 0) { parent = skip_subsystem(parent, "virtio"); supported_transport = true; - } else if (device_in_subsystem(parent, "scm")) { + } else if (device_in_subsystem(parent, "scm") > 0) { path_prepend(&path, "scm-%s", sysname); if (compat_path) path_prepend(&compat_path, "scm-%s", sysname); parent = skip_subsystem(parent, "scm"); supported_transport = true; supported_parent = true; - } else if (device_in_subsystem(parent, "ccw")) { + } else if (device_in_subsystem(parent, "ccw") > 0) { path_prepend(&path, "ccw-%s", sysname); if (compat_path) path_prepend(&compat_path, "ccw-%s", sysname); parent = skip_subsystem(parent, "ccw"); supported_transport = true; supported_parent = true; - } else if (device_in_subsystem(parent, "ccwgroup")) { + } else if (device_in_subsystem(parent, "ccwgroup") > 0) { path_prepend(&path, "ccwgroup-%s", sysname); if (compat_path) path_prepend(&compat_path, "ccwgroup-%s", sysname); parent = skip_subsystem(parent, "ccwgroup"); supported_transport = true; supported_parent = true; - } else if (device_in_subsystem(parent, "ap")) { + } else if (device_in_subsystem(parent, "ap") > 0) { parent = handle_ap(parent, &path); supported_transport = true; supported_parent = true; - } else if (device_in_subsystem(parent, "iucv")) { + } else if (device_in_subsystem(parent, "iucv") > 0) { path_prepend(&path, "iucv-%s", sysname); if (compat_path) path_prepend(&compat_path, "iucv-%s", sysname); parent = skip_subsystem(parent, "iucv"); supported_transport = true; supported_parent = true; - } else if (device_in_subsystem(parent, "nvme") || device_in_subsystem(parent, "nvme-subsystem")) { + } else if (device_in_subsystem(parent, "nvme", "nvme-subsystem") > 0) { const char *nsid; if (sd_device_get_sysattr_value(dev, "nsid", &nsid) >= 0) { @@ -795,7 +795,7 @@ static int builtin_path_id(UdevEvent *event, int argc, char *argv[]) { if (compat_path) path_prepend(&compat_path, "nvme-%s", nsid); - if (device_in_subsystem(parent, "nvme-subsystem")) { + if (device_in_subsystem(parent, "nvme-subsystem") > 0) { r = find_real_nvme_parent(dev, &dev_other_branch); if (r < 0) return r; @@ -807,7 +807,7 @@ static int builtin_path_id(UdevEvent *event, int argc, char *argv[]) { supported_parent = true; supported_transport = true; } - } else if (device_in_subsystem(parent, "spi")) { + } else if (device_in_subsystem(parent, "spi") > 0) { const char *sysnum; if (sd_device_get_sysnum(parent, &sysnum) >= 0) { @@ -838,7 +838,7 @@ static int builtin_path_id(UdevEvent *event, int argc, char *argv[]) { * devices do not expose their buses and do not provide a unique * and predictable name that way. */ - if (device_in_subsystem(dev, "block") && !supported_transport) + if (device_in_subsystem(dev, "block") > 0 && !supported_transport) return -ENOENT; add_id_with_usb_revision(event, path); diff --git a/src/udev/udev-node.c b/src/udev/udev-node.c index 08ce26a0c5..957f028f17 100644 --- a/src/udev/udev-node.c +++ b/src/udev/udev-node.c @@ -506,7 +506,11 @@ static int device_get_devpath_by_devnum(sd_device *dev, char **ret) { if (r < 0) return r; - return device_path_make_major_minor(device_in_subsystem(dev, "block") ? S_IFBLK : S_IFCHR, devnum, ret); + r = device_in_subsystem(dev, "block"); + if (r < 0) + return r; + + return device_path_make_major_minor(r > 0 ? S_IFBLK : S_IFCHR, devnum, ret); } int udev_node_update(sd_device *dev, sd_device *dev_old) { diff --git a/src/udev/udev-watch.c b/src/udev/udev-watch.c index 455ebd40e5..945bf0b8ca 100644 --- a/src/udev/udev-watch.c +++ b/src/udev/udev-watch.c @@ -199,12 +199,16 @@ static int synthesize_change(Manager *manager, sd_device *dev) { assert(manager); assert(dev); - const char *sysname; - r = sd_device_get_sysname(dev, &sysname); + r = device_sysname_startswith(dev, "dm-"); if (r < 0) return r; + if (r > 0) + return synthesize_change_one(dev, dev); - if (startswith(sysname, "dm-") || block_device_is_whole_disk(dev) <= 0) + r = block_device_is_whole_disk(dev); + if (r < 0) + return r; + if (r == 0) return synthesize_change_one(dev, dev); _cleanup_(pidref_done) PidRef pidref = PIDREF_NULL; diff --git a/src/udev/udev-worker.c b/src/udev/udev-worker.c index ba3c18c534..1f00606d87 100644 --- a/src/udev/udev-worker.c +++ b/src/udev/udev-worker.c @@ -47,17 +47,16 @@ int udev_get_whole_disk(sd_device *dev, sd_device **ret_device, const char **ret if (device_for_action(dev, SD_DEVICE_REMOVE)) goto irrelevant; - r = sd_device_get_sysname(dev, &val); - if (r < 0) - return log_device_debug_errno(dev, r, "Failed to get sysname: %m"); - /* Exclude the following devices: * For "dm-", see the comment added by e918a1b5a94f270186dca59156354acd2a596494. * For "md", see the commit message of 2e5b17d01347d3c3118be2b8ad63d20415dbb1f0, * but not sure the assumption is still valid even when partitions are created on the md * devices, surprisingly which seems to be possible, see PR #22973. * For "drbd", see the commit message of fee854ee8ccde0cd28e0f925dea18cce35f3993d. */ - if (STARTSWITH_SET(val, "dm-", "md", "drbd")) + r = device_sysname_startswith(dev, "dm-", "md", "drbd"); + if (r < 0) + return log_device_debug_errno(dev, r, "Failed to check sysname: %m"); + if (r > 0) goto irrelevant; r = block_device_get_whole_disk(dev, &dev); @@ -149,9 +148,7 @@ nolock: } static int worker_mark_block_device_read_only(sd_device *dev) { - _cleanup_close_ int fd = -EBADF; - const char *val; - int state = 1, r; + int r; assert(dev); @@ -161,23 +158,31 @@ static int worker_mark_block_device_read_only(sd_device *dev) { if (!device_for_action(dev, SD_DEVICE_ADD)) return 0; - if (!device_in_subsystem(dev, "block")) - return 0; - - r = sd_device_get_sysname(dev, &val); + r = device_in_subsystem(dev, "block"); if (r < 0) - return log_device_debug_errno(dev, r, "Failed to get sysname: %m"); + return r; + if (r == 0) + return 0; /* Exclude synthetic devices for now, this is supposed to be a safety feature to avoid modification * of physical devices, and what sits on top of those doesn't really matter if we don't allow the * underlying block devices to receive changes. */ - if (STARTSWITH_SET(val, "dm-", "md", "drbd", "loop", "nbd", "zram")) + r = device_sysname_startswith(dev, "dm-", "md", "drbd", "loop", "nbd", "zram"); + if (r < 0) + return log_device_debug_errno(dev, r, "Failed to check sysname: %m"); + if (r > 0) return 0; - fd = sd_device_open(dev, O_RDONLY|O_CLOEXEC|O_NONBLOCK|O_NOCTTY); + const char *val; + r = sd_device_get_devname(dev, &val); + if (r < 0) + return log_device_debug_errno(dev, r, "Failed to get device node: %m"); + + _cleanup_close_ int fd = sd_device_open(dev, O_RDONLY|O_CLOEXEC|O_NONBLOCK|O_NOCTTY); if (fd < 0) return log_device_debug_errno(dev, fd, "Failed to open '%s', ignoring: %m", val); + int state = 1; if (ioctl(fd, BLKROSET, &state) < 0) return log_device_warning_errno(dev, errno, "Failed to mark block device '%s' read-only: %m", val); From 2012d6d74e79ddf46686234eabc50edad619d4df Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Tue, 13 May 2025 23:39:09 +0900 Subject: [PATCH 4/9] device-util: introduce device_get_seat() helper function --- src/home/homed-home.c | 7 +++---- src/libsystemd/sd-device/device-util.c | 15 +++++++++++++++ src/libsystemd/sd-device/device-util.h | 2 ++ src/login/logind-core.c | 10 ++++++---- src/login/logind-session-dbus.c | 6 +++++- src/login/sysfs-show.c | 12 +++++++----- src/shared/devnode-acl.c | 5 +++-- src/udev/udev-builtin-uaccess.c | 5 +++-- 8 files changed, 44 insertions(+), 18 deletions(-) diff --git a/src/home/homed-home.c b/src/home/homed-home.c index da5e304f1b..58a8ffff83 100644 --- a/src/home/homed-home.c +++ b/src/home/homed-home.c @@ -9,6 +9,7 @@ #include "build-path.h" #include "bus-common-errors.h" #include "bus-locator.h" +#include "device-util.h" #include "env-util.h" #include "errno-list.h" #include "errno-util.h" @@ -3206,10 +3207,8 @@ static int home_get_image_path_seat(Home *h, char **ret) { if (r < 0) return r; - r = sd_device_get_property_value(d, "ID_SEAT", &seat); - if (r == -ENOENT) /* no property means seat0 */ - seat = "seat0"; - else if (r < 0) + r = device_get_seat(d, &seat); + if (r < 0) return r; return strdup_to(ret, seat); diff --git a/src/libsystemd/sd-device/device-util.c b/src/libsystemd/sd-device/device-util.c index 67bcbf8af5..5169c24d84 100644 --- a/src/libsystemd/sd-device/device-util.c +++ b/src/libsystemd/sd-device/device-util.c @@ -191,6 +191,21 @@ int device_sysname_startswith_strv(sd_device *device, char * const *prefixes, co return !!suffix; } +int device_get_seat(sd_device *device, const char **ret) { + const char *seat = NULL; + int r; + + assert(device); + assert(ret); + + r = sd_device_get_property_value(device, "ID_SEAT", &seat); + if (r < 0 && r != -ENOENT) + return r; + + *ret = isempty(seat) ? "seat0" : seat; + return 0; +} + bool device_property_can_set(const char *property) { return property && !STR_IN_SET(property, diff --git a/src/libsystemd/sd-device/device-util.h b/src/libsystemd/sd-device/device-util.h index 008946f4de..5b899ecfcd 100644 --- a/src/libsystemd/sd-device/device-util.h +++ b/src/libsystemd/sd-device/device-util.h @@ -111,6 +111,8 @@ int device_is_devtype(sd_device *device, const char *devtype); int device_is_subsystem_devtype(sd_device *device, const char *subsystem, const char *devtype); +int device_get_seat(sd_device *device, const char **ret); + int device_sysname_startswith_strv(sd_device *device, char * const *prefixes, const char **ret_suffix); #define device_sysname_startswith(device, ...) \ device_sysname_startswith_strv(device, STRV_MAKE(__VA_ARGS__), NULL) diff --git a/src/login/logind-core.c b/src/login/logind-core.c index 6d435c0ebe..0208c74233 100644 --- a/src/login/logind-core.c +++ b/src/login/logind-core.c @@ -289,8 +289,9 @@ int manager_process_seat_device(Manager *m, sd_device *d) { bool master; Seat *seat; - if (sd_device_get_property_value(d, "ID_SEAT", &sn) < 0 || isempty(sn)) - sn = "seat0"; + r = device_get_seat(d, &sn); + if (r < 0) + return r; if (!seat_name_is_valid(sn)) { log_device_warning(d, "Device with invalid seat name %s found, ignoring.", sn); @@ -352,8 +353,9 @@ int manager_process_button_device(Manager *m, sd_device *d) { if (r < 0) return r; - if (sd_device_get_property_value(d, "ID_SEAT", &sn) < 0 || isempty(sn)) - sn = "seat0"; + r = device_get_seat(d, &sn); + if (r < 0) + return r; button_set_seat(b, sn); diff --git a/src/login/logind-session-dbus.c b/src/login/logind-session-dbus.c index f3fe877a2c..50a965c5ff 100644 --- a/src/login/logind-session-dbus.c +++ b/src/login/logind-session-dbus.c @@ -9,6 +9,7 @@ #include "bus-label.h" #include "bus-polkit.h" #include "bus-util.h" +#include "device-util.h" #include "devnum-util.h" #include "fd-util.h" #include "format-util.h" @@ -705,7 +706,10 @@ static int method_set_brightness(sd_bus_message *message, void *userdata, sd_bus if (r < 0) return sd_bus_error_set_errnof(error, r, "Failed to open device %s:%s: %m", subsystem, name); - if (sd_device_get_property_value(d, "ID_SEAT", &seat) >= 0 && !streq_ptr(seat, s->seat->id)) + r = device_get_seat(d, &seat); + if (r < 0) + return sd_bus_error_set_errnof(error, r, "Failed to get seat of %s:%s: %m", subsystem, name); + if (!streq(seat, s->seat->id)) return sd_bus_error_setf(error, BUS_ERROR_NOT_YOUR_DEVICE, "Device %s:%s does not belong to your seat %s, refusing.", subsystem, name, s->seat->id); r = manager_write_brightness(s->manager, d, brightness, message); diff --git a/src/login/sysfs-show.c b/src/login/sysfs-show.c index 43eb13ac83..d1f105b1e3 100644 --- a/src/login/sysfs-show.c +++ b/src/login/sysfs-show.c @@ -6,6 +6,7 @@ #include "alloc-util.h" #include "device-enumerator-private.h" +#include "device-util.h" #include "glyph-util.h" #include "path-util.h" #include "string-util.h" @@ -47,8 +48,9 @@ static int show_sysfs_one( !path_startswith(sysfs, sub)) return 0; - if (sd_device_get_property_value(dev_list[*i_dev], "ID_SEAT", &sn) < 0 || isempty(sn)) - sn = "seat0"; + r = device_get_seat(dev_list[*i_dev], &sn); + if (r < 0) + return r; /* Explicitly also check for tag 'seat' here */ if (!streq(seat, sn) || @@ -75,9 +77,9 @@ static int show_sysfs_one( !path_startswith(lookahead_sysfs, sysfs)) { const char *lookahead_sn; - if (sd_device_get_property_value(dev_list[lookahead], "ID_SEAT", &lookahead_sn) < 0 || - isempty(lookahead_sn)) - lookahead_sn = "seat0"; + r = device_get_seat(dev_list[lookahead], &lookahead_sn); + if (r < 0) + return r; if (streq(seat, lookahead_sn) && sd_device_has_current_tag(dev_list[lookahead], "seat") > 0) break; diff --git a/src/shared/devnode-acl.c b/src/shared/devnode-acl.c index 81961c5b05..945ad8db32 100644 --- a/src/shared/devnode-acl.c +++ b/src/shared/devnode-acl.c @@ -170,8 +170,9 @@ int devnode_acl_all(const char *seat, if (sd_device_has_current_tag(d, "uaccess") <= 0) continue; - if (sd_device_get_property_value(d, "ID_SEAT", &sn) < 0 || isempty(sn)) - sn = "seat0"; + r = device_get_seat(d, &sn); + if (r < 0) + return r; if (!streq(seat, sn)) continue; diff --git a/src/udev/udev-builtin-uaccess.c b/src/udev/udev-builtin-uaccess.c index 7fbd91a5c2..38863c0b10 100644 --- a/src/udev/udev-builtin-uaccess.c +++ b/src/udev/udev-builtin-uaccess.c @@ -32,8 +32,9 @@ static int builtin_uaccess(UdevEvent *event, int argc, char *argv[]) { return log_device_error_errno(dev, r, "Failed to get device node: %m"); const char *seat; - if (sd_device_get_property_value(dev, "ID_SEAT", &seat) < 0) - seat = "seat0"; + r = device_get_seat(dev, &seat); + if (r < 0) + return log_device_error_errno(dev, r, "Failed to get seat: %m"); uid_t uid; r = sd_seat_get_active(seat, /* ret_session = */ NULL, &uid); From 3400abf3cafd0291da5e6016154f7c65763bdea1 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Wed, 14 May 2025 00:21:41 +0900 Subject: [PATCH 5/9] login: use FOREACH_STRING() at one more place --- src/login/logind.c | 16 +++++----------- 1 file changed, 5 insertions(+), 11 deletions(-) diff --git a/src/login/logind.c b/src/login/logind.c index 877f30f937..31565f1fce 100644 --- a/src/login/logind.c +++ b/src/login/logind.c @@ -889,17 +889,11 @@ static int manager_connect_udev(Manager *m) { if (r < 0) return r; - r = sd_device_monitor_filter_add_match_subsystem_devtype(m->device_monitor, "input", NULL); - if (r < 0) - return r; - - r = sd_device_monitor_filter_add_match_subsystem_devtype(m->device_monitor, "graphics", NULL); - if (r < 0) - return r; - - r = sd_device_monitor_filter_add_match_subsystem_devtype(m->device_monitor, "drm", NULL); - if (r < 0) - return r; + FOREACH_STRING(s, "input", "graphics", "drm") { + r = sd_device_monitor_filter_add_match_subsystem_devtype(m->device_monitor, s, /* devtype = */ NULL); + if (r < 0) + return r; + } r = sd_device_monitor_attach_event(m->device_monitor, m->event); if (r < 0) From 26a675dd56463cba39b627e053fd71dc32366fde Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Tue, 13 May 2025 23:50:22 +0900 Subject: [PATCH 6/9] login: do not call manager_process_seat_device() more than once per event When udevd broadcasts an event for e.g. a graphics device with master-of-seat tag, then previously manager_process_seat_device() was called twice for the event. With this commit, the function is called only once even for an event for such device. --- src/login/logind.c | 20 ++++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) diff --git a/src/login/logind.c b/src/login/logind.c index 31565f1fce..0a8663412d 100644 --- a/src/login/logind.c +++ b/src/login/logind.c @@ -610,19 +610,35 @@ static int manager_enumerate_inhibitors(Manager *m) { static int manager_dispatch_seat_udev(sd_device_monitor *monitor, sd_device *device, void *userdata) { Manager *m = ASSERT_PTR(userdata); + int r; assert(device); - manager_process_seat_device(m, device); + r = manager_process_seat_device(m, device); + if (r < 0) + log_device_warning_errno(device, r, "Failed to process seat device, ignoring: %m"); + return 0; } static int manager_dispatch_device_udev(sd_device_monitor *monitor, sd_device *device, void *userdata) { Manager *m = ASSERT_PTR(userdata); + int r; assert(device); - manager_process_seat_device(m, device); + /* If the device currently has "master-of-seat" tag, then it has been or will be processed by + * manager_dispatch_seat_udev(). */ + r = sd_device_has_current_tag(device, "master-of-seat"); + if (r < 0) + log_device_warning_errno(device, r, "Failed to check if the device currently has master-of-seat tag, ignoring: %m"); + if (r != 0) + return 0; + + r = manager_process_seat_device(m, device); + if (r < 0) + log_device_warning_errno(device, r, "Failed to process seat device, ignoring: %m"); + return 0; } From c960ca2be1cfd183675df581f049a0c022c1c802 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Tue, 13 May 2025 23:02:13 +0900 Subject: [PATCH 7/9] login,udev: avoid race between systemd-logind and systemd-udevd in setting ACLs Previously, both udevd and logind modifies ACLs of a device node. Hence, there exists a race something like the following: 1. udevd reads an old state file, 2. logind updates the state file, and apply new ACLs, 3. udevd applies ACLs based on the old state file. This makes logind not update ACLs but trigger uevents for relevant devices to make ACLs updated by udevd. --- src/login/logind-seat.c | 144 +++++++++++++++++++++++++++++++--------- src/login/logind-seat.h | 7 +- src/login/logind.c | 16 +++++ 3 files changed, 136 insertions(+), 31 deletions(-) diff --git a/src/login/logind-seat.c b/src/login/logind-seat.c index 2c0905e64d..fb52c5475e 100644 --- a/src/login/logind-seat.c +++ b/src/login/logind-seat.c @@ -8,12 +8,13 @@ #include "sd-messages.h" #include "alloc-util.h" -#include "devnode-acl.h" +#include "device-util.h" #include "errno-util.h" #include "fd-util.h" #include "fileio.h" #include "format-util.h" #include "fs-util.h" +#include "id128-util.h" #include "logind.h" #include "logind-device.h" #include "logind-seat.h" @@ -78,6 +79,7 @@ Seat* seat_free(Seat *s) { hashmap_remove(s->manager->seats, s->id); + set_free(s->uevents); free(s->positions); free(s->state_file); free(s->id); @@ -207,19 +209,120 @@ int seat_preallocate_vts(Seat *s) { return r; } -int seat_apply_acls(Seat *s, Session *old_active) { +static void seat_triggered_uevents_done(Seat *s) { + assert(s); + + if (!set_isempty(s->uevents)) + return; + + Session *session = s->active; + + if (session) { + session_save(session); + user_save(session->user); + } + + if (session && session->started) { + session_send_changed(session, "Active"); + session_device_resume_all(session); + } + + if (!session || session->started) + seat_send_changed(s, "ActiveSession"); +} + +int manager_process_device_triggered_by_seat(Manager *m, sd_device *dev) { + int r; + + assert(m); + assert(dev); + + sd_id128_t uuid; + r = sd_device_get_trigger_uuid(dev, &uuid); + if (IN_SET(r, -ENOENT, -ENODATA)) + return 0; + if (r < 0) + return r; + + Seat *s; + HASHMAP_FOREACH(s, m->seats) + if (set_contains(s->uevents, &uuid)) + break; + if (!s) + return 0; + + free(ASSERT_PTR(set_remove(s->uevents, &uuid))); + seat_triggered_uevents_done(s); + + const char *id; + r = device_get_seat(dev, &id); + if (r < 0) + return r; + + if (!streq(id, s->id)) { + log_device_debug(dev, "ID_SEAT is changed in the triggered uevent: \"%s\" -> \"%s\"", s->id, id); + return 0; + } + + return 1; /* The uevent is triggered by the relevant seat. */ +} + +static int seat_trigger_devices(Seat *s) { int r; assert(s); - r = devnode_acl_all(s->id, - false, - !!old_active, old_active ? old_active->user->user_record->uid : 0, - !!s->active, s->active ? s->active->user->user_record->uid : 0); + set_clear(s->uevents); + _cleanup_(sd_device_enumerator_unrefp) sd_device_enumerator *e = NULL; + r = sd_device_enumerator_new(&e); if (r < 0) - return log_error_errno(r, "Failed to apply ACLs: %m"); + return r; + r = sd_device_enumerator_add_match_tag(e, "uaccess"); + if (r < 0) + return r; + + FOREACH_DEVICE(e, d) { + /* Verify that the tag is still in place. */ + r = sd_device_has_current_tag(d, "uaccess"); + if (r < 0) + return r; + if (r == 0) + continue; + + /* In case people mistag devices without nodes, we need to ignore this. */ + r = sd_device_get_devname(d, NULL); + if (r == -ENOENT) + continue; + if (r < 0) + return r; + + const char *id; + r = device_get_seat(d, &id); + if (r < 0) + return r; + + if (!streq(id, s->id)) + continue; + + sd_id128_t uuid; + r = sd_device_trigger_with_uuid(d, SD_DEVICE_CHANGE, &uuid); + if (r < 0) { + log_device_debug_errno(d, r, "Failed to trigger 'change' event, ignoring: %m"); + continue; + } + + _cleanup_free_ sd_id128_t *copy = newdup(sd_id128_t, &uuid, 1); + if (!copy) + return -ENOMEM; + + r = set_ensure_consume(&s->uevents, &id128_hash_ops_free, TAKE_PTR(copy)); + if (r < 0) + return r; + } + + seat_triggered_uevents_done(s); return 0; } @@ -237,7 +340,7 @@ int seat_set_active(Seat *s, Session *session) { * Therefore, if the active session has executed session_leave_vt , * A resume is required here. */ if (session == s->active) { - if (session) { + if (session && set_isempty(s->uevents)) { log_debug("Active session remains unchanged, resuming session devices."); session_device_resume_all(session); } @@ -250,32 +353,13 @@ int seat_set_active(Seat *s, Session *session) { seat_save(s); if (old_active) { + user_save(old_active->user); + session_save(old_active); session_device_pause_all(old_active); session_send_changed(old_active, "Active"); } - (void) seat_apply_acls(s, old_active); - - if (session && session->started) { - session_send_changed(session, "Active"); - session_device_resume_all(session); - } - - if (!session || session->started) - seat_send_changed(s, "ActiveSession"); - - if (session) { - session_save(session); - user_save(session->user); - } - - if (old_active) { - session_save(old_active); - if (!session || session->user != old_active->user) - user_save(old_active->user); - } - - return 0; + return seat_trigger_devices(s); } static Session* seat_get_position(Seat *s, unsigned pos) { diff --git a/src/login/logind-seat.h b/src/login/logind-seat.h index 4c4a6fa6bd..2dc57686e9 100644 --- a/src/login/logind-seat.h +++ b/src/login/logind-seat.h @@ -1,6 +1,8 @@ /* SPDX-License-Identifier: LGPL-2.1-or-later */ #pragma once +#include "sd-device.h" + #include "list.h" #include "memory-util.h" #include "string-util.h" @@ -19,6 +21,8 @@ struct Seat { LIST_HEAD(Device, devices); + Set *uevents; + Session *active; Session *pending_switch; LIST_HEAD(Session, sessions); @@ -39,7 +43,8 @@ DEFINE_TRIVIAL_CLEANUP_FUNC(Seat*, seat_free); int seat_save(Seat *s); int seat_load(Seat *s); -int seat_apply_acls(Seat *s, Session *old_active); +int manager_process_device_triggered_by_seat(Manager *m, sd_device *dev); + int seat_set_active(Seat *s, Session *session); int seat_switch_to(Seat *s, unsigned num); int seat_switch_to_next(Seat *s); diff --git a/src/login/logind.c b/src/login/logind.c index 0a8663412d..2a59c4af52 100644 --- a/src/login/logind.c +++ b/src/login/logind.c @@ -614,6 +614,14 @@ static int manager_dispatch_seat_udev(sd_device_monitor *monitor, sd_device *dev assert(device); + /* If the event is triggered by us, do not try to start the relevant seat again. Otherwise, starting + * the seat may trigger uevents again again again... */ + r = manager_process_device_triggered_by_seat(m, device); + if (r < 0) + log_device_warning_errno(device, r, "Failed to process seat device event triggered by us, ignoring: %m"); + if (r != 0) + return 0; + r = manager_process_seat_device(m, device); if (r < 0) log_device_warning_errno(device, r, "Failed to process seat device, ignoring: %m"); @@ -635,6 +643,14 @@ static int manager_dispatch_device_udev(sd_device_monitor *monitor, sd_device *d if (r != 0) return 0; + /* If the event is triggered by us, do not try to start the relevant seat again. Otherwise, starting + * the seat may trigger uevents again again again... */ + r = manager_process_device_triggered_by_seat(m, device); + if (r < 0) + log_device_warning_errno(device, r, "Failed to process seat device event triggered by us, ignoring: %m"); + if (r != 0) + return 0; + r = manager_process_seat_device(m, device); if (r < 0) log_device_warning_errno(device, r, "Failed to process seat device, ignoring: %m"); From 1abb592f2f886913492e4967cc96816c167177a9 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Thu, 27 Feb 2025 10:45:13 +0900 Subject: [PATCH 8/9] udev: move devnoce_acl() to udev-builtin-uaccess.c As it is now only used by udev-builtin-uaccess.c. This also makes devnode_acl() use fd rather than path to device node. --- src/shared/devnode-acl.c | 228 -------------------------------- src/shared/devnode-acl.h | 34 ----- src/shared/meson.build | 4 - src/udev/udev-builtin-uaccess.c | 113 ++++++++++++++-- 4 files changed, 100 insertions(+), 279 deletions(-) delete mode 100644 src/shared/devnode-acl.c delete mode 100644 src/shared/devnode-acl.h diff --git a/src/shared/devnode-acl.c b/src/shared/devnode-acl.c deleted file mode 100644 index 945ad8db32..0000000000 --- a/src/shared/devnode-acl.c +++ /dev/null @@ -1,228 +0,0 @@ -/* SPDX-License-Identifier: LGPL-2.1-or-later */ - -#include - -#include "sd-device.h" - -#include "acl-util.h" -#include "alloc-util.h" -#include "device-util.h" -#include "devnode-acl.h" -#include "dirent-util.h" -#include "errno-util.h" -#include "fd-util.h" -#include "format-util.h" -#include "fs-util.h" -#include "glyph-util.h" -#include "set.h" -#include "string-util.h" - -static int flush_acl(acl_t acl) { - acl_entry_t i; - int found; - bool changed = false; - - assert(acl); - - for (found = acl_get_entry(acl, ACL_FIRST_ENTRY, &i); - found > 0; - found = acl_get_entry(acl, ACL_NEXT_ENTRY, &i)) { - - acl_tag_t tag; - - if (acl_get_tag_type(i, &tag) < 0) - return -errno; - - if (tag != ACL_USER) - continue; - - if (acl_delete_entry(acl, i) < 0) - return -errno; - - changed = true; - } - - if (found < 0) - return -errno; - - return changed; -} - -int devnode_acl(const char *path, - bool flush, - bool del, uid_t old_uid, - bool add, uid_t new_uid) { - - _cleanup_(acl_freep) acl_t acl = NULL; - int r; - bool changed = false; - - assert(path); - - acl = acl_get_file(path, ACL_TYPE_ACCESS); - if (!acl) - return -errno; - - if (flush) { - - r = flush_acl(acl); - if (r < 0) - return r; - if (r > 0) - changed = true; - - } else if (del && old_uid > 0) { - acl_entry_t entry; - - r = acl_find_uid(acl, old_uid, &entry); - if (r < 0) - return r; - - if (r > 0) { - if (acl_delete_entry(acl, entry) < 0) - return -errno; - - changed = true; - } - } - - if (add && new_uid > 0) { - acl_entry_t entry; - acl_permset_t permset; - int rd, wt; - - r = acl_find_uid(acl, new_uid, &entry); - if (r < 0) - return r; - - if (r == 0) { - if (acl_create_entry(&acl, &entry) < 0) - return -errno; - - if (acl_set_tag_type(entry, ACL_USER) < 0 || - acl_set_qualifier(entry, &new_uid) < 0) - return -errno; - } - - if (acl_get_permset(entry, &permset) < 0) - return -errno; - - rd = acl_get_perm(permset, ACL_READ); - if (rd < 0) - return -errno; - - wt = acl_get_perm(permset, ACL_WRITE); - if (wt < 0) - return -errno; - - if (!rd || !wt) { - - if (acl_add_perm(permset, ACL_READ|ACL_WRITE) < 0) - return -errno; - - changed = true; - } - } - - if (!changed) - return 0; - - if (acl_calc_mask(&acl) < 0) - return -errno; - - if (acl_set_file(path, ACL_TYPE_ACCESS, acl) < 0) - return -errno; - - return 0; -} - -int devnode_acl_all(const char *seat, - bool flush, - bool del, uid_t old_uid, - bool add, uid_t new_uid) { - - _cleanup_(sd_device_enumerator_unrefp) sd_device_enumerator *e = NULL; - _cleanup_set_free_ Set *nodes = NULL; - _cleanup_closedir_ DIR *dir = NULL; - char *n; - int r; - - r = sd_device_enumerator_new(&e); - if (r < 0) - return r; - - if (isempty(seat)) - seat = "seat0"; - - /* We can only match by one tag in libudev. We choose - * "uaccess" for that. If we could match for two tags here we - * could add the seat name as second match tag, but this would - * be hardly optimizable in libudev, and hence checking the - * second tag manually in our loop is a good solution. */ - r = sd_device_enumerator_add_match_tag(e, "uaccess"); - if (r < 0) - return r; - - FOREACH_DEVICE(e, d) { - const char *node, *sn; - - /* Make sure the tag is still in place */ - if (sd_device_has_current_tag(d, "uaccess") <= 0) - continue; - - r = device_get_seat(d, &sn); - if (r < 0) - return r; - - if (!streq(seat, sn)) - continue; - - /* In case people mistag devices with nodes, we need to ignore this */ - if (sd_device_get_devname(d, &node) < 0) - continue; - - log_device_debug(d, "Found udev node %s for seat %s", node, seat); - r = set_put_strdup_full(&nodes, &path_hash_ops_free, node); - if (r < 0) - return r; - } - - /* udev exports "dead" device nodes to allow module on-demand loading, - * these devices are not known to the kernel at this moment */ - dir = opendir("/run/udev/static_node-tags/uaccess"); - if (dir) { - FOREACH_DIRENT(de, dir, return -errno) { - r = readlinkat_malloc(dirfd(dir), de->d_name, &n); - if (r == -ENOENT) - continue; - if (r < 0) { - log_debug_errno(r, - "Unable to read symlink '/run/udev/static_node-tags/uaccess/%s', ignoring: %m", - de->d_name); - continue; - } - - log_debug("Found static node %s for seat %s", n, seat); - r = set_ensure_consume(&nodes, &path_hash_ops_free, n); - if (r < 0) - return r; - } - } - - r = 0; - SET_FOREACH(n, nodes) { - int k; - - log_debug("Changing ACLs at %s for seat %s (uid "UID_FMT"%s"UID_FMT"%s%s)", - n, seat, old_uid, glyph(GLYPH_ARROW_RIGHT), new_uid, - del ? " del" : "", add ? " add" : ""); - - k = devnode_acl(n, flush, del, old_uid, add, new_uid); - if (k == -ENOENT) - log_debug("Device %s disappeared while setting ACLs", n); - else - RET_GATHER(r, k); - } - - return r; -} diff --git a/src/shared/devnode-acl.h b/src/shared/devnode-acl.h deleted file mode 100644 index c88f3c0cf3..0000000000 --- a/src/shared/devnode-acl.h +++ /dev/null @@ -1,34 +0,0 @@ -/* SPDX-License-Identifier: LGPL-2.1-or-later */ -#pragma once - -#include -#include - -#if HAVE_ACL - -int devnode_acl(const char *path, - bool flush, - bool del, uid_t old_uid, - bool add, uid_t new_uid); - -int devnode_acl_all(const char *seat, - bool flush, - bool del, uid_t old_uid, - bool add, uid_t new_uid); -#else - -static inline int devnode_acl(const char *path, - bool flush, - bool del, uid_t old_uid, - bool add, uid_t new_uid) { - return 0; -} - -static inline int devnode_acl_all(const char *seat, - bool flush, - bool del, uid_t old_uid, - bool add, uid_t new_uid) { - return 0; -} - -#endif diff --git a/src/shared/meson.build b/src/shared/meson.build index b5522637f0..d57bd93da1 100644 --- a/src/shared/meson.build +++ b/src/shared/meson.build @@ -238,10 +238,6 @@ syscall_list_h = custom_target( generated_sources += syscall_list_h -if conf.get('HAVE_ACL') == 1 - shared_sources += files('devnode-acl.c') -endif - if conf.get('ENABLE_UTMP') == 1 shared_sources += files('utmp-wtmp.c') endif diff --git a/src/udev/udev-builtin-uaccess.c b/src/udev/udev-builtin-uaccess.c index 38863c0b10..9a9ba43b1c 100644 --- a/src/udev/udev-builtin-uaccess.c +++ b/src/udev/udev-builtin-uaccess.c @@ -5,12 +5,106 @@ #include "sd-login.h" +#include "acl-util.h" #include "device-util.h" -#include "devnode-acl.h" #include "errno-util.h" +#include "fd-util.h" #include "login-util.h" #include "udev-builtin.h" +static int devnode_acl(int fd, uid_t uid) { + bool changed = false, found = false; + int r; + + assert(fd >= 0); + + _cleanup_(acl_freep) acl_t acl = NULL; + acl = acl_get_fd(fd); + if (!acl) + return -errno; + + acl_entry_t entry; + for (r = acl_get_entry(acl, ACL_FIRST_ENTRY, &entry); + r > 0; + r = acl_get_entry(acl, ACL_NEXT_ENTRY, &entry)) { + + acl_tag_t tag; + if (acl_get_tag_type(entry, &tag) < 0) + return -errno; + + if (tag != ACL_USER) + continue; + + if (uid > 0) { + uid_t *u = acl_get_qualifier(entry); + if (!u) + return -errno; + + if (*u == uid) { + acl_permset_t permset; + if (acl_get_permset(entry, &permset) < 0) + return -errno; + + int rd = acl_get_perm(permset, ACL_READ); + if (rd < 0) + return -errno; + + int wt = acl_get_perm(permset, ACL_WRITE); + if (wt < 0) + return -errno; + + if (!rd || !wt) { + if (acl_add_perm(permset, ACL_READ|ACL_WRITE) < 0) + return -errno; + + changed = true; + } + + found = true; + continue; + } + } + + if (acl_delete_entry(acl, entry) < 0) + return -errno; + + changed = true; + } + if (r < 0) + return -errno; + + if (!found && uid > 0) { + if (acl_create_entry(&acl, &entry) < 0) + return -errno; + + if (acl_set_tag_type(entry, ACL_USER) < 0) + return -errno; + + if (acl_set_qualifier(entry, &uid) < 0) + return -errno; + + acl_permset_t permset; + if (acl_get_permset(entry, &permset) < 0) + return -errno; + + if (acl_add_perm(permset, ACL_READ|ACL_WRITE) < 0) + return -errno; + + changed = true; + } + + if (!changed) + return 0; + + if (acl_calc_mask(&acl) < 0) + return -errno; + + if (acl_set_fd(fd, acl) < 0) + return -errno; + + return 0; +} + static int builtin_uaccess(UdevEvent *event, int argc, char *argv[]) { sd_device *dev = ASSERT_PTR(ASSERT_PTR(event)->dev); int r, k; @@ -26,10 +120,9 @@ static int builtin_uaccess(UdevEvent *event, int argc, char *argv[]) { if (!logind_running()) return 0; - const char *node; - r = sd_device_get_devname(dev, &node); - if (r < 0) - return log_device_error_errno(dev, r, "Failed to get device node: %m"); + _cleanup_close_ int fd = sd_device_open(dev, O_CLOEXEC|O_RDWR); + if (fd < 0) + return log_device_error_errno(dev, fd, "Failed to open device node: %m"); const char *seat; r = device_get_seat(dev, &seat); @@ -48,10 +141,7 @@ static int builtin_uaccess(UdevEvent *event, int argc, char *argv[]) { goto reset; } - r = devnode_acl(node, - /* flush = */ true, - /* del = */ false, /* old_uid = */ 0, - /* add = */ true, /* new_uid = */ uid); + r = devnode_acl(fd, uid); if (r < 0) { log_device_full_errno(dev, r == -ENOENT ? LOG_DEBUG : LOG_ERR, r, "Failed to apply ACL: %m"); goto reset; @@ -61,10 +151,7 @@ static int builtin_uaccess(UdevEvent *event, int argc, char *argv[]) { reset: /* Better be safe than sorry and reset ACL */ - k = devnode_acl(node, - /* flush = */ true, - /* del = */ false, /* old_uid = */ 0, - /* add = */ false, /* new_uid = */ 0); + k = devnode_acl(fd, /* uid = */ 0); if (k < 0) RET_GATHER(r, log_device_full_errno(dev, k == -ENOENT ? LOG_DEBUG : LOG_ERR, k, "Failed to flush ACLs: %m")); From 003c6faff4e8b90c56e8153968c9a5cf722b9c73 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Thu, 27 Feb 2025 11:07:17 +0900 Subject: [PATCH 9/9] acl-util: make acl_find_uid() static --- src/shared/acl-util.c | 2 +- src/shared/acl-util.h | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/src/shared/acl-util.c b/src/shared/acl-util.c index db3341d083..b5c8fa0135 100644 --- a/src/shared/acl-util.c +++ b/src/shared/acl-util.c @@ -16,7 +16,7 @@ #if HAVE_ACL -int acl_find_uid(acl_t acl, uid_t uid, acl_entry_t *ret_entry) { +static int acl_find_uid(acl_t acl, uid_t uid, acl_entry_t *ret_entry) { acl_entry_t i; int r; diff --git a/src/shared/acl-util.h b/src/shared/acl-util.h index 3fe3d6c735..8d62e6460f 100644 --- a/src/shared/acl-util.h +++ b/src/shared/acl-util.h @@ -15,7 +15,6 @@ int fd_acl_make_writable_fallback(int fd); #include "macro.h" #include "memory-util.h" -int acl_find_uid(acl_t acl, uid_t uid, acl_entry_t *entry); int calc_acl_mask_if_needed(acl_t *acl_p); int add_base_acls_if_needed(acl_t *acl_p, const char *path); int acl_search_groups(const char* path, char ***ret_groups);