public inbox for linux-block@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH] loop: fix partition scan race between udev and loop_reread_partitions()
@ 2026-03-30  8:18 Daan De Meyer
  2026-03-30 11:03 ` [PATCH v2] " Daan De Meyer
  0 siblings, 1 reply; 3+ messages in thread
From: Daan De Meyer @ 2026-03-30  8:18 UTC (permalink / raw)
  To: linux-block; +Cc: axboe, brauner, Daan De Meyer

When LOOP_CONFIGURE is called with LO_FLAGS_PARTSCAN, the following
sequence occurs:

  1. disk_force_media_change() sets GD_NEED_PART_SCAN
  2. Uevent suppression is lifted and a KOBJ_CHANGE uevent is sent
  3. loop_global_unlock() releases the lock
  4. loop_reread_partitions() calls bdev_disk_changed() to scan

There is a race between steps 2 and 4: when udev receives the uevent
and opens the device before loop_reread_partitions() runs,
blkdev_get_whole() in bdev.c sees GD_NEED_PART_SCAN set and calls
bdev_disk_changed() for a first scan. Then loop_reread_partitions()
does a second scan. The open_mutex serializes these two scans, but
does not prevent both from running.

The second scan in bdev_disk_changed() drops all partition devices
from the first scan (via blk_drop_partitions()) before re-adding
them, causing partition block devices to briefly disappear. This
breaks any systemd unit with BindsTo= on the partition device: systemd
observes the device going dead, fails the dependent units, and does
not retry them when the device reappears.

Fix this by clearing GD_NEED_PART_SCAN before sending the uevent when
LO_FLAGS_PARTSCAN is set. Since loop_reread_partitions() will perform
the authoritative partition scan, the lazy on-open scan triggered by
GD_NEED_PART_SCAN is redundant. With the flag cleared, udev opening
the device no longer triggers a scan in blkdev_get_whole(), and only
the single explicit scan from loop_reread_partitions() runs.

When LO_FLAGS_PARTSCAN is not set, GD_NEED_PART_SCAN is left as-is
since loop_reread_partitions() will not be called and GD_SUPPRESS_PART_SCAN
remains set, preventing the lazy scan path from triggering anyway.

Signed-off-by: Daan De Meyer <daan@amutable.com>
---
 drivers/block/loop.c | 18 +++++++++++++++++-
 1 file changed, 17 insertions(+), 1 deletion(-)

diff --git a/drivers/block/loop.c b/drivers/block/loop.c
index 0000913f7efc..4e87d65c45e0 100644
--- a/drivers/block/loop.c
+++ b/drivers/block/loop.c
@@ -1085,9 +1085,25 @@ static int loop_configure(struct loop_device *lo, blk_mode_t mode,
 	if (part_shift)
 		lo->lo_flags |= LO_FLAGS_PARTSCAN;
 	partscan = lo->lo_flags & LO_FLAGS_PARTSCAN;
-	if (partscan)
+	if (partscan) {
 		clear_bit(GD_SUPPRESS_PART_SCAN, &lo->lo_disk->state);
 
+		/*
+		 * disk_force_media_change() above sets GD_NEED_PART_SCAN to
+		 * trigger a partition scan on the next device open. However,
+		 * when LO_FLAGS_PARTSCAN is set, loop_reread_partitions() below
+		 * will do the scan explicitly. If we leave GD_NEED_PART_SCAN
+		 * set, the uevent we're about to send causes udev to open the
+		 * device, which triggers blkdev_get_whole() to scan first, and
+		 * then loop_reread_partitions() scans again. The second scan
+		 * drops all partitions from the first scan before re-adding
+		 * them, causing partition devices to briefly disappear. Clear
+		 * the flag to prevent this since loop_reread_partitions() will
+		 * handle the scan.
+		 */
+		clear_bit(GD_NEED_PART_SCAN, &lo->lo_disk->state);
+	}
+
 	dev_set_uevent_suppress(disk_to_dev(lo->lo_disk), 0);
 	kobject_uevent(&disk_to_dev(lo->lo_disk)->kobj, KOBJ_CHANGE);
 
-- 
2.53.0


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* [PATCH v2] loop: fix partition scan race between udev and loop_reread_partitions()
  2026-03-30  8:18 [PATCH] loop: fix partition scan race between udev and loop_reread_partitions() Daan De Meyer
@ 2026-03-30 11:03 ` Daan De Meyer
  2026-03-31  7:23   ` Christoph Hellwig
  0 siblings, 1 reply; 3+ messages in thread
From: Daan De Meyer @ 2026-03-30 11:03 UTC (permalink / raw)
  To: linux-block; +Cc: axboe, brauner, Daan De Meyer

When LOOP_CONFIGURE is called with LO_FLAGS_PARTSCAN, the following
sequence occurs:

  1. disk_force_media_change() sets GD_NEED_PART_SCAN
  2. Uevent suppression is lifted and a KOBJ_CHANGE uevent is sent
  3. loop_global_unlock() releases the lock
  4. loop_reread_partitions() calls bdev_disk_changed() to scan

There is a race between steps 2 and 4: when udev receives the uevent
and opens the device before loop_reread_partitions() runs,
blkdev_get_whole() in bdev.c sees GD_NEED_PART_SCAN set and calls
bdev_disk_changed() for a first scan. Then loop_reread_partitions()
does a second scan. The open_mutex serializes these two scans, but
does not prevent both from running.

The second scan in bdev_disk_changed() drops all partition devices
from the first scan (via blk_drop_partitions()) before re-adding
them, causing partition block devices to briefly disappear. This
breaks any systemd unit with BindsTo= on the partition device: systemd
observes the device going dead, fails the dependent units, and does
not retry them when the device reappears.

Fix this by clearing GD_NEED_PART_SCAN before sending the uevent when
LO_FLAGS_PARTSCAN is set. Since loop_reread_partitions() will perform
the authoritative partition scan, the lazy on-open scan triggered by
GD_NEED_PART_SCAN is redundant. With the flag cleared, udev opening
the device no longer triggers a scan in blkdev_get_whole(), and only
the single explicit scan from loop_reread_partitions() runs.

When LO_FLAGS_PARTSCAN is not set, GD_NEED_PART_SCAN is left as-is
since loop_reread_partitions() will not be called and GD_SUPPRESS_PART_SCAN
remains set, preventing the lazy scan path from triggering anyway.

Fixes: 9f65c489b68d ("loop: raise media_change event")
Signed-off-by: Daan De Meyer <daan@amutable.com>
---
 drivers/block/loop.c | 18 +++++++++++++++++-
 1 file changed, 17 insertions(+), 1 deletion(-)

diff --git a/drivers/block/loop.c b/drivers/block/loop.c
index 0000913f7efc..4e87d65c45e0 100644
--- a/drivers/block/loop.c
+++ b/drivers/block/loop.c
@@ -1085,9 +1085,25 @@ static int loop_configure(struct loop_device *lo, blk_mode_t mode,
 	if (part_shift)
 		lo->lo_flags |= LO_FLAGS_PARTSCAN;
 	partscan = lo->lo_flags & LO_FLAGS_PARTSCAN;
-	if (partscan)
+	if (partscan) {
 		clear_bit(GD_SUPPRESS_PART_SCAN, &lo->lo_disk->state);
 
+		/*
+		 * disk_force_media_change() above sets GD_NEED_PART_SCAN to
+		 * trigger a partition scan on the next device open. However,
+		 * when LO_FLAGS_PARTSCAN is set, loop_reread_partitions() below
+		 * will do the scan explicitly. If we leave GD_NEED_PART_SCAN
+		 * set, the uevent we're about to send causes udev to open the
+		 * device, which triggers blkdev_get_whole() to scan first, and
+		 * then loop_reread_partitions() scans again. The second scan
+		 * drops all partitions from the first scan before re-adding
+		 * them, causing partition devices to briefly disappear. Clear
+		 * the flag to prevent this since loop_reread_partitions() will
+		 * handle the scan.
+		 */
+		clear_bit(GD_NEED_PART_SCAN, &lo->lo_disk->state);
+	}
+
 	dev_set_uevent_suppress(disk_to_dev(lo->lo_disk), 0);
 	kobject_uevent(&disk_to_dev(lo->lo_disk)->kobj, KOBJ_CHANGE);
 
-- 
2.53.0


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH v2] loop: fix partition scan race between udev and loop_reread_partitions()
  2026-03-30 11:03 ` [PATCH v2] " Daan De Meyer
@ 2026-03-31  7:23   ` Christoph Hellwig
  0 siblings, 0 replies; 3+ messages in thread
From: Christoph Hellwig @ 2026-03-31  7:23 UTC (permalink / raw)
  To: Daan De Meyer; +Cc: linux-block, axboe, brauner, Daan De Meyer

On Mon, Mar 30, 2026 at 11:03:10AM +0000, Daan De Meyer wrote:
> Fix this by clearing GD_NEED_PART_SCAN before sending the uevent when
> LO_FLAGS_PARTSCAN is set. Since loop_reread_partitions() will perform
> the authoritative partition scan, the lazy on-open scan triggered by
> GD_NEED_PART_SCAN is redundant. With the flag cleared, udev opening
> the device no longer triggers a scan in blkdev_get_whole(), and only
> the single explicit scan from loop_reread_partitions() runs.

Setting the flag and instantly clearing them seems like a bad idea.
So add a variant that doesn't set it instead, or audit the callers
for who even needs the flag set.  floppy doesn't because it doesn't
support partitions, and the other callers in loop also look
suspicious.

Also can you wire up a test for this in blktests?

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2026-03-31  7:23 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-03-30  8:18 [PATCH] loop: fix partition scan race between udev and loop_reread_partitions() Daan De Meyer
2026-03-30 11:03 ` [PATCH v2] " Daan De Meyer
2026-03-31  7:23   ` Christoph Hellwig

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox