loop: fix partition scan race between udev and loop_reread_partitions()
PATCH 1/1 loop: fix partition scan race between udev and loop_reread_partitions()
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.
--- drivers/block/loop.c | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-)
drivers/block/loop.c +17 -1
@@ -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 );
Signature · 1 line
2.53.0
PATCH v2 1/1 loop: fix partition scan race between udev and loop_reread_partitions()
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.
--- drivers/block/loop.c | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-)
drivers/block/loop.c +17 -1
@@ -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 );
Signature · 1 line
2.53.0
Quoted 6 lines · Daan De Meyer
Fix this by clearing
GD_NEED_PART_SCANbefore sending the uevent whenLO_FLAGS_PARTSCANis set. Sinceloop_reread_partitions()will perform the authoritative partition scan, the lazy on-open scan triggered byGD_NEED_PART_SCANis redundant. With the flag cleared, udev opening the device no longer triggers a scan inblkdev_get_whole(), and only the single explicit scan fromloop_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?