tree-wide: take BSD lock on loopback devices we dissect/mount/operate on

So here's something we should always keep in mind:

systemd-udevd actually does *two* things with BSD file locks on block
devices:

1. While it probes a device it takes a LOCK_SH lock. Thus everyone else
   taking a LOCK_EX lock will temporarily block udev from probing
   devices, which is good when making changes to it.

2. Whenever a device is closed after write (detected via inotify), udevd
   will issue BLKRRPART (requesting the kernel to reread the partition
   table). It does this while holding a LOCK_EX lock on the block
   device. Thus anyone else taking LOCK_SH or LOCK_EX will temporarily
   block udevd from issuing that ioctl. And that's quite relevant, since
   the kernel will temporarily flush out all partitions while re-reading
   the partition table and then create them anew. Thus it is smart to
   take LOCK_SH when dissecting a block device to ensure that no
   BLKRRPART is issued in the background, until we mounted the devices.

1 files changed, 24 insertions, 0 deletions

diff --git a/src/test/test-loop-block.c b/src/test/test-loop-block.c
index 714d51cfc2..d1793222f0 100644
--- a/src/test/test-loop-block.c
+++ b/src/test/test-loop-block.c
@@ -55,6 +55,9 @@ static void* thread_func(void *ptr) {
 
                 log_notice("Acquired loop device %s, will mount on %s", loop->node, mounted);
 
+                /* Let's make sure udev doesn't call BLKRRPART in the background, while we try to mount the device. */
+                assert_se(loop_device_flock(loop, LOCK_SH) >= 0);
+
                 r = dissect_image(loop->fd, NULL, NULL, loop->diskseq, loop->uevent_seqnum_not_before, loop->timestamp_not_before, DISSECT_IMAGE_READ_ONLY, &dissected);
                 if (r < 0)
                         log_error_errno(r, "Failed dissect loopback device %s: %m", loop->node);
@@ -85,6 +88,10 @@ static void* thread_func(void *ptr) {
                 log_notice_errno(r, "Mounted %s → %s: %m", loop->node, mounted);
                 assert_se(r >= 0);
 
+                /* Now the block device is mounted, we don't need no manual lock anymore, the devices are now
+                 * pinned by the mounts. */
+                assert_se(loop_device_flock(loop, LOCK_UN) >= 0);
+
                 log_notice("Unmounting %s", mounted);
                 mounted = umount_and_rmdir_and_free(mounted);
 
@@ -215,6 +222,11 @@ static int run(int argc, char *argv[]) {
         pthread_t threads[arg_n_threads];
         sd_id128_t id;
 
+        /* Take an explicit lock while we format the file systems, in accordance with
+         * https://systemd.io/BLOCK_DEVICE_LOCKING/. We don't want udev to interfere and probe while we write
+         * or even issue BLKRRPART or similar while we are working on this. */
+        assert_se(loop_device_flock(loop, LOCK_EX) >= 0);
+
         assert_se(dissect_image(loop->fd, NULL, NULL, loop->diskseq, loop->uevent_seqnum_not_before, loop->timestamp_not_before, 0, &dissected) >= 0);
 
         assert_se(dissected->partitions[PARTITION_ESP].found);
@@ -243,9 +255,21 @@ static int run(int argc, char *argv[]) {
 
         assert_se(mkdtemp_malloc(NULL, &mounted) >= 0);
 
+        /* We are particularly correct here, and now downgrade LOCK → LOCK_SH. That's because we are done
+         * with formatting the file systems, so we don't need the exclusive lock anymore. From now on a
+         * shared one is fine. This way udev can now probe the device if it wants, but still won't call
+         * BLKRRPART on it, and that's good, because that would destroy our partition table while we are at
+         * it. */
+        assert_se(loop_device_flock(loop, LOCK_SH) >= 0);
+
         /* This first (writable) mount will initialize the mount point dirs, so that the subsequent read-only ones can work */
         assert_se(dissected_image_mount(dissected, mounted, UID_INVALID, UID_INVALID, 0) >= 0);
 
+        /* Now we mounted everything, the partitions are pinned. Now it's fine to release the lock
+         * fully. This means udev could now issue BLKRRPART again, but that's OK given this will fail because
+         * we now mounted the device. */
+        assert_se(loop_device_flock(loop, LOCK_UN) >= 0);
+
         assert_se(umount_recursive(mounted, 0) >= 0);
         loop = loop_device_unref(loop);