From db93e063bdffe0a8b95fcc522aeacddf62d1a9f9 Mon Sep 17 00:00:00 2001 From: Tom Gundersen Date: Mon, 9 Mar 2015 16:16:23 +0100 Subject: [PATCH] udevd: close race in udev settle The udev-settle guarantees that udevd is no longer processing any of the events casued by udev-trigger. The way this works is that it sends a synchronous PING to udevd after udev-trigger has ran, and when that returns it knows that udevd has started processing the events from udev-trigger. udev-settle will then wait for the event queue to empty before returning. However, there was a race here, as we would only update the /run state at the beginning of the event loop, before reading out new events and before processing the ping. That means that if the first uevent arrived in the same event-loop iteration as the PING, we would return the ping before updating the queue state in /run (which would happen on the next iteration). The race window here is tiny (as the /run state would probably get updated before udev-settle got a chance to read /run), but still a possibility. Fix the problem by updating the /run state as the last step before returning the PING. We must still update it at the beginning of the loop as well, otherwise we risk being stuck in poll() with a stale state in /run. Reported-by: Daniel Drake --- src/udev/udevd.c | 26 +++++++++++++++++--------- 1 file changed, 17 insertions(+), 9 deletions(-) diff --git a/src/udev/udevd.c b/src/udev/udevd.c index 0556356f6..1c510f44f 100644 --- a/src/udev/udevd.c +++ b/src/udev/udevd.c @@ -903,6 +903,17 @@ static void handle_signal(struct udev *udev, int signo) { } } +static void event_queue_update(void) { + if (!udev_list_node_is_empty(&event_list)) { + int fd; + + fd = open("/run/udev/queue", O_WRONLY|O_CREAT|O_CLOEXEC|O_TRUNC|O_NOFOLLOW, 0444); + if (fd >= 0) + close(fd); + } else + unlink("/run/udev/queue"); +} + static int systemd_fds(struct udev *udev, int *rctrl, int *rnetlink) { int ctrl = -1, netlink = -1; int fd, n; @@ -1363,15 +1374,7 @@ int main(int argc, char *argv[]) { } /* tell settle that we are busy or idle */ - if (!udev_list_node_is_empty(&event_list)) { - int fd; - - fd = open("/run/udev/queue", O_WRONLY|O_CREAT|O_CLOEXEC|O_TRUNC|O_NOFOLLOW, 0444); - if (fd >= 0) - close(fd); - } else { - unlink("/run/udev/queue"); - } + event_queue_update(); fdcount = epoll_wait(fd_ep, ev, ELEMENTSOF(ev), timeout); if (fdcount < 0) @@ -1496,6 +1499,11 @@ int main(int argc, char *argv[]) { if (is_inotify) handle_inotify(udev); + /* tell settle that we are busy or idle, this needs to be before the + * PING handling + */ + event_queue_update(); + /* * This needs to be after the inotify handling, to make sure, * that the ping is send back after the possibly generated -- 2.30.2