From: Yu Watanabe Date: Sat, 6 Mar 2021 07:09:23 +0000 (+0900) Subject: udev: do not store inotify fd in a global variable X-Git-Tag: v249-rc1~315^2~15 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=df7ee95913c70088263c768723dc5572982221f9;p=thirdparty%2Fsystemd.git udev: do not store inotify fd in a global variable When manager_exit() or manager_free() is called, the global variable in udev-watch.c is not set '-1'. Of course, that is safe, as the event source for the inotify fd is unref()ed in manager_exit() and manager_free(). But let's not store fd globally. --- diff --git a/src/test/test-udev.c b/src/test/test-udev.c index 6bb8a9e4fc8..c2c63cc679c 100644 --- a/src/test/test-udev.c +++ b/src/test/test-udev.c @@ -130,7 +130,7 @@ static int run(int argc, char *argv[]) { } } - udev_event_execute_rules(event, 3 * USEC_PER_SEC, SIGKILL, NULL, rules); + udev_event_execute_rules(event, -1, 3 * USEC_PER_SEC, SIGKILL, NULL, rules); udev_event_execute_run(event, 3 * USEC_PER_SEC, SIGKILL); return 0; diff --git a/src/udev/udev-event.c b/src/udev/udev-event.c index 12597194a91..82cdd99b7af 100644 --- a/src/udev/udev-event.c +++ b/src/udev/udev-event.c @@ -913,6 +913,7 @@ static int update_devnode(UdevEvent *event) { static void event_execute_rules_on_remove( UdevEvent *event, + int inotify_fd, usec_t timeout_usec, int timeout_signal, Hashmap *properties_list, @@ -934,7 +935,7 @@ static void event_execute_rules_on_remove( log_device_debug_errno(dev, r, "Failed to delete database under /run/udev/data/, ignoring: %m"); if (sd_device_get_devnum(dev, NULL) >= 0) - (void) udev_watch_end(dev); + (void) udev_watch_end(inotify_fd, dev); (void) udev_rules_apply_to_event(rules, event, timeout_usec, timeout_signal, properties_list); @@ -971,11 +972,14 @@ static int copy_all_tags(sd_device *d, sd_device *s) { return 0; } -int udev_event_execute_rules(UdevEvent *event, - usec_t timeout_usec, - int timeout_signal, - Hashmap *properties_list, - UdevRules *rules) { +int udev_event_execute_rules( + UdevEvent *event, + int inotify_fd, /* This may be negative */ + usec_t timeout_usec, + int timeout_signal, + Hashmap *properties_list, + UdevRules *rules) { + const char *subsystem; sd_device_action_t action; sd_device *dev; @@ -995,7 +999,7 @@ int udev_event_execute_rules(UdevEvent *event, return log_device_error_errno(dev, r, "Failed to get ACTION: %m"); if (action == SD_DEVICE_REMOVE) { - event_execute_rules_on_remove(event, timeout_usec, timeout_signal, properties_list, rules); + event_execute_rules_on_remove(event, inotify_fd, timeout_usec, timeout_signal, properties_list, rules); return 0; } @@ -1009,7 +1013,7 @@ int udev_event_execute_rules(UdevEvent *event, if (sd_device_get_devnum(dev, NULL) >= 0) /* Disable watch during event processing. */ - (void) udev_watch_end(event->dev_db_clone); + (void) udev_watch_end(inotify_fd, event->dev_db_clone); if (action == SD_DEVICE_MOVE) { r = udev_event_on_move(event->dev); diff --git a/src/udev/udev-event.h b/src/udev/udev-event.h index ecbe957b4f6..d90cc10a2d8 100644 --- a/src/udev/udev-event.h +++ b/src/udev/udev-event.h @@ -51,20 +51,28 @@ UdevEvent *udev_event_new(sd_device *dev, usec_t exec_delay_usec, sd_netlink *rt UdevEvent *udev_event_free(UdevEvent *event); DEFINE_TRIVIAL_CLEANUP_FUNC(UdevEvent*, udev_event_free); -size_t udev_event_apply_format(UdevEvent *event, - const char *src, char *dest, size_t size, - bool replace_whitespace); +size_t udev_event_apply_format( + UdevEvent *event, + const char *src, + char *dest, + size_t size, + bool replace_whitespace); int udev_check_format(const char *value, size_t *offset, const char **hint); -int udev_event_spawn(UdevEvent *event, - usec_t timeout_usec, - int timeout_signal, - bool accept_failure, - const char *cmd, char *result, size_t ressize); -int udev_event_execute_rules(UdevEvent *event, - usec_t timeout_usec, - int timeout_signal, - Hashmap *properties_list, - UdevRules *rules); +int udev_event_spawn( + UdevEvent *event, + usec_t timeout_usec, + int timeout_signal, + bool accept_failure, + const char *cmd, + char *result, + size_t ressize); +int udev_event_execute_rules( + UdevEvent *event, + int inotify_fd, + usec_t timeout_usec, + int timeout_signal, + Hashmap *properties_list, + UdevRules *rules); void udev_event_execute_run(UdevEvent *event, usec_t timeout_usec, int timeout_signal); static inline usec_t udev_warn_timeout(usec_t timeout_usec) { diff --git a/src/udev/udev-watch.c b/src/udev/udev-watch.c index 8656fb0076d..e90fcd1bcc2 100644 --- a/src/udev/udev-watch.c +++ b/src/udev/udev-watch.c @@ -16,28 +16,13 @@ #include "stdio-util.h" #include "udev-watch.h" -static int inotify_fd = -1; - -/* inotify descriptor, will be shared with rules directory; - * set to cloexec since we need our children to be able to add - * watches for us. */ -int udev_watch_init(void) { - inotify_fd = inotify_init1(IN_CLOEXEC); - if (inotify_fd < 0) - return -errno; - - return inotify_fd; -} - /* Move any old watches directory out of the way, and then restore the watches. */ -int udev_watch_restore(void) { +int udev_watch_restore(int inotify_fd) { struct dirent *ent; DIR *dir; int r; - if (inotify_fd < 0) - return log_debug_errno(SYNTHETIC_ERRNO(EINVAL), - "Invalid inotify descriptor."); + assert(inotify_fd >= 0); if (rename("/run/udev/watch", "/run/udev/watch.old") < 0) { if (errno != ENOENT) @@ -70,7 +55,7 @@ int udev_watch_restore(void) { } log_device_debug(dev, "Restoring old watch"); - (void) udev_watch_begin(dev); + (void) udev_watch_begin(inotify_fd, dev); unlink: (void) unlinkat(dirfd(dir), ent->d_name, 0); } @@ -81,14 +66,13 @@ unlink: return 0; } -int udev_watch_begin(sd_device *dev) { +int udev_watch_begin(int inotify_fd, sd_device *dev) { char filename[STRLEN("/run/udev/watch/") + DECIMAL_STR_MAX(int)]; const char *devnode, *id_filename; int wd, r; - if (inotify_fd < 0) - return log_debug_errno(SYNTHETIC_ERRNO(EINVAL), - "Invalid inotify descriptor."); + assert(inotify_fd >= 0); + assert(dev); r = sd_device_get_devname(dev, &devnode); if (r < 0) @@ -118,12 +102,15 @@ int udev_watch_begin(sd_device *dev) { return 0; } -int udev_watch_end(sd_device *dev) { +int udev_watch_end(int inotify_fd, sd_device *dev) { char filename[STRLEN("/run/udev/watch/") + DECIMAL_STR_MAX(int)]; int wd, r; + assert(dev); + + /* This may be called by 'udevadm test'. In that case, inotify_fd is not initialized. */ if (inotify_fd < 0) - return 0; /* Nothing to do. */ + return 0; r = device_get_watch_handle(dev, &wd); if (r == -ENOENT) @@ -147,16 +134,9 @@ int udev_watch_lookup(int wd, sd_device **ret) { _cleanup_free_ char *device = NULL; int r; + assert(wd >= 0); assert(ret); - if (inotify_fd < 0) - return log_debug_errno(SYNTHETIC_ERRNO(EINVAL), - "Invalid inotify descriptor."); - - if (wd < 0) - return log_debug_errno(SYNTHETIC_ERRNO(EINVAL), - "Invalid watch handle."); - xsprintf(filename, "/run/udev/watch/%d", wd); r = readlink_malloc(filename, &device); if (r == -ENOENT) diff --git a/src/udev/udev-watch.h b/src/udev/udev-watch.h index a15fa2767e8..f5c55794c40 100644 --- a/src/udev/udev-watch.h +++ b/src/udev/udev-watch.h @@ -3,8 +3,7 @@ #include "sd-device.h" -int udev_watch_init(void); -int udev_watch_restore(void); -int udev_watch_begin(sd_device *dev); -int udev_watch_end(sd_device *dev); +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); int udev_watch_lookup(int wd, sd_device **ret); diff --git a/src/udev/udevadm-test.c b/src/udev/udevadm-test.c index 7a30e25135c..fbac719fa06 100644 --- a/src/udev/udevadm-test.c +++ b/src/udev/udevadm-test.c @@ -141,7 +141,7 @@ int test_main(int argc, char *argv[], void *userdata) { assert_se(sigfillset(&mask) >= 0); assert_se(sigprocmask(SIG_SETMASK, &mask, &sigmask_orig) >= 0); - udev_event_execute_rules(event, 60 * USEC_PER_SEC, SIGKILL, NULL, rules); + udev_event_execute_rules(event, -1, 60 * USEC_PER_SEC, SIGKILL, NULL, rules); FOREACH_DEVICE_PROPERTY(dev, key, value) printf("%s=%s\n", key, value); diff --git a/src/udev/udevd.c b/src/udev/udevd.c index 2c702d0388d..299f89566f5 100644 --- a/src/udev/udevd.c +++ b/src/udev/udevd.c @@ -92,10 +92,12 @@ typedef struct Manager { sd_device_monitor *monitor; struct udev_ctrl *ctrl; - int fd_inotify; int worker_watch[2]; + /* used by udev-watch */ + int inotify_fd; sd_event_source *inotify_event; + sd_event_source *kill_workers_event; usec_t last_usec; @@ -304,7 +306,7 @@ static Manager* manager_free(Manager *manager) { hashmap_free_free_free(manager->properties); udev_rules_free(manager->rules); - safe_close(manager->fd_inotify); + safe_close(manager->inotify_fd); safe_close_pair(manager->worker_watch); return mfree(manager); @@ -473,7 +475,7 @@ static int worker_process_device(Manager *manager, sd_device *dev) { * degree — they go hand-in-hand after all. */ log_device_debug(dev, "Block device is currently locked, installing watch to wait until the lock is released."); - (void) udev_watch_begin(dev); + (void) udev_watch_begin(manager->inotify_fd, dev); /* Now the watch is installed, let's lock the device again, maybe in the meantime things changed */ r = worker_lock_block_device(dev, &fd_lock); @@ -484,7 +486,13 @@ static int worker_process_device(Manager *manager, sd_device *dev) { (void) worker_mark_block_device_read_only(dev); /* apply rules, create node, symlinks */ - r = udev_event_execute_rules(udev_event, arg_event_timeout_usec, arg_timeout_signal, manager->properties, manager->rules); + r = udev_event_execute_rules( + udev_event, + manager->inotify_fd, + arg_event_timeout_usec, + arg_timeout_signal, + manager->properties, + manager->rules); if (r < 0) return r; @@ -496,12 +504,12 @@ static int worker_process_device(Manager *manager, sd_device *dev) { /* apply/restore/end inotify watch */ if (udev_event->inotify_watch) { - (void) udev_watch_begin(dev); + (void) udev_watch_begin(manager->inotify_fd, dev); r = device_update_db(dev); if (r < 0) return log_device_debug_errno(dev, r, "Failed to update database under /run/udev/data/: %m"); } else - (void) udev_watch_end(dev); + (void) udev_watch_end(manager->inotify_fd, dev); log_device_uevent(dev, "Device processed"); return 0; @@ -872,7 +880,7 @@ static void manager_exit(Manager *manager) { manager->ctrl = udev_ctrl_unref(manager->ctrl); manager->inotify_event = sd_event_source_unref(manager->inotify_event); - manager->fd_inotify = safe_close(manager->fd_inotify); + manager->inotify_fd = safe_close(manager->inotify_fd); manager->monitor = sd_device_monitor_unref(manager->monitor); @@ -1309,7 +1317,7 @@ static int on_inotify(sd_event_source *s, int fd, uint32_t revents, void *userda if (e->mask & IN_CLOSE_WRITE) synthesize_change(dev); else if (e->mask & IN_IGNORED) - udev_watch_end(dev); + udev_watch_end(manager->inotify_fd, dev); } return 1; @@ -1669,7 +1677,7 @@ static int manager_new(Manager **ret, int fd_ctrl, int fd_uevent, const char *cg return log_oom(); *manager = (Manager) { - .fd_inotify = -1, + .inotify_fd = -1, .worker_watch = { -1, -1 }, .cgroup = cgroup, }; @@ -1722,12 +1730,11 @@ static int main_loop(Manager *manager) { if (r < 0) return log_error_errno(r, "Failed to enable SO_PASSCRED: %m"); - r = udev_watch_init(); - if (r < 0) - return log_error_errno(r, "Failed to create inotify descriptor: %m"); - manager->fd_inotify = r; + manager->inotify_fd = inotify_init1(IN_CLOEXEC); + if (manager->inotify_fd < 0) + return log_error_errno(errno, "Failed to create inotify descriptor: %m"); - udev_watch_restore(); + udev_watch_restore(manager->inotify_fd); /* block and listen to all signals on signalfd */ assert_se(sigprocmask_many(SIG_BLOCK, NULL, SIGTERM, SIGINT, SIGHUP, SIGCHLD, -1) >= 0); @@ -1772,7 +1779,7 @@ static int main_loop(Manager *manager) { if (r < 0) return log_error_errno(r, "Failed to set IDLE event priority for udev control event source: %m"); - r = sd_event_add_io(manager->event, &manager->inotify_event, manager->fd_inotify, EPOLLIN, on_inotify, manager); + r = sd_event_add_io(manager->event, &manager->inotify_event, manager->inotify_fd, EPOLLIN, on_inotify, manager); if (r < 0) return log_error_errno(r, "Failed to create inotify event source: %m");