From e8467cd31c9d5da28c31a6559fbe40ccb663fa7f Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Thu, 6 Aug 2020 10:31:19 +0200 Subject: [PATCH 1/4] blockdev-util: add correct API for detecting if block device has partition scanning enabled Instead of checking the loopback ioctls, let's check sysfs, so that we catch all kinds of block devices, not just loopback block devices. --- src/basic/blockdev-util.c | 37 +++++++++++++++++++++++++++++++++++++ src/basic/blockdev-util.h | 2 ++ 2 files changed, 39 insertions(+) diff --git a/src/basic/blockdev-util.c b/src/basic/blockdev-util.c index 5f8212685b..21ff3ba1b1 100644 --- a/src/basic/blockdev-util.c +++ b/src/basic/blockdev-util.c @@ -214,3 +214,40 @@ int lock_whole_block_device(dev_t devt, int operation) { return TAKE_FD(lock_fd); } + +int blockdev_partscan_enabled(int fd) { + _cleanup_free_ char *p = NULL, *buf = NULL; + unsigned long long ull; + struct stat st; + int r; + + /* Checks if partition scanning is correctly enabled on the block device */ + + if (fstat(fd, &st) < 0) + return -errno; + + if (!S_ISBLK(st.st_mode)) + return -ENOTBLK; + + if (asprintf(&p, "/sys/dev/block/%u:%u/capability", major(st.st_rdev), minor(st.st_rdev)) < 0) + return -ENOMEM; + + r = read_one_line_file(p, &buf); + if (r == -ENOENT) /* If the capability file doesn't exist then we are most likely looking at a + * partition block device, not the whole block device. And that means we have no + * partition scanning on for it (we do for its parent, but not for the partition + * itself). */ + return false; + if (r < 0) + return r; + + r = safe_atollu_full(buf, 16, &ull); + if (r < 0) + return r; + +#ifndef GENHD_FL_NO_PART_SCAN +#define GENHD_FL_NO_PART_SCAN (0x0200) +#endif + + return !FLAGS_SET(ull, GENHD_FL_NO_PART_SCAN); +} diff --git a/src/basic/blockdev-util.h b/src/basic/blockdev-util.h index 1e7588f71c..58a7050f53 100644 --- a/src/basic/blockdev-util.h +++ b/src/basic/blockdev-util.h @@ -20,3 +20,5 @@ int get_block_device(const char *path, dev_t *dev); int get_block_device_harder(const char *path, dev_t *dev); int lock_whole_block_device(dev_t devt, int operation); + +int blockdev_partscan_enabled(int fd); From 834c15ec383ba7cf4be9d064c080d49294f98901 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Thu, 6 Aug 2020 10:35:29 +0200 Subject: [PATCH 2/4] dissect: use new blockdev_partscan_enabled() API where appropriate --- src/shared/dissect-image.c | 24 ++++++++++-------------- 1 file changed, 10 insertions(+), 14 deletions(-) diff --git a/src/shared/dissect-image.c b/src/shared/dissect-image.c index c98c84993e..d3f183b50c 100644 --- a/src/shared/dissect-image.c +++ b/src/shared/dissect-image.c @@ -215,22 +215,18 @@ static int wait_for_partitions_to_appear( break; r = -errno; if (r == -EINVAL) { - struct loop_info64 info; + /* If we are running on a block device that has partition scanning off, return an + * explicit recognizable error about this, so that callers can generate a proper + * message explaining the situation. */ - /* If we are running on a loop device that has partition scanning off, return - * an explicit recognizable error about this, so that callers can generate a - * proper message explaining the situation. */ + r = blockdev_partscan_enabled(fd); + if (r < 0) + return r; + if (r == 0) + return log_debug_errno(EPROTONOSUPPORT, + "Device is a loop device and partition scanning is off!"); - if (ioctl(fd, LOOP_GET_STATUS64, &info) >= 0) { -#if HAVE_VALGRIND_MEMCHECK_H - /* Valgrind currently doesn't know LOOP_GET_STATUS64. Remove this once it does */ - VALGRIND_MAKE_MEM_DEFINED(&info, sizeof(info)); -#endif - - if ((info.lo_flags & LO_FLAGS_PARTSCAN) == 0) - return log_debug_errno(EPROTONOSUPPORT, - "Device is a loop device and partition scanning is off!"); - } + return -EINVAL; /* original error */ } if (r != -EBUSY) return r; From 86c1c1f34522b960724a6adda5425712bcfd95f7 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Thu, 6 Aug 2020 09:47:14 +0200 Subject: [PATCH 3/4] loop-util: use new LOOP_CONFIGURE ioctl LOOP_CONFIGURE allows us to configure a loopback device in one ioctl instead of two, which is not just faster but also removes the race that udev might start probing the device before we adjusted things properly. Unfortunately LOOP_CONFIGURE is broken in regards to LO_FLAGS_PARTSCAN as of kernel 5.8.0. This patch contains a work-around for that, to fallback to old behaviour if partition scanning is requested but does not work. Sucks a bit. Proposed upstream fix for that issue: https://lkml.org/lkml/2020/8/6/97 --- src/basic/missing_loop.h | 15 +++++++ src/shared/loop-util.c | 89 +++++++++++++++++++++++++++++++--------- 2 files changed, 84 insertions(+), 20 deletions(-) create mode 100644 src/basic/missing_loop.h diff --git a/src/basic/missing_loop.h b/src/basic/missing_loop.h new file mode 100644 index 0000000000..5326777419 --- /dev/null +++ b/src/basic/missing_loop.h @@ -0,0 +1,15 @@ +/* SPDX-License-Identifier: LGPL-2.1+ */ +#pragma once + +#include + +#ifndef LOOP_CONFIGURE +struct loop_config { + __u32 fd; + __u32 block_size; + struct loop_info64 info; + __u64 __reserved[8]; +}; + +#define LOOP_CONFIGURE 0x4C0A +#endif diff --git a/src/shared/loop-util.c b/src/shared/loop-util.c index 4a593b05f3..8db2fed66f 100644 --- a/src/shared/loop-util.c +++ b/src/shared/loop-util.c @@ -14,20 +14,68 @@ #include #include "alloc-util.h" +#include "blockdev-util.h" #include "errno-util.h" #include "fd-util.h" #include "fileio.h" #include "loop-util.h" +#include "missing_loop.h" #include "parse-util.h" #include "stat-util.h" #include "stdio-util.h" #include "string-util.h" static void cleanup_clear_loop_close(int *fd) { - if (*fd >= 0) { - (void) ioctl(*fd, LOOP_CLR_FD); - (void) safe_close(*fd); + if (*fd < 0) + return; + + (void) ioctl(*fd, LOOP_CLR_FD); + (void) safe_close(*fd); +} + +static int loop_configure(int fd, const struct loop_config *c) { + int r; + + assert(fd >= 0); + assert(c); + + if (ioctl(fd, LOOP_CONFIGURE, c) < 0) { + /* Do fallback only if LOOP_CONFIGURE is not supported, propagate all other errors. Note that + * the kernel is weird: non-existing ioctls currently return EINVAL rather than ENOTTY on + * loopback block devices. They should fix that in the kernel, but in the meantime we accept + * both here. */ + if (!ERRNO_IS_NOT_SUPPORTED(errno) && errno != EINVAL) + return -errno; + } else { + if (!FLAGS_SET(c->info.lo_flags, LO_FLAGS_PARTSCAN)) + return 0; + + /* Kernel 5.8 vanilla doesn't properly propagate the partition scanning flag into the + * block device. Let's hence verify if things work correctly here before returning. */ + + r = blockdev_partscan_enabled(fd); + if (r < 0) + goto fail; + if (r > 0) + return 0; /* All is good. */ + + /* Otherwise, undo the attachment and use the old APIs */ + (void) ioctl(fd, LOOP_CLR_FD); } + + if (ioctl(fd, LOOP_SET_FD, c->fd) < 0) + return -errno; + + if (ioctl(fd, LOOP_SET_STATUS64, &c->info) < 0) { + r = -errno; + goto fail; + } + + return 0; + +fail: + (void) ioctl(fd, LOOP_CLR_FD); + return r; } int loop_device_make( @@ -39,7 +87,7 @@ int loop_device_make( LoopDevice **ret) { _cleanup_free_ char *loopdev = NULL; - struct loop_info64 info; + struct loop_config config; LoopDevice *d = NULL; struct stat st; int nr = -1, r; @@ -52,14 +100,14 @@ int loop_device_make( return -errno; if (S_ISBLK(st.st_mode)) { - if (ioctl(fd, LOOP_GET_STATUS64, &info) >= 0) { + if (ioctl(fd, LOOP_GET_STATUS64, &config.info) >= 0) { /* Oh! This is a loopback device? That's interesting! */ #if HAVE_VALGRIND_MEMCHECK_H /* Valgrind currently doesn't know LOOP_GET_STATUS64. Remove this once it does */ - VALGRIND_MAKE_MEM_DEFINED(&info, sizeof(info)); + VALGRIND_MAKE_MEM_DEFINED(&config.info, sizeof(config.info)); #endif - nr = info.lo_number; + nr = config.info.lo_number; if (asprintf(&loopdev, "/dev/loop%i", nr) < 0) return -ENOMEM; @@ -100,6 +148,16 @@ int loop_device_make( if (control < 0) return -errno; + config = (struct loop_config) { + .fd = fd, + .info = { + /* Use the specified flags, but configure the read-only flag from the open flags, and force autoclear */ + .lo_flags = (loop_flags & ~LO_FLAGS_READ_ONLY) | ((loop_flags & O_ACCMODE) == O_RDONLY ? LO_FLAGS_READ_ONLY : 0) | LO_FLAGS_AUTOCLEAR, + .lo_offset = offset, + .lo_sizelimit = size == UINT64_MAX ? 0 : size, + }, + }; + /* Loop around LOOP_CTL_GET_FREE, since at the moment we attempt to open the returned device it might * be gone already, taken by somebody else racing against us. */ for (unsigned n_attempts = 0;;) { @@ -119,12 +177,13 @@ int loop_device_make( if (errno != ENOENT) return -errno; } else { - if (ioctl(loop, LOOP_SET_FD, fd) >= 0) { + r = loop_configure(loop, &config); + if (r >= 0) { loop_with_fd = TAKE_FD(loop); break; } - if (errno != EBUSY) - return -errno; + if (r != -EBUSY) + return r; } if (++n_attempts >= 64) /* Give up eventually */ @@ -133,16 +192,6 @@ int loop_device_make( loopdev = mfree(loopdev); } - info = (struct loop_info64) { - /* Use the specified flags, but configure the read-only flag from the open flags, and force autoclear */ - .lo_flags = (loop_flags & ~LO_FLAGS_READ_ONLY) | ((loop_flags & O_ACCMODE) == O_RDONLY ? LO_FLAGS_READ_ONLY : 0) | LO_FLAGS_AUTOCLEAR, - .lo_offset = offset, - .lo_sizelimit = size == UINT64_MAX ? 0 : size, - }; - - if (ioctl(loop_with_fd, LOOP_SET_STATUS64, &info) < 0) - return -errno; - d = new(LoopDevice, 1); if (!d) return -ENOMEM; From d8857af4d053b6d10aea7684aad247bebec4a5e8 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Thu, 6 Aug 2020 10:41:20 +0200 Subject: [PATCH 4/4] update TODO --- TODO | 3 --- 1 file changed, 3 deletions(-) diff --git a/TODO b/TODO index c0fbf8bc6a..07f21fc06e 100644 --- a/TODO +++ b/TODO @@ -20,9 +20,6 @@ Features: * make use of the new statx mountid and rootmount fields in path_get_mnt_id() and fd_is_mount_point() -* make use of new loopback setup ioctl to setup loopback devices in one atomic - ioctl - * nspawn: move "incoming mount" directory to /run/host, move "inaccessible" nodes to /run/host, move notify socket (for sd_notify() between payload and container manager)