From 31c75fcc415056e7ded80136547fd4658fa554f7 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 20 Apr 2021 10:56:38 +0200 Subject: loop-util: read kernel's uevent seqnum right before attaching a loopback device Later, this will allow us to ignore uevents from earlier attachments a bit better, as we can compare uevent seqnums with this boundary. It's not a full fix for the race though, since we cannot atomically determine the uevent and attach the device, but it at least shortens the window a bit. --- src/shared/loop-util.c | 48 ++++++++++++++++++++++++++++++++++++++++++++++-- src/shared/loop-util.h | 1 + 2 files changed, 47 insertions(+), 2 deletions(-) diff --git a/src/shared/loop-util.c b/src/shared/loop-util.c index ce4a72a24a..3615995de0 100644 --- a/src/shared/loop-util.c +++ b/src/shared/loop-util.c @@ -53,6 +53,23 @@ static int loop_is_bound(int fd) { return true; /* bound! */ } +static int get_current_uevent_seqnum(uint64_t *ret) { + _cleanup_free_ char *p = NULL; + int r; + + r = read_full_virtual_file("/sys/kernel/uevent_seqnum", &p, NULL); + if (r < 0) + return log_debug_errno(r, "Failed to read current uevent sequence number: %m"); + + truncate_nl(p); + + r = safe_atou64(p, ret); + if (r < 0) + return log_debug_errno(r, "Failed to parse current uevent sequence number: %s", p); + + return 0; +} + static int device_has_block_children(sd_device *d) { _cleanup_(sd_device_enumerator_unrefp) sd_device_enumerator *e = NULL; const char *main_sn, *main_ss; @@ -114,11 +131,13 @@ static int loop_configure( int fd, int nr, const struct loop_config *c, - bool *try_loop_configure) { + bool *try_loop_configure, + uint64_t *ret_seqnum_not_before) { _cleanup_(sd_device_unrefp) sd_device *d = NULL; _cleanup_free_ char *sysname = NULL; _cleanup_close_ int lock_fd = -1; + uint64_t seqnum; int r; assert(fd >= 0); @@ -167,6 +186,16 @@ static int loop_configure( } if (*try_loop_configure) { + /* Acquire uevent seqnum immediately before attaching the loopback device. This allows + * callers to ignore all uevents with a seqnum before this one, if they need to associate + * uevent with this attachment. Doing so isn't race-free though, as uevents that happen in + * the window between this reading of the seqnum, and the LOOP_CONFIGURE call might still be + * mistaken as originating from our attachment, even though might be caused by an earlier + * use. But doing this at least shortens the race window a bit. */ + r = get_current_uevent_seqnum(&seqnum); + if (r < 0) + return r; + 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 @@ -224,10 +253,18 @@ static int loop_configure( goto fail; } + if (ret_seqnum_not_before) + *ret_seqnum_not_before = seqnum; + return 0; } } + /* Let's read the seqnum again, to shorten the window. */ + r = get_current_uevent_seqnum(&seqnum); + if (r < 0) + return r; + /* Since kernel commit 5db470e229e22b7eda6e23b5566e532c96fb5bc3 (kernel v5.0) the LOOP_SET_STATUS64 * ioctl can return EAGAIN in case we change the lo_offset field, if someone else is accessing the * block device while we try to reconfigure it. This is a pretty common case, since udev might @@ -255,6 +292,9 @@ static int loop_configure( random_u64_range(UINT64_C(240) * USEC_PER_MSEC * n_attempts/64)); } + if (ret_seqnum_not_before) + *ret_seqnum_not_before = seqnum; + return 0; fail: @@ -312,6 +352,7 @@ int loop_device_make( bool try_loop_configure = true; struct loop_config config; LoopDevice *d = NULL; + uint64_t seqnum = UINT64_MAX; struct stat st; int nr = -1, r; @@ -354,6 +395,7 @@ int loop_device_make( .node = TAKE_PTR(loopdev), .relinquished = true, /* It's not allocated by us, don't destroy it when this object is freed */ .devno = st.st_rdev, + .uevent_seqnum_not_before = UINT64_MAX, }; *ret = d; @@ -401,7 +443,7 @@ int loop_device_make( if (!IN_SET(errno, ENOENT, ENXIO)) return -errno; } else { - r = loop_configure(loop, nr, &config, &try_loop_configure); + r = loop_configure(loop, nr, &config, &try_loop_configure, &seqnum); if (r >= 0) { loop_with_fd = TAKE_FD(loop); break; @@ -438,6 +480,7 @@ int loop_device_make( .node = TAKE_PTR(loopdev), .nr = nr, .devno = st.st_rdev, + .uevent_seqnum_not_before = seqnum, }; *ret = d; @@ -573,6 +616,7 @@ int loop_device_open(const char *loop_path, int open_flags, LoopDevice **ret) { .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, + .uevent_seqnum_not_before = UINT64_MAX, }; *ret = d; diff --git a/src/shared/loop-util.h b/src/shared/loop-util.h index 619b34716b..6df4f91c22 100644 --- a/src/shared/loop-util.h +++ b/src/shared/loop-util.h @@ -13,6 +13,7 @@ struct LoopDevice { dev_t devno; char *node; bool relinquished; + uint64_t uevent_seqnum_not_before; /* uevent sequm right before we attached the loopback device, or UINT64_MAX if we don't know */ }; int loop_device_make(int fd, int open_flags, uint64_t offset, uint64_t size, uint32_t loop_flags, LoopDevice **ret); -- cgit v1.2.1