From ffcb33241e182a4f7b47f39ca84b49c9aa7ae314 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Mon, 5 Sep 2022 02:05:52 +0900 Subject: [PATCH 1/6] loop-util: also set LoopDevice.diskseq when created with loop_device_open() --- src/shared/loop-util.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/shared/loop-util.c b/src/shared/loop-util.c index 928792281c..90451415ff 100644 --- a/src/shared/loop-util.c +++ b/src/shared/loop-util.c @@ -800,6 +800,7 @@ int loop_device_open( _cleanup_close_ int loop_fd = -1, lock_fd = -1; _cleanup_free_ char *p = NULL; struct loop_info64 info; + uint64_t diskseq = 0; struct stat st; LoopDevice *d; int nr; @@ -826,6 +827,10 @@ int loop_device_open( } else nr = -1; + r = fd_get_diskseq(loop_fd, &diskseq); + if (r < 0 && r != -EOPNOTSUPP) + return r; + if ((lock_op & ~LOCK_NB) != LOCK_UN) { lock_fd = open_lock_fd(loop_fd, lock_op); if (lock_fd < 0) @@ -847,6 +852,7 @@ int loop_device_open( .node = TAKE_PTR(p), .relinquished = true, /* It's not ours, don't try to destroy it when this object is freed */ .devno = st.st_dev, + .diskseq = diskseq, .uevent_seqnum_not_before = UINT64_MAX, .timestamp_not_before = USEC_INFINITY, }; From 9b5626d67a63d7aae019d533d5c7173d7e1a2690 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Mon, 5 Sep 2022 02:39:16 +0900 Subject: [PATCH 2/6] loop-util: fix LoopDevice.devno assigned by loop_device_open() --- src/shared/loop-util.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/shared/loop-util.c b/src/shared/loop-util.c index 90451415ff..c376244cb7 100644 --- a/src/shared/loop-util.c +++ b/src/shared/loop-util.c @@ -851,7 +851,7 @@ int loop_device_open( .nr = nr, .node = TAKE_PTR(p), .relinquished = true, /* It's not ours, don't try to destroy it when this object is freed */ - .devno = st.st_dev, + .devno = st.st_rdev, .diskseq = diskseq, .uevent_seqnum_not_before = UINT64_MAX, .timestamp_not_before = USEC_INFINITY, From a8d8a61980df857244bbc929471c94fdb532507b Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Mon, 5 Sep 2022 02:48:01 +0900 Subject: [PATCH 3/6] loop-util: introduce loop_device_open_full() --- src/shared/loop-util.c | 52 +++++++++++++++++++++++++++++++----------- src/shared/loop-util.h | 5 +++- 2 files changed, 43 insertions(+), 14 deletions(-) diff --git a/src/shared/loop-util.c b/src/shared/loop-util.c index c376244cb7..4114ea1f4d 100644 --- a/src/shared/loop-util.c +++ b/src/shared/loop-util.c @@ -791,41 +791,55 @@ void loop_device_unrelinquish(LoopDevice *d) { d->relinquished = false; } -int loop_device_open( +int loop_device_open_full( const char *loop_path, + int loop_fd, int open_flags, int lock_op, LoopDevice **ret) { - _cleanup_close_ int loop_fd = -1, lock_fd = -1; + _cleanup_close_ int fd = -1, lock_fd = -1; _cleanup_free_ char *p = NULL; struct loop_info64 info; uint64_t diskseq = 0; struct stat st; LoopDevice *d; - int nr; + int r, nr = -1; - assert(loop_path); + assert(loop_path || loop_fd >= 0); assert(IN_SET(open_flags, O_RDWR, O_RDONLY)); assert(ret); - loop_fd = open(loop_path, O_CLOEXEC|O_NONBLOCK|O_NOCTTY|open_flags); - if (loop_fd < 0) - return -errno; + if (loop_fd < 0) { + fd = open(loop_path, O_CLOEXEC|O_NONBLOCK|O_NOCTTY|open_flags); + if (fd < 0) + return -errno; + loop_fd = fd; + } if (fstat(loop_fd, &st) < 0) return -errno; if (!S_ISBLK(st.st_mode)) return -ENOTBLK; + if (fd < 0) { + /* If loop_fd is provided through the argument, then we reopen the inode here, instead of + * keeping just a dup() clone of it around, since we want to ensure that the O_DIRECT + * flag of the handle we keep is off, we have our own file index, and have the right + * read/write mode in effect.*/ + fd = fd_reopen(loop_fd, O_CLOEXEC|O_NONBLOCK|O_NOCTTY|open_flags); + if (fd < 0) + return fd; + loop_fd = fd; + } + if (ioctl(loop_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 nr = info.lo_number; - } else - nr = -1; + } r = fd_get_diskseq(loop_fd, &diskseq); if (r < 0 && r != -EOPNOTSUPP) @@ -837,16 +851,28 @@ int loop_device_open( return lock_fd; } - p = strdup(loop_path); - if (!p) - return -ENOMEM; + if (loop_path) { + /* If loop_path is provided, then honor it. */ + p = strdup(loop_path); + if (!p) + return -ENOMEM; + } else if (nr >= 0) { + /* This is a loopback block device. Use its index. */ + if (asprintf(&p, "/dev/loop%i", nr) < 0) + return -ENOMEM; + } else { + /* This is a non-loopback block device. Let's get the path to the device node. */ + r = devname_from_stat_rdev(&st, &p); + if (r < 0) + return r; + } d = new(LoopDevice, 1); if (!d) return -ENOMEM; *d = (LoopDevice) { - .fd = TAKE_FD(loop_fd), + .fd = TAKE_FD(fd), .lock_fd = TAKE_FD(lock_fd), .nr = nr, .node = TAKE_PTR(p), diff --git a/src/shared/loop-util.h b/src/shared/loop-util.h index bdbdac60eb..89fcbc00a0 100644 --- a/src/shared/loop-util.h +++ b/src/shared/loop-util.h @@ -25,7 +25,10 @@ struct LoopDevice { int loop_device_make(int fd, int open_flags, uint64_t offset, uint64_t size, uint32_t loop_flags, int lock_op, LoopDevice **ret); int loop_device_make_by_path(const char *path, int open_flags, uint32_t loop_flags, int lock_op, LoopDevice **ret); -int loop_device_open(const char *loop_path, int open_flags, int lock_op, LoopDevice **ret); +int loop_device_open_full(const char *loop_path, int loop_fd, int open_flags, int lock_op, LoopDevice **ret); +static inline int loop_device_open(const char *loop_path, int open_flags, int lock_op, LoopDevice **ret) { + return loop_device_open_full(loop_path, -1, open_flags, lock_op, ret); +} LoopDevice* loop_device_unref(LoopDevice *d); DEFINE_TRIVIAL_CLEANUP_FUNC(LoopDevice*, loop_device_unref); From 1996ad2854e94313735097d024ab0cd1353d13dd Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Mon, 5 Sep 2022 02:57:29 +0900 Subject: [PATCH 4/6] loop-util: use loop_device_open_full() when whole block device is passed to loop_device_make() This also fixes a leak of lock_fd, which introduced by 7f52206a2bc128f9ae8306db43aa6e2f7d916f82, when fd is for a block device, and size or offset is non-zero. Fixes another issue in #24147. --- src/shared/loop-util.c | 62 ++---------------------------------------- 1 file changed, 3 insertions(+), 59 deletions(-) diff --git a/src/shared/loop-util.c b/src/shared/loop-util.c index 4114ea1f4d..df4cf1f0a1 100644 --- a/src/shared/loop-util.c +++ b/src/shared/loop-util.c @@ -375,67 +375,11 @@ static int loop_device_make_internal( return -errno; if (S_ISBLK(st.st_mode)) { - if (lock_op != LOCK_UN) { - lock_fd = open_lock_fd(fd, lock_op); - if (lock_fd < 0) - return lock_fd; - } - - 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(&config.info, sizeof(config.info)); -#endif - nr = config.info.lo_number; - - if (asprintf(&node, "/dev/loop%i", nr) < 0) - return -ENOMEM; - } else { - /* This is a non-loopback block device. Let's get the path to the device node. */ - r = devname_from_stat_rdev(&st, &node); - if (r < 0) - return r; - } - - if (offset == 0 && IN_SET(size, 0, UINT64_MAX)) { - _cleanup_close_ int copy = -1; - uint64_t diskseq = 0; - + if (offset == 0 && IN_SET(size, 0, UINT64_MAX)) /* 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. Note that we reopen the inode here, instead of - * keeping just a dup() clone of it around, since we want to ensure that the O_DIRECT - * flag of the handle we keep is off, we have our own file index, and have the right - * read/write mode in effect. */ - - copy = fd_reopen(fd, open_flags|O_NONBLOCK|O_CLOEXEC|O_NOCTTY); - if (copy < 0) - return copy; - - r = fd_get_diskseq(copy, &diskseq); - if (r < 0 && r != -EOPNOTSUPP) - return r; - - d = new(LoopDevice, 1); - if (!d) - return -ENOMEM; - *d = (LoopDevice) { - .fd = TAKE_FD(copy), - .lock_fd = TAKE_FD(lock_fd), - .nr = nr, - .node = TAKE_PTR(node), - .relinquished = true, /* It's not allocated by us, don't destroy it when this object is freed */ - .devno = st.st_rdev, - .diskseq = diskseq, - .uevent_seqnum_not_before = UINT64_MAX, - .timestamp_not_before = USEC_INFINITY, - }; - - *ret = d; - return d->fd; - } + * unnecessary loopback device for it. */ + return loop_device_open_full(NULL, fd, open_flags, lock_op, ret); } else { r = stat_verify_regular(&st); if (r < 0) From 9bf8600774689d34cfa5172c6eaba2093e095e23 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Mon, 5 Sep 2022 02:57:49 +0900 Subject: [PATCH 5/6] loop-util: drop unnecessary initializations --- src/shared/loop-util.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/shared/loop-util.c b/src/shared/loop-util.c index df4cf1f0a1..3cc72b65a0 100644 --- a/src/shared/loop-util.c +++ b/src/shared/loop-util.c @@ -361,10 +361,10 @@ static int loop_device_make_internal( _cleanup_free_ char *node = NULL; bool try_loop_configure = true; struct loop_config config; - LoopDevice *d = NULL; + LoopDevice *d; uint64_t seqnum = UINT64_MAX; usec_t timestamp = USEC_INFINITY; - int nr = -1, r, f_flags; + int nr, r, f_flags; struct stat st; assert(fd >= 0); From e42270b6a695b4aaf324d4570744f4b289184718 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Mon, 5 Sep 2022 03:04:07 +0900 Subject: [PATCH 6/6] loop-util: lock_fd must be closed before calling LOOP_CLR_FD Follow-up for 7f52206a2bc128f9ae8306db43aa6e2f7d916f82. C.f. 87862cc2b4abb9564f7e0365ac515dc9020a54e4. --- src/shared/loop-util.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/shared/loop-util.c b/src/shared/loop-util.c index 3cc72b65a0..ed237c6f20 100644 --- a/src/shared/loop-util.c +++ b/src/shared/loop-util.c @@ -357,7 +357,7 @@ static int loop_device_make_internal( int lock_op, LoopDevice **ret) { - _cleanup_close_ int direct_io_fd = -1, lock_fd = -1; + _cleanup_close_ int direct_io_fd = -1; _cleanup_free_ char *node = NULL; bool try_loop_configure = true; struct loop_config config; @@ -410,8 +410,10 @@ static int loop_device_make_internal( fd = direct_io_fd; /* From now on, operate on our new O_DIRECT fd */ } + /* On failure, lock_fd must be closed at first, otherwise LOOP_CLR_FD will fail. */ _cleanup_close_ int control = -1; _cleanup_(cleanup_clear_loop_close) int loop_with_fd = -1; + _cleanup_close_ int lock_fd = -1; control = open("/dev/loop-control", O_RDWR|O_CLOEXEC|O_NOCTTY|O_NONBLOCK); if (control < 0)