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