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