From ea8213dc47312e6091b38db2a783c594ebee4f5f Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Fri, 15 Apr 2022 06:08:13 +0900 Subject: [PATCH 1/8] udev: not necessary to return 1 from on_inotify() --- src/udev/udevd.c | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/src/udev/udevd.c b/src/udev/udevd.c index c2a4a8a7bd..2574844ed7 100644 --- a/src/udev/udevd.c +++ b/src/udev/udevd.c @@ -1430,17 +1430,15 @@ static int synthesize_change(sd_device *dev) { } static int on_inotify(sd_event_source *s, int fd, uint32_t revents, void *userdata) { - Manager *manager = userdata; + Manager *manager = ASSERT_PTR(userdata); union inotify_event_buffer buffer; ssize_t l; int r; - assert(manager); - l = read(fd, &buffer, sizeof(buffer)); if (l < 0) { if (ERRNO_IS_TRANSIENT(errno)) - return 1; + return 0; return log_error_errno(errno, "Failed to read inotify fd: %m"); } @@ -1468,7 +1466,7 @@ static int on_inotify(sd_event_source *s, int fd, uint32_t revents, void *userda * udev_event_execute_rules() -> event_execute_rules_on_remove() -> udev_watch_end(). */ } - return 1; + return 0; } static int on_sigterm(sd_event_source *s, const struct signalfd_siginfo *si, void *userdata) { From cd66f972d105c443f4ab89cc9d3406a2fc88f974 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Fri, 15 Apr 2022 09:42:15 +0900 Subject: [PATCH 2/8] udev: ignore IN_IGNORED inotify event earlier --- src/udev/udevd.c | 26 ++++++++++++++++++-------- 1 file changed, 18 insertions(+), 8 deletions(-) diff --git a/src/udev/udevd.c b/src/udev/udevd.c index 2574844ed7..25496975d5 100644 --- a/src/udev/udevd.c +++ b/src/udev/udevd.c @@ -1447,23 +1447,33 @@ static int on_inotify(sd_event_source *s, int fd, uint32_t revents, void *userda _cleanup_(sd_device_unrefp) sd_device *dev = NULL; const char *devnode; + /* Do not handle IN_IGNORED here. Especially, do not try to call udev_watch_end() from the + * main process. Otherwise, the pair of the symlinks may become inconsistent, and several + * garbage may remain. The old symlinks are removed by a worker that processes the + * corresponding 'remove' uevent; + * udev_event_execute_rules() -> event_execute_rules_on_remove() -> udev_watch_end(). */ + + if (!FLAGS_SET(e->mask, IN_CLOSE_WRITE)) + continue; + r = device_new_from_watch_handle(&dev, e->wd); if (r < 0) { + /* Device may be removed just after closed. */ log_debug_errno(r, "Failed to create sd_device object from watch handle, ignoring: %m"); continue; } - if (sd_device_get_devname(dev, &devnode) < 0) + r = sd_device_get_devname(dev, &devnode); + if (r < 0) { + /* Also here, device may be already removed. */ + log_device_debug_errno(dev, r, "Failed to get device node, ignoring: %m"); continue; - - log_device_debug(dev, "Inotify event: %x for %s", e->mask, devnode); - if (e->mask & IN_CLOSE_WRITE) { - (void) event_queue_assume_block_device_unlocked(manager, dev); - (void) synthesize_change(dev); } - /* Do not handle IN_IGNORED here. It should be handled by worker in 'remove' uevent; - * udev_event_execute_rules() -> event_execute_rules_on_remove() -> udev_watch_end(). */ + log_device_debug(dev, "Received inotify event for %s.", devnode); + + (void) event_queue_assume_block_device_unlocked(manager, dev); + (void) synthesize_change(dev); } return 0; From 691a596da15cb4171a86c5f95b30ad5ba91b6745 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Fri, 15 Apr 2022 06:31:21 +0900 Subject: [PATCH 3/8] udev-watch: remove symlink for saving inotify watch handle only when it is owned by the processing device Before removing symlinks that stores watch handles, this makes udev worker check if the symlink is owned by the processing device. Then, we can avoid TOCTOU and drop the try-and-wait loop. This partially reverts 2d3af41f0e837390b734253f5c4a99a9f33c53e3. --- src/udev/udev-watch.c | 176 +++++++++++++++++++++++++++++------------- 1 file changed, 121 insertions(+), 55 deletions(-) diff --git a/src/udev/udev-watch.c b/src/udev/udev-watch.c index a8b7d2d2d1..21007dee8b 100644 --- a/src/udev/udev-watch.c +++ b/src/udev/udev-watch.c @@ -10,15 +10,15 @@ #include "device-private.h" #include "device-util.h" #include "dirent-util.h" +#include "fd-util.h" #include "fs-util.h" +#include "mkdir.h" #include "parse-util.h" -#include "random-util.h" +#include "rm-rf.h" +#include "stdio-util.h" +#include "string-util.h" #include "udev-watch.h" -#define SAVE_WATCH_HANDLE_MAX_RETRIES 128 -#define MAX_RANDOM_DELAY (100 * USEC_PER_MSEC) -#define MIN_RANDOM_DELAY ( 10 * USEC_PER_MSEC) - int udev_watch_restore(int inotify_fd) { DIR *dir; int r; @@ -73,8 +73,79 @@ unlink: return 0; } +static int udev_watch_clear(sd_device *dev, int dirfd, int *ret_wd) { + _cleanup_free_ char *wd_str = NULL, *buf = NULL; + const char *id; + int wd = -1, r; + + assert(dev); + assert(dirfd >= 0); + + r = device_get_device_id(dev, &id); + if (r < 0) + return log_device_debug_errno(dev, r, "Failed to get device ID: %m"); + + /* 1. read symlink ID -> wd */ + r = readlinkat_malloc(dirfd, id, &wd_str); + if (r == -ENOENT) { + if (ret_wd) + *ret_wd = -1; + return 0; + } + if (r < 0) { + log_device_debug_errno(dev, r, "Failed to read symlink '/run/udev/watch/%s': %m", id); + goto finalize; + } + + r = safe_atoi(wd_str, &wd); + if (r < 0) { + log_device_debug_errno(dev, r, "Failed to parse watch handle from symlink '/run/udev/watch/%s': %m", id); + goto finalize; + } + + if (wd < 0) { + r = log_device_debug_errno(dev, SYNTHETIC_ERRNO(EBADF), "Invalid watch handle %i.", wd); + goto finalize; + } + + /* 2. read symlink wd -> ID */ + r = readlinkat_malloc(dirfd, wd_str, &buf); + if (r < 0) { + log_device_debug_errno(dev, r, "Failed to read symlink '/run/udev/watch/%s': %m", wd_str); + goto finalize; + } + + /* 3. check if the symlink wd -> ID is owned by the device. */ + if (!streq(buf, id)) { + r = log_device_debug_errno(dev, SYNTHETIC_ERRNO(ENOENT), + "Symlink '/run/udev/watch/%s' is owned by another device '%s'.", wd_str, buf); + goto finalize; + } + + /* 4. remove symlink wd -> ID. + * In the above, we already confirmed that the symlink is owned by us. Hence, no other workers remove + * the symlink and cannot create a new symlink with the same filename but to a different ID. Hence, + * the removal below is safe even the steps in this function are not atomic. */ + if (unlinkat(dirfd, wd_str, 0) < 0 && errno != -ENOENT) + log_device_debug_errno(dev, errno, "Failed to remove '/run/udev/watch/%s', ignoring: %m", wd_str); + + if (ret_wd) + *ret_wd = wd; + r = 0; + +finalize: + /* 5. remove symlink ID -> wd. + * The file is always owned by the device. Hence, it is safe to remove it unconditionally. */ + if (unlinkat(dirfd, id, 0) < 0 && errno != -ENOENT) + log_device_debug_errno(dev, errno, "Failed to remove '/run/udev/watch/%s': %m", id); + + return r; +} + int udev_watch_begin(int inotify_fd, sd_device *dev) { - const char *devnode; + char wd_str[DECIMAL_STR_MAX(int)]; + _cleanup_close_ int dirfd = -1; + const char *devnode, *id; int wd, r; assert(inotify_fd >= 0); @@ -82,62 +153,51 @@ int udev_watch_begin(int inotify_fd, sd_device *dev) { r = sd_device_get_devname(dev, &devnode); if (r < 0) - return log_device_debug_errno(dev, r, "Failed to get device name: %m"); + return log_device_debug_errno(dev, r, "Failed to get device node: %m"); + r = device_get_device_id(dev, &id); + if (r < 0) + return log_device_debug_errno(dev, r, "Failed to get device ID: %m"); + + r = dirfd = open_mkdir_at(AT_FDCWD, "/run/udev/watch", O_CLOEXEC | O_RDONLY, 0755); + if (r < 0) + return log_device_debug_errno(dev, r, "Failed to create and open '/run/udev/watch/': %m"); + + /* 1. Clear old symlinks */ + (void) udev_watch_clear(dev, dirfd, NULL); + + /* 2. Add inotify watch */ log_device_debug(dev, "Adding watch on '%s'", devnode); wd = inotify_add_watch(inotify_fd, devnode, IN_CLOSE_WRITE); - if (wd < 0) { - bool ignore = errno == ENOENT; + if (wd < 0) + return log_device_debug_errno(dev, errno, "Failed to watch device node '%s': %m", devnode); - r = log_device_full_errno(dev, ignore ? LOG_DEBUG : LOG_WARNING, errno, - "Failed to add device '%s' to watch%s: %m", - devnode, ignore ? ", ignoring" : ""); + xsprintf(wd_str, "%d", wd); - (void) device_set_watch_handle(dev, -1); - return ignore ? 0 : r; + /* 3. Create new symlinks */ + if (symlinkat(wd_str, dirfd, id) < 0) { + r = log_device_debug_errno(dev, errno, "Failed to create symlink '/run/udev/watch/%s' to '%s': %m", id, wd_str); + goto on_failure; } - for (unsigned i = 0; i < SAVE_WATCH_HANDLE_MAX_RETRIES; i++) { - if (i > 0) { - usec_t delay = MIN_RANDOM_DELAY + random_u64_range(MAX_RANDOM_DELAY - MIN_RANDOM_DELAY); - - /* When the same handle is reused for different device node, we may fail to - * save the watch handle with -EEXIST. Let's consider the case of two workers A - * and B do the following: - * - * 1. A calls inotify_rm_watch() - * 2. B calls inotify_add_watch() - * 3. B calls device_set_watch_handle() - * 4. A calls device_set_watch_handle(-1) - * - * At step 3, the old symlinks to save the watch handle still exist. So, - * device_set_watch_handle() fails with -EEXIST. */ - - log_device_debug_errno(dev, r, - "Failed to save watch handle '%i' for %s in " - "/run/udev/watch, retrying in after %s: %m", - wd, devnode, FORMAT_TIMESPAN(delay, USEC_PER_MSEC)); - (void) usleep(delay); - } - - r = device_set_watch_handle(dev, wd); - if (r >= 0) - return 0; - if (r != -EEXIST) - break; + if (symlinkat(id, dirfd, wd_str) < 0) { + /* Possibly, the watch handle is previously assigned to another device, and udev_watch_end() + * is not called for the device yet. */ + r = log_device_debug_errno(dev, errno, "Failed to create symlink '/run/udev/watch/%s' to '%s': %m", wd_str, id); + goto on_failure; } - log_device_warning_errno(dev, r, - "Failed to save watch handle '%i' for %s in /run/udev/watch: %m", - wd, devnode); + return 0; +on_failure: + (void) unlinkat(dirfd, id, 0); (void) inotify_rm_watch(inotify_fd, wd); - return r; } int udev_watch_end(int inotify_fd, sd_device *dev) { - int wd; + _cleanup_close_ int dirfd = -1; + int wd, r; assert(dev); @@ -148,14 +208,20 @@ int udev_watch_end(int inotify_fd, sd_device *dev) { if (sd_device_get_devname(dev, NULL) < 0) return 0; - wd = device_get_watch_handle(dev); - if (wd < 0) - log_device_debug_errno(dev, wd, "Failed to get watch handle, ignoring: %m"); - else { - log_device_debug(dev, "Removing watch"); - (void) inotify_rm_watch(inotify_fd, wd); - } - (void) device_set_watch_handle(dev, -1); + dirfd = RET_NERRNO(open("/run/udev/watch", O_CLOEXEC | O_DIRECTORY | O_NOFOLLOW | O_RDONLY)); + if (dirfd == -ENOENT) + return 0; + if (dirfd < 0) + return log_device_debug_errno(dev, dirfd, "Failed to open '/run/udev/watch/': %m"); + + /* First, clear symlinks. */ + r = udev_watch_clear(dev, dirfd, &wd); + if (r < 0) + return r; + + /* Then, remove inotify watch. */ + log_device_debug(dev, "Removing watch handle %i.", wd); + (void) inotify_rm_watch(inotify_fd, wd); return 0; } From 2369152394b41f1fdfc23184ffcc67b29e1c3d13 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Thu, 28 Apr 2022 15:54:06 +0900 Subject: [PATCH 4/8] udev: use rm_rf() to remove old watch directory --- src/udev/udev-watch.c | 56 ++++++++++++++++++++++++------------------- 1 file changed, 31 insertions(+), 25 deletions(-) diff --git a/src/udev/udev-watch.c b/src/udev/udev-watch.c index 21007dee8b..0cc83ddb9c 100644 --- a/src/udev/udev-watch.c +++ b/src/udev/udev-watch.c @@ -20,57 +20,63 @@ #include "udev-watch.h" int udev_watch_restore(int inotify_fd) { - DIR *dir; + _cleanup_closedir_ DIR *dir = NULL; int r; /* Move any old watches directory out of the way, and then restore the watches. */ assert(inotify_fd >= 0); - if (rename("/run/udev/watch", "/run/udev/watch.old") < 0) { - if (errno != ENOENT) - return log_warning_errno(errno, "Failed to move watches directory /run/udev/watch. " - "Old watches will not be restored: %m"); + rm_rf("/run/udev/watch.old", REMOVE_ROOT); - return 0; + if (rename("/run/udev/watch", "/run/udev/watch.old") < 0) { + if (errno == ENOENT) + return 0; + + r = log_warning_errno(errno, + "Failed to move watches directory '/run/udev/watch/'. " + "Old watches will not be restored: %m"); + goto finalize; } dir = opendir("/run/udev/watch.old"); - if (!dir) - return log_warning_errno(errno, "Failed to open old watches directory /run/udev/watch.old. " - "Old watches will not be restored: %m"); + if (!dir) { + r = log_warning_errno(errno, + "Failed to open old watches directory '/run/udev/watch.old/'. " + "Old watches will not be restored: %m"); + goto finalize; + } - FOREACH_DIRENT_ALL(ent, dir, break) { + FOREACH_DIRENT_ALL(de, dir, break) { _cleanup_(sd_device_unrefp) sd_device *dev = NULL; int wd; - if (ent->d_name[0] == '.') + /* For backward compatibility, read symlink from watch handle to device ID. This is necessary + * when udevd is restarted after upgrading from v248 or older. The new format (ID -> wd) was + * introduced by e7f781e473f5119bf9246208a6de9f6b76a39c5d (v249). */ + + if (dot_or_dot_dot(de->d_name)) continue; - /* For backward compatibility, read symlink from watch handle to device id, and ignore - * the opposite direction symlink. */ - - if (safe_atoi(ent->d_name, &wd) < 0) - goto unlink; + if (safe_atoi(de->d_name, &wd) < 0) + continue; r = device_new_from_watch_handle_at(&dev, dirfd(dir), wd); if (r < 0) { log_full_errno(r == -ENODEV ? LOG_DEBUG : LOG_WARNING, r, - "Failed to create sd_device object from saved watch handle '%s', ignoring: %m", - ent->d_name); - goto unlink; + "Failed to create sd_device object from saved watch handle '%i', ignoring: %m", + wd); + continue; } - log_device_debug(dev, "Restoring old watch"); (void) udev_watch_begin(inotify_fd, dev); -unlink: - (void) unlinkat(dirfd(dir), ent->d_name, 0); } - (void) closedir(dir); - (void) rmdir("/run/udev/watch.old"); + r = 0; - return 0; +finalize: + (void) rm_rf("/run/udev/watch.old", REMOVE_ROOT); + return r; } static int udev_watch_clear(sd_device *dev, int dirfd, int *ret_wd) { From 4443b1857be367b492cf74523e0842125d28ce64 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Thu, 28 Apr 2022 16:39:49 +0900 Subject: [PATCH 5/8] udev: drop unnecessary call of udev_watch_end() As it is already called by udev_event_execute_rules(). --- src/udev/udev-event.c | 16 ++++++---------- src/udev/udev-event.h | 2 +- src/udev/udevd.c | 4 +--- 3 files changed, 8 insertions(+), 14 deletions(-) diff --git a/src/udev/udev-event.c b/src/udev/udev-event.c index 9b53105199..62c6addb9a 100644 --- a/src/udev/udev-event.c +++ b/src/udev/udev-event.c @@ -1151,23 +1151,19 @@ void udev_event_execute_run(UdevEvent *event, usec_t timeout_usec, int timeout_s } } -int udev_event_process_inotify_watch(UdevEvent *event, int inotify_fd) { +void udev_event_process_inotify_watch(UdevEvent *event, int inotify_fd) { sd_device *dev; assert(event); assert(inotify_fd >= 0); - dev = event->dev; + dev = ASSERT_PTR(event->dev); - assert(dev); + if (!event->inotify_watch) + return; if (device_for_action(dev, SD_DEVICE_REMOVE)) - return 0; + return; - if (event->inotify_watch) - (void) udev_watch_begin(inotify_fd, dev); - else - (void) udev_watch_end(inotify_fd, dev); - - return 0; + (void) udev_watch_begin(inotify_fd, dev); } diff --git a/src/udev/udev-event.h b/src/udev/udev-event.h index d201fb580a..74d065ce23 100644 --- a/src/udev/udev-event.h +++ b/src/udev/udev-event.h @@ -76,7 +76,7 @@ int udev_event_execute_rules( Hashmap *properties_list, UdevRules *rules); void udev_event_execute_run(UdevEvent *event, usec_t timeout_usec, int timeout_signal); -int udev_event_process_inotify_watch(UdevEvent *event, int inotify_fd); +void udev_event_process_inotify_watch(UdevEvent *event, int inotify_fd); static inline usec_t udev_warn_timeout(usec_t timeout_usec) { return DIV_ROUND_UP(timeout_usec, 3); diff --git a/src/udev/udevd.c b/src/udev/udevd.c index 25496975d5..4bb6bf3a33 100644 --- a/src/udev/udevd.c +++ b/src/udev/udevd.c @@ -654,9 +654,7 @@ static int worker_process_device(Manager *manager, sd_device *dev) { /* in case rtnl was initialized */ manager->rtnl = sd_netlink_ref(udev_event->rtnl); - r = udev_event_process_inotify_watch(udev_event, manager->inotify_fd); - if (r < 0) - return r; + udev_event_process_inotify_watch(udev_event, manager->inotify_fd); log_device_uevent(dev, "Device processed"); return 0; From 790da548b0c37af60aed2f46867ba3885ea78718 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Thu, 28 Apr 2022 16:42:12 +0900 Subject: [PATCH 6/8] udev: warn on udev_watch_{begin,end}() failure --- src/udev/udev-event.c | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/src/udev/udev-event.c b/src/udev/udev-event.c index 62c6addb9a..41449da84d 100644 --- a/src/udev/udev-event.c +++ b/src/udev/udev-event.c @@ -1005,7 +1005,9 @@ static int event_execute_rules_on_remove( if (r < 0) log_device_debug_errno(dev, r, "Failed to delete database under /run/udev/data/, ignoring: %m"); - (void) udev_watch_end(inotify_fd, dev); + r = udev_watch_end(inotify_fd, dev); + if (r < 0) + log_device_warning_errno(dev, r, "Failed to remove inotify watch, ignoring: %m"); r = udev_rules_apply_to_event(rules, event, timeout_usec, timeout_signal, properties_list); @@ -1069,7 +1071,9 @@ int udev_event_execute_rules( return event_execute_rules_on_remove(event, inotify_fd, timeout_usec, timeout_signal, properties_list, rules); /* Disable watch during event processing. */ - (void) udev_watch_end(inotify_fd, event->dev); + r = udev_watch_end(inotify_fd, event->dev); + if (r < 0) + log_device_warning_errno(dev, r, "Failed to remove inotify watch, ignoring: %m"); r = device_clone_with_db(dev, &event->dev_db_clone); if (r < 0) @@ -1153,6 +1157,7 @@ void udev_event_execute_run(UdevEvent *event, usec_t timeout_usec, int timeout_s void udev_event_process_inotify_watch(UdevEvent *event, int inotify_fd) { sd_device *dev; + int r; assert(event); assert(inotify_fd >= 0); @@ -1165,5 +1170,7 @@ void udev_event_process_inotify_watch(UdevEvent *event, int inotify_fd) { if (device_for_action(dev, SD_DEVICE_REMOVE)) return; - (void) udev_watch_begin(inotify_fd, dev); + r = udev_watch_begin(inotify_fd, dev); + if (r < 0) + log_device_warning_errno(dev, r, "Failed to add inotify watch, ignoring: %m"); } From 3fb94c70621d73f656e02c016400075175b8b0bf Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Fri, 15 Apr 2022 06:38:33 +0900 Subject: [PATCH 7/8] sd-device: move device_new_from_watch_handle_at() to udev-watch.c And drop unused watch handle related functions. --- src/libsystemd/sd-device/device-internal.h | 2 - src/libsystemd/sd-device/device-private.c | 138 --------------------- src/libsystemd/sd-device/device-private.h | 6 - src/libsystemd/sd-device/sd-device.c | 1 - src/udev/udev-watch.c | 23 ++++ src/udev/udev-watch.h | 5 + 6 files changed, 28 insertions(+), 147 deletions(-) diff --git a/src/libsystemd/sd-device/device-internal.h b/src/libsystemd/sd-device/device-internal.h index 09325aae04..9a4533a8f6 100644 --- a/src/libsystemd/sd-device/device-internal.h +++ b/src/libsystemd/sd-device/device-internal.h @@ -21,8 +21,6 @@ struct sd_device { */ unsigned database_version; - int watch_handle; - sd_device *parent; OrderedHashmap *properties; diff --git a/src/libsystemd/sd-device/device-private.c b/src/libsystemd/sd-device/device-private.c index a453899910..9e120f734e 100644 --- a/src/libsystemd/sd-device/device-private.c +++ b/src/libsystemd/sd-device/device-private.c @@ -619,144 +619,6 @@ int device_get_devlink_priority(sd_device *device, int *ret) { return 0; } -int device_get_watch_handle(sd_device *device) { - char path_wd[STRLEN("/run/udev/watch/") + DECIMAL_STR_MAX(int)]; - _cleanup_free_ char *buf = NULL; - const char *id, *path_id; - int wd, r; - - assert(device); - - if (device->watch_handle >= 0) - return device->watch_handle; - - r = device_get_device_id(device, &id); - if (r < 0) - return r; - - path_id = strjoina("/run/udev/watch/", id); - r = readlink_malloc(path_id, &buf); - if (r < 0) - return r; - - r = safe_atoi(buf, &wd); - if (r < 0) - return r; - - if (wd < 0) - return -EBADF; - - buf = mfree(buf); - xsprintf(path_wd, "/run/udev/watch/%d", wd); - r = readlink_malloc(path_wd, &buf); - if (r < 0) - return r; - - if (!streq(buf, id)) - return -EBADF; - - return device->watch_handle = wd; -} - -static void device_remove_watch_handle(sd_device *device) { - const char *id; - int wd; - - assert(device); - - /* First, remove the symlink from handle to device id. */ - wd = device_get_watch_handle(device); - if (wd >= 0) { - char path_wd[STRLEN("/run/udev/watch/") + DECIMAL_STR_MAX(int)]; - - xsprintf(path_wd, "/run/udev/watch/%d", wd); - if (unlink(path_wd) < 0 && errno != ENOENT) - log_device_debug_errno(device, errno, - "sd-device: failed to remove %s, ignoring: %m", - path_wd); - } - - /* Next, remove the symlink from device id to handle. */ - if (device_get_device_id(device, &id) >= 0) { - const char *path_id; - - path_id = strjoina("/run/udev/watch/", id); - if (unlink(path_id) < 0 && errno != ENOENT) - log_device_debug_errno(device, errno, - "sd-device: failed to remove %s, ignoring: %m", - path_id); - } - - device->watch_handle = -1; -} - -int device_set_watch_handle(sd_device *device, int wd) { - char path_wd[STRLEN("/run/udev/watch/") + DECIMAL_STR_MAX(int)]; - const char *id, *path_id; - int r; - - assert(device); - - if (wd >= 0 && wd == device_get_watch_handle(device)) - return 0; - - device_remove_watch_handle(device); - - if (wd < 0) - /* negative wd means that the caller requests to clear saved watch handle. */ - return 0; - - r = device_get_device_id(device, &id); - if (r < 0) - return r; - - path_id = strjoina("/run/udev/watch/", id); - xsprintf(path_wd, "/run/udev/watch/%d", wd); - - r = mkdir_parents(path_wd, 0755); - if (r < 0) - return r; - - if (symlink(id, path_wd) < 0) - return -errno; - - if (symlink(path_wd + STRLEN("/run/udev/watch/"), path_id) < 0) { - r = -errno; - if (unlink(path_wd) < 0 && errno != ENOENT) - log_device_debug_errno(device, errno, - "sd-device: failed to remove %s, ignoring: %m", - path_wd); - return r; - } - - device->watch_handle = wd; - - return 0; -} - -int device_new_from_watch_handle_at(sd_device **ret, int dirfd, int wd) { - char path_wd[STRLEN("/run/udev/watch/") + DECIMAL_STR_MAX(int)]; - _cleanup_free_ char *id = NULL; - int r; - - assert(ret); - - if (wd < 0) - return -EBADF; - - if (dirfd >= 0) { - xsprintf(path_wd, "%d", wd); - r = readlinkat_malloc(dirfd, path_wd, &id); - } else { - xsprintf(path_wd, "/run/udev/watch/%d", wd); - r = readlink_malloc(path_wd, &id); - } - if (r < 0) - return r; - - return sd_device_new_from_device_id(ret, id); -} - int device_rename(sd_device *device, const char *name) { _cleanup_free_ char *new_syspath = NULL; const char *interface; diff --git a/src/libsystemd/sd-device/device-private.h b/src/libsystemd/sd-device/device-private.h index d33cddcf80..724061a28c 100644 --- a/src/libsystemd/sd-device/device-private.h +++ b/src/libsystemd/sd-device/device-private.h @@ -13,17 +13,12 @@ int device_new_from_mode_and_devnum(sd_device **ret, mode_t mode, dev_t devnum); int device_new_from_nulstr(sd_device **ret, char *nulstr, size_t len); int device_new_from_strv(sd_device **ret, char **strv); -int device_new_from_watch_handle_at(sd_device **ret, int dirfd, int wd); -static inline int device_new_from_watch_handle(sd_device **ret, int wd) { - return device_new_from_watch_handle_at(ret, -1, wd); -} int device_get_property_bool(sd_device *device, const char *key); int device_get_sysattr_unsigned(sd_device *device, const char *sysattr, unsigned *ret_value); int device_get_sysattr_bool(sd_device *device, const char *sysattr); int device_get_device_id(sd_device *device, const char **ret); int device_get_devlink_priority(sd_device *device, int *ret); -int device_get_watch_handle(sd_device *device); int device_get_devnode_mode(sd_device *device, mode_t *ret); int device_get_devnode_uid(sd_device *device, uid_t *ret); int device_get_devnode_gid(sd_device *device, gid_t *ret); @@ -34,7 +29,6 @@ int device_get_cached_sysattr_value(sd_device *device, const char *key, const ch void device_seal(sd_device *device); void device_set_is_initialized(sd_device *device); -int device_set_watch_handle(sd_device *device, int wd); void device_set_db_persist(sd_device *device); void device_set_devlink_priority(sd_device *device, int priority); int device_ensure_usec_initialized(sd_device *device, sd_device *device_old); diff --git a/src/libsystemd/sd-device/sd-device.c b/src/libsystemd/sd-device/sd-device.c index a7ba7dd6e9..91af4a27ed 100644 --- a/src/libsystemd/sd-device/sd-device.c +++ b/src/libsystemd/sd-device/sd-device.c @@ -46,7 +46,6 @@ int device_new_aux(sd_device **ret) { *device = (sd_device) { .n_ref = 1, - .watch_handle = -1, .devmode = MODE_INVALID, .devuid = UID_INVALID, .devgid = GID_INVALID, diff --git a/src/udev/udev-watch.c b/src/udev/udev-watch.c index 0cc83ddb9c..2a9ff2909b 100644 --- a/src/udev/udev-watch.c +++ b/src/udev/udev-watch.c @@ -19,6 +19,29 @@ #include "string-util.h" #include "udev-watch.h" +int device_new_from_watch_handle_at(sd_device **ret, int dirfd, int wd) { + char path_wd[STRLEN("/run/udev/watch/") + DECIMAL_STR_MAX(int)]; + _cleanup_free_ char *id = NULL; + int r; + + assert(ret); + + if (wd < 0) + return -EBADF; + + if (dirfd >= 0) { + xsprintf(path_wd, "%d", wd); + r = readlinkat_malloc(dirfd, path_wd, &id); + } else { + xsprintf(path_wd, "/run/udev/watch/%d", wd); + r = readlink_malloc(path_wd, &id); + } + if (r < 0) + return r; + + return sd_device_new_from_device_id(ret, id); +} + int udev_watch_restore(int inotify_fd) { _cleanup_closedir_ DIR *dir = NULL; int r; diff --git a/src/udev/udev-watch.h b/src/udev/udev-watch.h index d211d99f49..c454deead3 100644 --- a/src/udev/udev-watch.h +++ b/src/udev/udev-watch.h @@ -3,6 +3,11 @@ #include "sd-device.h" +int device_new_from_watch_handle_at(sd_device **ret, int dirfd, int wd); +static inline int device_new_from_watch_handle(sd_device **ret, int wd) { + return device_new_from_watch_handle_at(ret, -1, wd); +} + int udev_watch_restore(int inotify_fd); int udev_watch_begin(int inotify_fd, sd_device *dev); int udev_watch_end(int inotify_fd, sd_device *dev); From ee2750122572523f8d414cf5ab37235e3b5595ee Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Thu, 28 Apr 2022 19:28:11 +0900 Subject: [PATCH 8/8] test: add testcase for udev-watch --- test/units/testsuite-64.sh | 43 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 43 insertions(+) diff --git a/test/units/testsuite-64.sh b/test/units/testsuite-64.sh index 6704e9933c..4ddea69c51 100755 --- a/test/units/testsuite-64.sh +++ b/test/units/testsuite-64.sh @@ -41,6 +41,48 @@ helper_check_device_symlinks() {( done < <(find "${paths[@]}" -type l) )} +helper_check_udev_watch() {( + set +x + + local link target id dev + + while read -r link; do + target="$(readlink "$link")" + echo "$link -> $target" + + if [[ ! -L "/run/udev/watch/$target" ]]; then + echo >&2 "ERROR: symlink /run/udev/watch/$target does not exist" + return 1 + fi + if [[ "$(readlink "/run/udev/watch/$target")" != "$(basename "$link")" ]]; then + echo >&2 "ERROR: symlink target of /run/udev/watch/$target is inconsistent with $link" + return 1 + fi + + if [[ "$target" =~ ^[0-9]+$ ]]; then + # $link is ID -> wd + id="$(basename "$link")" + else + # $link is wd -> ID + id="$target" + fi + + if [[ "${id:0:1}" == "b" ]]; then + dev="/dev/block/${id:1}" + elif [[ "${id:0:1}" == "c" ]]; then + dev="/dev/char/${id:1}" + else + echo >&2 "ERROR: unexpected device ID '$id'" + return 1 + fi + + if [[ ! -e "$dev" ]]; then + echo >&2 "ERROR: device '$dev' corresponding to symlink '$link' does not exist" + return 1 + fi + done < <(find /run/udev/watch -type l) +)} + testcase_megasas2_basic() { lsblk -S [[ "$(lsblk --scsi --noheadings | wc -l)" -ge 128 ]] @@ -201,6 +243,7 @@ EOF if ((i % 10 == 0)); then udevadm wait --settle --timeout="$timeout" "$blockdev" helper_check_device_symlinks + helper_check_udev_watch fi done