udev: make sure to install an inotify watch whenever we find a block device locked

This fixes a race where a block device that pops up and immediately is
locked (such as a loopback device in preparation) might result in
udev never run any rules for it, and thus never turn on inotify watching
for it (as inotify watching is controlled via an option set via udev
rules), thus not noticing when the device is unlocked/closed again
(which is noticed via IN_CLOSE_WRITE inotify events).

This changes two things:

1. Whenever we encounter a locked block device we'll now inotify watch
   it, so that it is guaranteed we'll notice when the BSD lock fd is
   closed again, and will reprobe.

2. We'll now turn off inotify watching again once we realise the
   udev rules don't actually want that. Previously, once watching a
   device was enabled via a udev rule, it would be watched forever until
   the device disappeared, even if the option was dropped by the rules
   for later events.

Together this will make sure that we'll watch the device via inotify
in both of the following cases:

a) The block device has been BSD locked when udev wanted to look at it

b) The udev rules run for the last seen event for the device say so

In all other cases inotify is off for block devices.

This new behaviour both fixes the race, but also makes the most sense,
as the rules (when they are run) actually really control the watch state
now. And if someone BSD locks a block device then it should be OK to
inotify watch it briefly until the lock is released again as the user
this way more or less opts into the locking protocol.

diff --git a/src/udev/udevd.c b/src/udev/udevd.c
index d6415ad57e9a51f9cda531eef70ca6d3dc0133de..18079d363d8476e93d0f749d39b735ee9c41ee71 100644 (file)
--- a/src/udev/udevd.c
+++ b/src/udev/udevd.c
@@ -459,6 +459,43 @@ static int worker_process_device(Manager *manager, sd_device *dev) {
                 return -ENOMEM;
 
         r = worker_lock_block_device(dev, &fd_lock);
+        if (r == -EAGAIN) {
+                /* So this is a block device and the device is locked currently via the BSD advisory locks —
+                 * someone else is exclusively using it. This means we don't run our udev rules now, to not
+                 * interfere. However we want to know when the device is unlocked again, and retrigger the
+                 * device again then, so that the rules are run eventually. For that we use IN_CLOSE_WRITE
+                 * inotify watches (which isn't exactly the same as waiting for the BSD locks to release, but
+                 * not totally off, as long as unlock+close() is done together, as it usually is).
+                 *
+                 * (The user-facing side of this: https://systemd.io/BLOCK_DEVICE_LOCKING)
+                 *
+                 * There's a bit of a chicken and egg problem here for this however: inotify watching is
+                 * supposed to be enabled via an option set via udev rules (OPTIONS+="watch"). If we skip the
+                 * udev rules here however (as we just said we do), we would thus never see that specific
+                 * udev rule, and thus never turn on inotify watching. But in order to catch up eventually
+                 * and run them we we need the inotify watching: hence a classic chicken and egg problem.
+                 *
+                 * Our way out here: if we see the block device locked, unconditionally watch the device via
+                 * inotify, regardless of any explicit request via OPTIONS+="watch". Thus, a device that is
+                 * currently locked via the BSD file locks will be treated as if we ran a single udev rule
+                 * only for it: the one that turns on inotify watching for it. If we eventually see the
+                 * inotify IN_CLOSE_WRITE event, and then run the rules after all and we then realize that
+                 * this wasn't actually requested (i.e. no OPTIONS+="watch" set) we'll simply turn off the
+                 * watching again (see below). Effectively this means: inotify watching is now enabled either
+                 * a) when the udev rules say so, or b) while the device is locked.
+                 *
+                 * Worst case scenario hence: in the (unlikely) case someone locked the device and we clash
+                 * with that we might do inotify watching for a brief moment for a device where we actually
+                 * weren't supposed to. But that shouldn't be too bad, in particular as BSD locks being taken
+                 * on a block device is kinda an indication that the inotify logic is desired too, to some
+                 * 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);
+
+                /* 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);
+        }
         if (r < 0)
                 return r;
 
@@ -475,13 +512,14 @@ static int worker_process_device(Manager *manager, sd_device *dev) {
                 /* in case rtnl was initialized */
                 manager->rtnl = sd_netlink_ref(udev_event->rtnl);
 
-        /* apply/restore inotify watch */
+        /* apply/restore/end inotify watch */
         if (udev_event->inotify_watch) {
                 (void) udev_watch_begin(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);
 
         log_device_debug(dev, "Device (SEQNUM=%"PRIu64", ACTION=%s) processed",
                          seqnum, device_action_to_string(action));