From 6f7530929538b28cb41c289ad9cbced51848fc02 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 26 May 2021 16:07:55 +0200 Subject: [PATCH 1/4] man: document that it is guaranteed that generated ID128 are never all-zero or all-one This is the case because the ID128 we generate are all marked as v4 UUID which requires that some bits are zero and others are one. Let's document this so that people can rely on SD_ID128_NULL being a special value for "uninitialized" that is always distinguishable from generated UUIDs. --- man/sd_id128_get_machine.xml | 16 ++++++++++------ man/sd_id128_randomize.xml | 5 +++-- 2 files changed, 13 insertions(+), 8 deletions(-) diff --git a/man/sd_id128_get_machine.xml b/man/sd_id128_get_machine.xml index 2df4496f19..7fa9860898 100644 --- a/man/sd_id128_get_machine.xml +++ b/man/sd_id128_get_machine.xml @@ -99,12 +99,16 @@ systemd.exec5 for details. The ID is cached internally. In future a different mechanism to determine the invocation ID may be added. - Note that sd_id128_get_machine_app_specific(), sd_id128_get_boot(), - sd_id128_get_boot_app_specific(), and sd_id128_get_invocation() always - return UUID v4 compatible IDs. sd_id128_get_machine() will also return a UUID v4-compatible - ID on new installations but might not on older. It is possible to convert the machine ID into a UUID v4-compatible - one. For more information, see - machine-id5. + Note that sd_id128_get_machine_app_specific(), + sd_id128_get_boot(), sd_id128_get_boot_app_specific(), and + sd_id128_get_invocation() always return UUID v4 compatible IDs. + sd_id128_get_machine() will also return a UUID v4-compatible ID on new installations + but might not on older. It is possible to convert the machine ID into a UUID v4-compatible one. For more + information, see + machine-id5. It is + hence guaranteed that thes functions will never return the ID consisting of all zero or all one bits + (SD_ID128_NULL, SD_ID128_ALLF) — with the possible exception of + sd_id128_get_machine(), as mentioned. For more information about the sd_id128_t type see diff --git a/man/sd_id128_randomize.xml b/man/sd_id128_randomize.xml index cf6ca7726f..0bc1d0abd8 100644 --- a/man/sd_id128_randomize.xml +++ b/man/sd_id128_randomize.xml @@ -42,8 +42,9 @@ /dev/urandom kernel random number generator. - Note that sd_id128_randomize() always - returns a UUID v4-compatible ID. + Note that sd_id128_randomize() always returns a UUID v4-compatible ID. It is + hence guaranteed that this function will never return the ID consisting of all zero or all one bits + (SD_ID128_NULL, SD_ID128_ALLF). For more information about the sd_id128_t type, see From b485fd932ad37a945569857127b7e929e87e17b2 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 26 May 2021 16:09:18 +0200 Subject: [PATCH 2/4] sd-device: add API for triggering synthetic uevents with UUID MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Since kernel 4.13 the kerne allows passing a UUID to generated uevents. Optionally do so via a new sd_device_trigger_with_uuid() call, and add sd_device_get_trigger_uuid() as helper to retrieve the UUID from a uevent we receive. This is useful for tracking uevents through the udev system, and waiting for specific triggers. (Note that the 4.13 patch allows passing arbitrary meta-info into the uevent as well. This does not add an API for that, because I am not convinced it makes sense — as it conflicts with our general rule that events are "stateless" if you so will — and it complicates the interface quite a bit). This replaces #13881 in a way, which added a similar infra, but which stalled, and whose synchronous settling APIs are somewhat problematic and probably not material to merge. --- src/libsystemd/libsystemd.sym | 2 + src/libsystemd/sd-device/sd-device.c | 65 ++++++++++++++++++++++++++++ src/systemd/sd-device.h | 3 ++ 3 files changed, 70 insertions(+) diff --git a/src/libsystemd/libsystemd.sym b/src/libsystemd/libsystemd.sym index 996bf25d6c..1606981e1e 100644 --- a/src/libsystemd/libsystemd.sym +++ b/src/libsystemd/libsystemd.sym @@ -757,4 +757,6 @@ global: sd_device_monitor_filter_add_match_sysattr; sd_device_monitor_filter_add_match_parent; sd_device_get_usec_initialized; + sd_device_trigger_with_uuid; + sd_device_get_trigger_uuid; } LIBSYSTEMD_248; diff --git a/src/libsystemd/sd-device/sd-device.c b/src/libsystemd/sd-device/sd-device.c index 363e4f2dd0..a5bd7d8d84 100644 --- a/src/libsystemd/sd-device/sd-device.c +++ b/src/libsystemd/sd-device/sd-device.c @@ -16,6 +16,7 @@ #include "fileio.h" #include "fs-util.h" #include "hashmap.h" +#include "id128-util.h" #include "macro.h" #include "parse-util.h" #include "path-util.h" @@ -1869,6 +1870,34 @@ _public_ int sd_device_get_property_value(sd_device *device, const char *key, co return 0; } +_public_ int sd_device_get_trigger_uuid(sd_device *device, sd_id128_t *ret) { + const char *s; + sd_id128_t id; + int r; + + assert_return(device, -EINVAL); + + /* Retrieves the UUID attached to a uevent when triggering it from userspace via + * sd_device_trigger_with_uuid() or an equivalent interface. Returns -ENOENT if the record is not + * caused by a synthetic event and -ENODATA if it was but no UUID was specified */ + + r = sd_device_get_property_value(device, "SYNTH_UUID", &s); + if (r < 0) + return r; + + if (streq(s, "0")) /* SYNTH_UUID=0 is set whenever a device is triggered by userspace without specifying a UUID */ + return -ENODATA; + + r = sd_id128_from_string(s, &id); + if (r < 0) + return r; + + if (ret) + *ret = id; + + return 0; +} + static int device_cache_sysattr_value(sd_device *device, const char *key, char *value) { _cleanup_free_ char *new_key = NULL, *old_value = NULL; int r; @@ -2096,5 +2125,41 @@ _public_ int sd_device_trigger(sd_device *device, sd_device_action_t 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); } + +_public_ int sd_device_trigger_with_uuid( + sd_device *device, + sd_device_action_t action, + sd_id128_t *ret_uuid) { + + char buf[ID128_UUID_STRING_MAX]; + const char *s, *j; + sd_id128_t u; + int r; + + assert_return(device, -EINVAL); + + /* If noone 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; + + r = sd_id128_randomize(&u); + if (r < 0) + return r; + + id128_to_uuid_string(u, buf); + j = strjoina(s, " ", buf); + + r = sd_device_set_sysattr_value(device, "uevent", j); + if (r < 0) + return r; + + *ret_uuid = u; + return 0; +} diff --git a/src/systemd/sd-device.h b/src/systemd/sd-device.h index 8b7b361295..04599ea382 100644 --- a/src/systemd/sd-device.h +++ b/src/systemd/sd-device.h @@ -24,6 +24,7 @@ #include #include "sd-event.h" +#include "sd-id128.h" #include "_sd-common.h" @@ -96,11 +97,13 @@ const char *sd_device_get_sysattr_next(sd_device *device); int sd_device_has_tag(sd_device *device, const char *tag); int sd_device_has_current_tag(sd_device *device, const char *tag); int sd_device_get_property_value(sd_device *device, const char *key, const char **value); +int sd_device_get_trigger_uuid(sd_device *device, sd_id128_t *ret); int sd_device_get_sysattr_value(sd_device *device, const char *sysattr, const char **_value); int sd_device_set_sysattr_value(sd_device *device, const char *sysattr, const char *value); int sd_device_set_sysattr_valuef(sd_device *device, const char *sysattr, const char *format, ...) _sd_printf_(3, 4); int sd_device_trigger(sd_device *device, sd_device_action_t action); +int sd_device_trigger_with_uuid(sd_device *device, sd_device_action_t action, sd_id128_t *ret_uuid); /* device enumerator */ From 730b9c1e1469dfd6d6850e9ea06da3cac469eba0 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 26 May 2021 16:13:39 +0200 Subject: [PATCH 3/4] udevadm: make use of the new uuid-enabled triggering for "udevadm trigger" This adds two things: - A new switch --uuid is added to "udevadm trigger". If specified a random UUID is associated with the synthettic uevent and it is printed to stdout. It may then be used manually to match up uevents as they propagate through the system. - The UUID logic is now implicitly enabled if "udevadm trigger --settle" is used, in order to wait for precisely the uevents we actually trigger. Fallback support is kept for pre-4.13 kernels (where the requests for trigger uevents with uuids results in EINVAL). --- man/udevadm.xml | 9 +++ src/basic/hash-funcs.c | 3 + src/basic/hash-funcs.h | 1 + src/udev/udevadm-trigger.c | 118 +++++++++++++++++++++++++++++-------- 4 files changed, 108 insertions(+), 23 deletions(-) diff --git a/man/udevadm.xml b/man/udevadm.xml index 9f89783521..531a88d8a3 100644 --- a/man/udevadm.xml +++ b/man/udevadm.xml @@ -375,6 +375,15 @@ the same command to finish. + + + + Trigger the synthetic device events, and associate a randomized UUID with each. These UUIDs + are printed to standard output, one line for each event. These UUIDs are included in the uevent + environment block (in the SYNTH_UUID= property) and may be used to track + delivery of the generated events. + + diff --git a/src/basic/hash-funcs.c b/src/basic/hash-funcs.c index e033de1ae1..d88df65a0d 100644 --- a/src/basic/hash-funcs.c +++ b/src/basic/hash-funcs.c @@ -57,6 +57,9 @@ void path_hash_func(const char *q, struct siphash *state) { DEFINE_HASH_OPS(path_hash_ops, char, path_hash_func, path_compare); DEFINE_HASH_OPS_WITH_KEY_DESTRUCTOR(path_hash_ops_free, char, path_hash_func, path_compare, free); +DEFINE_HASH_OPS_FULL(path_hash_ops_free_free, + char, path_hash_func, path_compare, free, + void, free); void trivial_hash_func(const void *p, struct siphash *state) { siphash24_compress(&p, sizeof(p), state); diff --git a/src/basic/hash-funcs.h b/src/basic/hash-funcs.h index 5672df1da4..023cfdf530 100644 --- a/src/basic/hash-funcs.h +++ b/src/basic/hash-funcs.h @@ -82,6 +82,7 @@ extern const struct hash_ops string_hash_ops_free_free; void path_hash_func(const char *p, struct siphash *state); extern const struct hash_ops path_hash_ops; extern const struct hash_ops path_hash_ops_free; +extern const struct hash_ops path_hash_ops_free_free; /* This will compare the passed pointers directly, and will not dereference them. This is hence not useful for strings * or suchlike. */ diff --git a/src/udev/udevadm-trigger.c b/src/udev/udevadm-trigger.c index ade92286d4..c75445959e 100644 --- a/src/udev/udevadm-trigger.c +++ b/src/udev/udevadm-trigger.c @@ -24,8 +24,14 @@ static bool arg_verbose = false; static bool arg_dry_run = false; static bool arg_quiet = false; +static bool arg_uuid = false; -static int exec_list(sd_device_enumerator *e, sd_device_action_t action, Set **settle_set) { +static int exec_list( + sd_device_enumerator *e, + sd_device_action_t action, + Hashmap *settle_hashmap) { + + bool skip_uuid_logic = false; const char *action_str; sd_device *d; int r, ret = 0; @@ -33,18 +39,33 @@ static int exec_list(sd_device_enumerator *e, sd_device_action_t action, Set **s action_str = device_action_to_string(action); FOREACH_DEVICE_AND_SUBSYSTEM(e, d) { + sd_id128_t id = SD_ID128_NULL; const char *syspath; - if (sd_device_get_syspath(d, &syspath) < 0) + r = sd_device_get_syspath(d, &syspath); + if (r < 0) { + log_debug_errno(r, "Failed to get syspath of enumerated devices, ignoring: %m"); continue; + } if (arg_verbose) - printf("%s\n", strna(syspath)); + printf("%s\n", syspath); if (arg_dry_run) continue; - r = sd_device_trigger(d, action); + /* 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 || settle_hashmap) && !skip_uuid_logic ? &id : NULL); + if (r == -EINVAL && !arg_uuid && settle_hashmap && !skip_uuid_logic) { + /* 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) + skip_uuid_logic = true; /* dropping the uuid stuff changed the return code, + * hence don't bother next time */ + } 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. @@ -86,10 +107,28 @@ static int exec_list(sd_device_enumerator *e, sd_device_action_t action, Set **s continue; } - if (settle_set) { - r = set_put_strdup(settle_set, syspath); + /* If the user asked for it, write event UUID to stdout */ + if (arg_uuid) + printf(SD_ID128_FORMAT_STR "\n", SD_ID128_FORMAT_VAL(id)); + + if (settle_hashmap) { + _cleanup_free_ sd_id128_t *mid = NULL; + _cleanup_free_ char *sp = NULL; + + sp = strdup(syspath); + if (!sp) + return log_oom(); + + mid = newdup(sd_id128_t, &id, 1); + if (!d) + return log_oom(); + + r = hashmap_put(settle_hashmap, sp, mid); if (r < 0) return log_oom(); + + TAKE_PTR(sp); + TAKE_PTR(mid); } } @@ -97,24 +136,51 @@ static int exec_list(sd_device_enumerator *e, sd_device_action_t action, Set **s } static int device_monitor_handler(sd_device_monitor *m, sd_device *dev, void *userdata) { - _cleanup_free_ char *val = NULL; - Set *settle_set = userdata; + Hashmap *settle_hashmap = userdata; + sd_id128_t *settle_id; const char *syspath; + char *k; + int r; assert(dev); - assert(settle_set); + assert(settle_hashmap); - if (sd_device_get_syspath(dev, &syspath) < 0) + r = sd_device_get_syspath(dev, &syspath); + if (r < 0) { + log_debug_errno(r, "Failed to get syspath of device event, ignoring: %m"); return 0; + } + + settle_id = hashmap_get2(settle_hashmap, syspath, (void**) &k); + if (!settle_id) { + log_debug("Got uevent for unexpected device '%s', ignoring.", syspath); + return 0; + } + if (!sd_id128_is_null(*settle_id)) { /* If this is SD_ID128_NULL then we are on pre-4.13 and have no UUID to check, hence don't */ + sd_id128_t event_id; + + r = sd_device_get_trigger_uuid(dev, &event_id); + if (r < 0) { + log_debug_errno(r, "Got uevent without synthetic UUID for device '%s', ignoring: %m", syspath); + return 0; + } + + if (!sd_id128_equal(event_id, *settle_id)) { + log_debug("Got uevent not matching expected UUID for device '%s', ignoring.", syspath); + return 0; + } + } if (arg_verbose) printf("settle %s\n", syspath); - val = set_remove(settle_set, syspath); - if (!val) - log_debug("Got epoll event on syspath %s not present in syspath set", syspath); + if (arg_uuid) + printf("settle " SD_ID128_FORMAT_STR "\n", SD_ID128_FORMAT_VAL(*settle_id)); - if (set_isempty(settle_set)) + free(hashmap_remove(settle_hashmap, syspath)); + free(k); + + if (hashmap_isempty(settle_hashmap)) return sd_event_exit(sd_device_monitor_get_event(m), 0); return 0; @@ -162,7 +228,8 @@ static int help(void) { " -b --parent-match=NAME Trigger devices with that parent device\n" " -w --settle Wait for the triggered events to complete\n" " --wait-daemon[=SECONDS] Wait for udevd daemon to be initialized\n" - " before triggering uevents\n", + " before triggering uevents\n" + " --uuid Print synthetic uevent UUID\n", program_invocation_short_name); return 0; @@ -172,6 +239,7 @@ int trigger_main(int argc, char *argv[], void *userdata) { enum { ARG_NAME = 0x100, ARG_PING, + ARG_UUID, }; static const struct option options[] = { @@ -193,6 +261,7 @@ int trigger_main(int argc, char *argv[], void *userdata) { { "wait-daemon", optional_argument, NULL, ARG_PING }, { "version", no_argument, NULL, 'V' }, { "help", no_argument, NULL, 'h' }, + { "uuid", no_argument, NULL, ARG_UUID }, {} }; enum { @@ -203,7 +272,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_set = NULL; + _cleanup_hashmap_free_ Hashmap *settle_hashmap = NULL; usec_t ping_timeout_usec = 5 * USEC_PER_SEC; bool settle = false, ping = false; int c, r; @@ -327,7 +396,7 @@ int trigger_main(int argc, char *argv[], void *userdata) { break; } - case ARG_PING: { + case ARG_PING: ping = true; if (optarg) { r = parse_sec(optarg, &ping_timeout_usec); @@ -335,7 +404,10 @@ int trigger_main(int argc, char *argv[], void *userdata) { log_error_errno(r, "Failed to parse timeout value '%s', ignoring: %m", optarg); } break; - } + + case ARG_UUID: + arg_uuid = true; + break; case 'V': return print_version(); @@ -377,8 +449,8 @@ int trigger_main(int argc, char *argv[], void *userdata) { } if (settle) { - settle_set = set_new(&string_hash_ops_free); - if (!settle_set) + settle_hashmap = hashmap_new(&path_hash_ops_free_free); + if (!settle_hashmap) return log_oom(); r = sd_event_default(&event); @@ -393,7 +465,7 @@ 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_set); + r = sd_device_monitor_start(m, device_monitor_handler, settle_hashmap); if (r < 0) return log_error_errno(r, "Failed to start device monitor: %m"); } @@ -413,11 +485,11 @@ int trigger_main(int argc, char *argv[], void *userdata) { assert_not_reached("Unknown device type"); } - r = exec_list(e, action, settle ? &settle_set : NULL); + r = exec_list(e, action, settle_hashmap); if (r < 0) return r; - if (event && !set_isempty(settle_set)) { + if (event && !hashmap_isempty(settle_hashmap)) { r = sd_event_loop(event); if (r < 0) return log_error_errno(r, "Event loop failed: %m"); From 74bbc85ca68e2845534620bd0e60c9a6ee152c06 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 26 May 2021 21:40:43 +0200 Subject: [PATCH 4/4] hash-func: change value type of string_hash_ops_free_free to void* The generic string_hash_ops_free_free hash operations vtable currently assumes the data pointer is of type char*. There's really no reason to assume that though, we regularly store non-string data as value in a hashmap. Hence, to accomodate for that, use void* as pointer for the value (and keep char* for the key, as that's what string_hash_ops_free_free is for, after all). --- src/basic/hash-funcs.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/basic/hash-funcs.c b/src/basic/hash-funcs.c index d88df65a0d..608131a1c1 100644 --- a/src/basic/hash-funcs.c +++ b/src/basic/hash-funcs.c @@ -14,7 +14,7 @@ DEFINE_HASH_OPS_WITH_KEY_DESTRUCTOR(string_hash_ops_free, char, string_hash_func, string_compare_func, free); DEFINE_HASH_OPS_FULL(string_hash_ops_free_free, char, string_hash_func, string_compare_func, free, - char, free); + void, free); void path_hash_func(const char *q, struct siphash *state) { size_t n;