From df7cef094071d9bb4930990f8b48f77b222607ef Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Wed, 26 Feb 2025 02:56:47 +0900 Subject: [PATCH 1/4] udev-util: drop unnecessary inclusion of missing_threads.h Follow-up for a3df693799499a26735acc1f0c4f1b5d1f182fa7. --- src/shared/udev-util.c | 1 - 1 file changed, 1 deletion(-) diff --git a/src/shared/udev-util.c b/src/shared/udev-util.c index 15996ca724..a6956f5865 100644 --- a/src/shared/udev-util.c +++ b/src/shared/udev-util.c @@ -14,7 +14,6 @@ #include "id128-util.h" #include "log.h" #include "macro.h" -#include "missing_threads.h" #include "parse-util.h" #include "path-util.h" #include "signal-util.h" From 0e1c87b4aac3b7db398a3dbc9842d10f38c0f594 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Thu, 20 Feb 2025 02:09:11 +0900 Subject: [PATCH 2/4] udev-builtin-uaccess: modernize code No functional change, just refactoring. --- src/udev/udev-builtin-uaccess.c | 55 ++++++++++++++------------------- 1 file changed, 23 insertions(+), 32 deletions(-) diff --git a/src/udev/udev-builtin-uaccess.c b/src/udev/udev-builtin-uaccess.c index 805d048e81..7fbd91a5c2 100644 --- a/src/udev/udev-builtin-uaccess.c +++ b/src/udev/udev-builtin-uaccess.c @@ -3,26 +3,17 @@ * manage device node user ACL */ -#include -#include -#include -#include - #include "sd-login.h" #include "device-util.h" #include "devnode-acl.h" #include "errno-util.h" #include "login-util.h" -#include "log.h" #include "udev-builtin.h" static int builtin_uaccess(UdevEvent *event, int argc, char *argv[]) { sd_device *dev = ASSERT_PTR(ASSERT_PTR(event)->dev); - const char *path = NULL, *seat; - bool changed_acl = false; - uid_t uid; - int r; + int r, k; if (event->event_mode != EVENT_UDEV_WORKER) { log_device_debug(dev, "Running in test mode, skipping execution of 'uaccess' builtin command."); @@ -35,16 +26,17 @@ static int builtin_uaccess(UdevEvent *event, int argc, char *argv[]) { if (!logind_running()) return 0; - r = sd_device_get_devname(dev, &path); - if (r < 0) { - log_device_error_errno(dev, r, "Failed to get device name: %m"); - goto finish; - } + 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"); + const char *seat; if (sd_device_get_property_value(dev, "ID_SEAT", &seat) < 0) seat = "seat0"; - r = sd_seat_get_active(seat, NULL, &uid); + uid_t uid; + r = sd_seat_get_active(seat, /* ret_session = */ NULL, &uid); if (r < 0) { if (IN_SET(r, -ENXIO, -ENODATA)) /* No active session on this seat */ @@ -52,29 +44,28 @@ static int builtin_uaccess(UdevEvent *event, int argc, char *argv[]) { else log_device_error_errno(dev, r, "Failed to determine active user on seat %s: %m", seat); - goto finish; + goto reset; } - r = devnode_acl(path, true, false, 0, true, uid); + r = devnode_acl(node, + /* flush = */ true, + /* del = */ false, /* old_uid = */ 0, + /* add = */ true, /* new_uid = */ uid); if (r < 0) { log_device_full_errno(dev, r == -ENOENT ? LOG_DEBUG : LOG_ERR, r, "Failed to apply ACL: %m"); - goto finish; + goto reset; } - changed_acl = true; - r = 0; + return 0; -finish: - if (path && !changed_acl) { - int k; - - /* Better be safe than sorry and reset ACL */ - k = devnode_acl(path, true, false, 0, false, 0); - if (k < 0) { - log_device_full_errno(dev, k == -ENOENT ? LOG_DEBUG : LOG_ERR, k, "Failed to apply ACL: %m"); - RET_GATHER(r, k); - } - } +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); + if (k < 0) + RET_GATHER(r, log_device_full_errno(dev, k == -ENOENT ? LOG_DEBUG : LOG_ERR, k, "Failed to flush ACLs: %m")); return r; } From fde9f2bc4857b4d34cdb89f93c52c10f7bcf90de Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Wed, 26 Feb 2025 03:13:09 +0900 Subject: [PATCH 3/4] udevadm-trigger: drop support of kernels order than 4.13 Now our kernel baseline is 5.4, hence we can always write action string with a synthetic UUID. --- README | 2 +- src/udev/udevadm-trigger.c | 152 +++++++++++-------------------------- 2 files changed, 45 insertions(+), 109 deletions(-) diff --git a/README b/README index 346f1aad42..5194faae5e 100644 --- a/README +++ b/README @@ -37,7 +37,7 @@ REQUIREMENTS: ≥ 4.9 for RENAME_NOREPLACE support in vfat ≥ 4.10 for cgroup-bpf egress and ingress hooks ≥ 4.11 for nsfs NS_GET_NSTYPE - ≥ 4.13 for TIOCGPTPEER + ≥ 4.13 for TIOCGPTPEER and SYNTH_UUID= property support in uevent ≥ 4.15 for cgroup-bpf device hook and cpu controller in cgroup v2 ≥ 4.17 for cgroup-bpf socket address hooks, /sys/power/resume_offset, and FRA_PROTOCOL attribute for fib rules diff --git a/src/udev/udevadm-trigger.c b/src/udev/udevadm-trigger.c index 8ab4b49789..860335c8d5 100644 --- a/src/udev/udevadm-trigger.c +++ b/src/udev/udevadm-trigger.c @@ -32,22 +32,17 @@ static bool arg_settle = false; static int exec_list( sd_device_enumerator *e, sd_device_action_t action, - Set **ret_settle_path_or_ids) { + Set *settle_ids) { - _cleanup_set_free_ Set *settle_path_or_ids = NULL; - int uuid_supported = -1; - const char *action_str; - sd_device *d; + const char *action_str = device_action_to_string(action); int r, ret = 0; assert(e); - action_str = device_action_to_string(action); - + sd_device *d; FOREACH_DEVICE_AND_SUBSYSTEM(e, d) { - sd_id128_t id = SD_ID128_NULL; - const char *syspath; + const char *syspath; r = sd_device_get_syspath(d, &syspath); if (r < 0) { log_debug_errno(r, "Failed to get syspath of enumerated devices, ignoring: %m"); @@ -60,18 +55,8 @@ static int exec_list( if (arg_dry_run) continue; - /* Use the UUID mode if the user explicitly asked for it, or if --settle has been specified, - * so that we can recognize our own uevent. */ - r = sd_device_trigger_with_uuid(d, action, (arg_uuid || arg_settle) && uuid_supported != 0 ? &id : NULL); - if (r == -EINVAL && !arg_uuid && arg_settle && uuid_supported < 0) { - /* If we specified a UUID because of the settling logic, and we got EINVAL this might - * be caused by an old kernel which doesn't know the UUID logic (pre-4.13). Let's try - * if it works without the UUID logic then. */ - r = sd_device_trigger(d, action); - if (r != -EINVAL) - uuid_supported = false; /* dropping the uuid stuff changed the return code, - * hence don't bother next time */ - } + sd_id128_t id; + r = sd_device_trigger_with_uuid(d, action, &id); if (r < 0) { /* ENOENT may be returned when a device does not have /uevent or is already * removed. Hence, this is logged at debug level and ignored. @@ -108,117 +93,63 @@ static int exec_list( if (r == -EROFS) return r; - if (ret == 0 && !ignore) - ret = r; + if (!ignore) + RET_GATHER(ret, r); continue; } else log_device_debug(d, "Triggered device with action '%s'.", action_str); - if (uuid_supported < 0) - uuid_supported = true; - /* If the user asked for it, write event UUID to stdout */ if (arg_uuid) printf(SD_ID128_UUID_FORMAT_STR "\n", SD_ID128_FORMAT_VAL(id)); - if (arg_settle) { - if (uuid_supported) { - sd_id128_t *dup; + if (settle_ids) { + sd_id128_t *dup = newdup(sd_id128_t, &id, 1); + if (!dup) + return log_oom(); - dup = newdup(sd_id128_t, &id, 1); - if (!dup) - return log_oom(); - - r = set_ensure_consume(&settle_path_or_ids, &id128_hash_ops_free, dup); - } else { - char *dup; - - dup = strdup(syspath); - if (!dup) - return log_oom(); - - r = set_ensure_consume(&settle_path_or_ids, &path_hash_ops_free, dup); - } + r = set_consume(settle_ids, dup); if (r < 0) return log_oom(); } } - if (ret_settle_path_or_ids) - *ret_settle_path_or_ids = TAKE_PTR(settle_path_or_ids); - return ret; } static int device_monitor_handler(sd_device_monitor *m, sd_device *dev, void *userdata) { - Set *settle_path_or_ids = * (Set**) ASSERT_PTR(userdata); - const char *syspath; - sd_id128_t id; + Set *settle_ids = ASSERT_PTR(userdata); int r; assert(dev); - r = sd_device_get_syspath(dev, &syspath); + sd_id128_t id; + r = sd_device_get_trigger_uuid(dev, &id); if (r < 0) { - log_device_debug_errno(dev, r, "Failed to get syspath of device event, ignoring: %m"); + log_device_debug_errno(dev, r, "Got uevent without UUID, ignoring: %m"); return 0; } - if (sd_device_get_trigger_uuid(dev, &id) >= 0) { - _cleanup_free_ sd_id128_t *saved = NULL; - - saved = set_remove(settle_path_or_ids, &id); - if (!saved) { - log_device_debug(dev, "Got uevent not matching expected UUID, ignoring."); - return 0; - } - } else { - _cleanup_free_ char *saved = NULL; - - saved = set_remove(settle_path_or_ids, syspath); - if (!saved) { - const char *old_sysname; - - /* When the device is renamed, the new name is broadcast, and the old name is saved - * in INTERFACE_OLD. - * - * TODO: remove support for INTERFACE_OLD when kernel baseline is bumped to 4.13 or - * higher. See 1193448cb68e5a90cab027e16a093bbd367e9494. - */ - - if (sd_device_get_property_value(dev, "INTERFACE_OLD", &old_sysname) >= 0) { - _cleanup_free_ char *dir = NULL, *old_syspath = NULL; - - r = path_extract_directory(syspath, &dir); - if (r < 0) { - log_device_debug_errno(dev, r, - "Failed to extract directory from '%s', ignoring: %m", - syspath); - return 0; - } - - old_syspath = path_join(dir, old_sysname); - if (!old_syspath) { - log_oom_debug(); - return 0; - } - - saved = set_remove(settle_path_or_ids, old_syspath); - } - } - if (!saved) { - log_device_debug(dev, "Got uevent for unexpected device, ignoring."); - return 0; - } + _cleanup_free_ sd_id128_t *saved = set_remove(settle_ids, &id); + if (!saved) { + log_device_debug(dev, "Got uevent with unexpected UUID, ignoring."); + return 0; } - if (arg_verbose) - printf("settle %s\n", syspath); + if (arg_verbose) { + const char *syspath; + + r = sd_device_get_syspath(dev, &syspath); + if (r < 0) + log_device_debug_errno(dev, r, "Failed to get syspath of device event, ignoring: %m"); + else + printf("settle %s\n", syspath); + } if (arg_uuid) printf("settle " SD_ID128_UUID_FORMAT_STR "\n", SD_ID128_FORMAT_VAL(id)); - if (set_isempty(settle_path_or_ids)) + if (set_isempty(settle_ids)) return sd_event_exit(sd_device_monitor_get_event(m), 0); return 0; @@ -325,7 +256,7 @@ int trigger_main(int argc, char *argv[], void *userdata) { _cleanup_(sd_device_enumerator_unrefp) sd_device_enumerator *e = NULL; _cleanup_(sd_device_monitor_unrefp) sd_device_monitor *m = NULL; _cleanup_(sd_event_unrefp) sd_event *event = NULL; - _cleanup_set_free_ Set *settle_path_or_ids = NULL; + _cleanup_set_free_ Set *settle_ids = NULL; usec_t ping_timeout_usec = 5 * USEC_PER_SEC; bool ping = false; int c, r; @@ -526,7 +457,11 @@ int trigger_main(int argc, char *argv[], void *userdata) { if (r < 0) return log_error_errno(r, "Failed to attach event to device monitor: %m"); - r = sd_device_monitor_start(m, device_monitor_handler, &settle_path_or_ids); + settle_ids = set_new(&id128_hash_ops_free); + if (!settle_ids) + return log_oom(); + + r = sd_device_monitor_start(m, device_monitor_handler, settle_ids); if (r < 0) return log_error_errno(r, "Failed to start device monitor: %m"); } @@ -551,15 +486,16 @@ int trigger_main(int argc, char *argv[], void *userdata) { assert_not_reached(); } - r = exec_list(e, action, arg_settle ? &settle_path_or_ids : NULL); + r = exec_list(e, action, settle_ids); if (r < 0) return r; - if (!set_isempty(settle_path_or_ids)) { - r = sd_event_loop(event); - if (r < 0) - return log_error_errno(r, "Event loop failed: %m"); - } + if (set_isempty(settle_ids)) + return 0; + + r = sd_event_loop(event); + if (r < 0) + return log_error_errno(r, "Event loop failed: %m"); return 0; } From 2c051721ec156160997cc9598c9e9aeeaed86d4f Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Wed, 26 Feb 2025 03:33:16 +0900 Subject: [PATCH 4/4] sd-device: always pass random UUID on triggering uevent Then, this makes sd_device_trigger() a simple wrapper of sd_device_trigger_with_uuid(). --- src/libsystemd/sd-device/sd-device.c | 18 +++--------------- 1 file changed, 3 insertions(+), 15 deletions(-) diff --git a/src/libsystemd/sd-device/sd-device.c b/src/libsystemd/sd-device/sd-device.c index ac441a7f38..181a9256dd 100644 --- a/src/libsystemd/sd-device/sd-device.c +++ b/src/libsystemd/sd-device/sd-device.c @@ -2692,16 +2692,7 @@ _public_ int sd_device_set_sysattr_valuef(sd_device *device, const char *sysattr } _public_ int sd_device_trigger(sd_device *device, sd_device_action_t action) { - const char *s; - - assert_return(device, -EINVAL); - - s = device_action_to_string(action); - if (!s) - return -EINVAL; - - /* This uses the simple no-UUID interface of kernel < 4.13 */ - return sd_device_set_sysattr_value(device, "uevent", s); + return sd_device_trigger_with_uuid(device, action, NULL); } _public_ int sd_device_trigger_with_uuid( @@ -2715,10 +2706,6 @@ _public_ int sd_device_trigger_with_uuid( assert_return(device, -EINVAL); - /* If no one wants to know the UUID, use the simple interface from pre-4.13 times */ - if (!ret_uuid) - return sd_device_trigger(device, action); - s = device_action_to_string(action); if (!s) return -EINVAL; @@ -2733,7 +2720,8 @@ _public_ int sd_device_trigger_with_uuid( if (r < 0) return r; - *ret_uuid = u; + if (ret_uuid) + *ret_uuid = u; return 0; }