loop-util: don't reuse partition fd when partscan needed

Some devices (e.g. android phones running pmOS) cannot have their OEM
partition table altered without breaking the firmware, so the distros's
partitions live inside a nested GPT carved into one of the OEM
partitions. Exposing these subpartitions requires wrapping the outer
partition in a loop device with partscan enabled, since the kernel does
not go into nested partition tables.

systemd already detects this case in udev-builtin-blkid
(ID_PART_GPT_AUTO_ROOT_DISK_NEEDS_LOOP) and acts on with
systemd-loop@.service, but this fails towards the end.
loop_device_make_internal has an optimization where if the input is
already a block device with a matching sector size, it skips creating
a loop and just hands back the original fd. That's fine for whole disks
but wrong for partitions, which don't support partscan, so this causes
dissect_image to fail with EPROTONOSUPPORT.

This patch changes the behavior to only take the shortcut when the input
is a whole disk, or when partscan was not requested.

Co-Authored-By: Clayton Craft <clayton@craftyguy.net>

diff --git a/src/shared/loop-util.c b/src/shared/loop-util.c
index e6fe4dbb49d7a4da9e50969fe85fff1fc63aba85..3437afcda49f69c83f732eaf22e1bbf8a922e1a2 100644 (file)
--- a/src/shared/loop-util.c
+++ b/src/shared/loop-util.c
@@ -445,6 +445,42 @@ static int probe_sector_size_harder(int fd, uint32_t *ret) {
         return probe_sector_size(probe_fd, ret);
 }
 
+static int loop_device_can_shortcut(
+                int fd,
+                uint64_t offset,
+                uint64_t size,
+                uint32_t sector_size,
+                uint32_t device_ssz,
+                uint32_t loop_flags) {
+
+        int r;
+
+        /* Returns whether we can hand back the original block device fd instead of allocating a real
+         * loopback device for it: it must cover the whole device, the requested sector size must match the
+         * device's sector size, and if partscan was requested it must already be enabled on the device
+         * (otherwise e.g. partition block devices or loop devices created without LO_FLAGS_PARTSCAN would
+         * be reused even though they cannot expose nested partitions). */
+
+        assert(fd >= 0);
+
+        if (offset != 0)
+                return false;
+        if (!IN_SET(size, 0, UINT64_MAX))
+                return false;
+        if (sector_size != device_ssz)
+                return false;
+
+        if (FLAGS_SET(loop_flags, LO_FLAGS_PARTSCAN)) {
+                r = blockdev_partscan_enabled_fd(fd);
+                if (r < 0)
+                        return r;
+                if (r == 0)
+                        return false;
+        }
+
+        return true;
+}
+
 static int loop_device_make_internal(
                 const char *path,
                 int fd,
@@ -510,13 +546,10 @@ static int loop_device_make_internal(
                 if (sector_size == 0)
                         sector_size = device_ssz;
 
-                if (offset == 0 && IN_SET(size, 0, UINT64_MAX) && sector_size == device_ssz)
-                        /* If this is already a block device and we are supposed to cover the whole of it
-                         * then store an fd to the original open device node — and do not actually create
-                         * an unnecessary loopback device for it. If an explicit sector size was requested
-                         * that differs from the device sector size, or if the probed GPT sector size
-                         * differs (e.g. CD-ROMs with 2048-byte blocks but a 512-byte sector GPT), create
-                         * a real loop device to change the sector size. */
+                r = loop_device_can_shortcut(fd, offset, size, sector_size, device_ssz, loop_flags);
+                if (r < 0)
+                        return r;
+                if (r > 0)
                         return loop_device_open_from_fd(fd, open_flags, lock_op, ret);
         } else {
                 r = stat_verify_regular(&st);

diff --git a/src/test/test-loop-util.c b/src/test/test-loop-util.c
index f90bf0e1998fbdcee2c152efbd6f505df1edb198..fca125564a18b8a2260494a111772e29fc6fe9df 100644 (file)
--- a/src/test/test-loop-util.c
+++ b/src/test/test-loop-util.c
@@ -538,4 +538,37 @@ TEST(sector_size_mismatch) {
         loop = loop_device_unref(loop);
 }
 
+TEST(partscan_required) {
+        _cleanup_(loop_device_unrefp) LoopDevice *block_loop = NULL, *loop = NULL;
+        _cleanup_close_ int fd = -EBADF;
+
+        if (have_effective_cap(CAP_SYS_ADMIN) <= 0) {
+                log_tests_skipped("not running privileged");
+                return;
+        }
+
+        if (detect_container() != 0 || running_in_chroot() != 0) {
+                log_tests_skipped("Test not supported in a container/chroot, requires udev/uevent notifications");
+                return;
+        }
+
+        ASSERT_OK(make_test_image(&fd));
+
+        /* Set up a backing loop device without LO_FLAGS_PARTSCAN. */
+        ASSERT_OK(loop_device_make(fd, O_RDWR, 0, UINT64_MAX, 0, 0, LOCK_EX, &block_loop));
+        ASSERT_TRUE(block_loop->created);
+        ASSERT_OK(loop_device_flock(block_loop, LOCK_SH));
+
+        /* Without LO_FLAGS_PARTSCAN: shortcut should be taken (reuse existing loop). */
+        ASSERT_OK(loop_device_make(block_loop->fd, O_RDWR, 0, UINT64_MAX, 0, 0, LOCK_SH, &loop));
+        ASSERT_FALSE(loop->created);
+        loop = loop_device_unref(loop);
+
+        /* With LO_FLAGS_PARTSCAN: backing loop has partscan disabled, so a new loop device with
+         * partscan must be created. */
+        ASSERT_OK(loop_device_make(block_loop->fd, O_RDWR, 0, UINT64_MAX, 0, LO_FLAGS_PARTSCAN, LOCK_SH, &loop));
+        ASSERT_TRUE(loop->created);
+        loop = loop_device_unref(loop);
+}
+
 DEFINE_TEST_MAIN_WITH_INTRO(LOG_DEBUG, intro);