]> git.ipfire.org Git - thirdparty/systemd.git/commitdiff
loop-util: read kernel's uevent seqnum right before attaching a loopback device
authorLennart Poettering <lennart@poettering.net>
Tue, 20 Apr 2021 08:56:38 +0000 (10:56 +0200)
committerLennart Poettering <lennart@poettering.net>
Tue, 20 Apr 2021 15:13:56 +0000 (17:13 +0200)
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
src/shared/loop-util.h

index ce4a72a24a9c12309d808d2e082abbc4459d8e89..3615995de097065538d8197f23ad018cd4021a06 100644 (file)
@@ -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;
index 619b34716bf26b4aa890cdcf8a40c3b211e3996c..6df4f91c2270d85e82a92bc7f6dd558a4af03b6b 100644 (file)
@@ -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);