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 6bb8a9e4fc879477f8d58c0024f595aa591249b1..c2c63cc679c2bf3c55fd3aaddbeb8a90e707a0cb 100644 (file)
--- 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 12597194a91d7e2f64a385d5985da16c99371639..82cdd99b7afb2434ad8cf11058ba67df46187011 100644 (file)
--- 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 ecbe957b4f6affdeb7ea5ed4f8d1cb0a1e95ce5b..d90cc10a2d8a67018b56aaa022553f11e5474dc1 100644 (file)
--- 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 8656fb0076dc4b7c39553bc0a55f8633b929a43a..e90fcd1bcc28b80432b0baa36e03eb105d7bf0b7 100644 (file)
--- 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 a15fa2767e836379fca6e5df6fea43d7ce355c79..f5c55794c40337db86613509f161d1dfc3b6e9a1 100644 (file)
--- 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 7a30e25135c5dad6976b8d4cc2d59bda7115a19c..fbac719fa066a1955a6c97cc6883b41691bd7c05 100644 (file)
--- 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 2c702d0388d3e33765cc7731aa790c6a82dd47a3..299f89566f57395e736bdeac6f08cd4169e718cd 100644 (file)
--- 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");