Skip to content

Commit 89e524c

Browse files
jankaraaxboe
jankara
authored and
axboe
committed
loop: Fix mount(2) failure due to race with LOOP_SET_FD
Commit 33ec3e5 ("loop: Don't change loop device under exclusive opener") made LOOP_SET_FD ioctl acquire exclusive block device reference while it updates loop device binding. However this can make perfectly valid mount(2) fail with EBUSY due to racing LOOP_SET_FD holding temporarily the exclusive bdev reference in cases like this: for i in {a..z}{a..z}; do dd if=/dev/zero of=$i.image bs=1k count=0 seek=1024 mkfs.ext2 $i.image mkdir mnt$i done echo "Run" for i in {a..z}{a..z}; do mount -o loop -t ext2 $i.image mnt$i & done Fix the problem by not getting full exclusive bdev reference in LOOP_SET_FD but instead just mark the bdev as being claimed while we update the binding information. This just blocks new exclusive openers instead of failing them with EBUSY thus fixing the problem. Fixes: 33ec3e5 ("loop: Don't change loop device under exclusive opener") Cc: [email protected] Tested-by: Kai-Heng Feng <[email protected]> Signed-off-by: Jan Kara <[email protected]> Signed-off-by: Jens Axboe <[email protected]>
1 parent 71d6c50 commit 89e524c

File tree

3 files changed

+73
-32
lines changed

3 files changed

+73
-32
lines changed

‎drivers/block/loop.c

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -924,6 +924,7 @@ static int loop_set_fd(struct loop_device *lo, fmode_t mode,
924924
struct file *file;
925925
struct inode *inode;
926926
struct address_space *mapping;
927+
struct block_device *claimed_bdev = NULL;
927928
int lo_flags = 0;
928929
int error;
929930
loff_t size;
@@ -942,10 +943,11 @@ static int loop_set_fd(struct loop_device *lo, fmode_t mode,
942943
* here to avoid changing device under exclusive owner.
943944
*/
944945
if (!(mode & FMODE_EXCL)) {
945-
bdgrab(bdev);
946-
error = blkdev_get(bdev, mode | FMODE_EXCL, loop_set_fd);
947-
if (error)
946+
claimed_bdev = bd_start_claiming(bdev, loop_set_fd);
947+
if (IS_ERR(claimed_bdev)) {
948+
error = PTR_ERR(claimed_bdev);
948949
goto out_putf;
950+
}
949951
}
950952

951953
error = mutex_lock_killable(&loop_ctl_mutex);
@@ -1015,15 +1017,15 @@ static int loop_set_fd(struct loop_device *lo, fmode_t mode,
10151017
mutex_unlock(&loop_ctl_mutex);
10161018
if (partscan)
10171019
loop_reread_partitions(lo, bdev);
1018-
if (!(mode & FMODE_EXCL))
1019-
blkdev_put(bdev, mode | FMODE_EXCL);
1020+
if (claimed_bdev)
1021+
bd_abort_claiming(bdev, claimed_bdev, loop_set_fd);
10201022
return 0;
10211023

10221024
out_unlock:
10231025
mutex_unlock(&loop_ctl_mutex);
10241026
out_bdev:
1025-
if (!(mode & FMODE_EXCL))
1026-
blkdev_put(bdev, mode | FMODE_EXCL);
1027+
if (claimed_bdev)
1028+
bd_abort_claiming(bdev, claimed_bdev, loop_set_fd);
10271029
out_putf:
10281030
fput(file);
10291031
out:

‎fs/block_dev.c

Lines changed: 58 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -1181,8 +1181,7 @@ static struct gendisk *bdev_get_gendisk(struct block_device *bdev, int *partno)
11811181
* Pointer to the block device containing @bdev on success, ERR_PTR()
11821182
* value on failure.
11831183
*/
1184-
static struct block_device *bd_start_claiming(struct block_device *bdev,
1185-
void *holder)
1184+
struct block_device *bd_start_claiming(struct block_device *bdev, void *holder)
11861185
{
11871186
struct gendisk *disk;
11881187
struct block_device *whole;
@@ -1229,6 +1228,62 @@ static struct block_device *bd_start_claiming(struct block_device *bdev,
12291228
return ERR_PTR(err);
12301229
}
12311230
}
1231+
EXPORT_SYMBOL(bd_start_claiming);
1232+
1233+
static void bd_clear_claiming(struct block_device *whole, void *holder)
1234+
{
1235+
lockdep_assert_held(&bdev_lock);
1236+
/* tell others that we're done */
1237+
BUG_ON(whole->bd_claiming != holder);
1238+
whole->bd_claiming = NULL;
1239+
wake_up_bit(&whole->bd_claiming, 0);
1240+
}
1241+
1242+
/**
1243+
* bd_finish_claiming - finish claiming of a block device
1244+
* @bdev: block device of interest
1245+
* @whole: whole block device (returned from bd_start_claiming())
1246+
* @holder: holder that has claimed @bdev
1247+
*
1248+
* Finish exclusive open of a block device. Mark the device as exlusively
1249+
* open by the holder and wake up all waiters for exclusive open to finish.
1250+
*/
1251+
void bd_finish_claiming(struct block_device *bdev, struct block_device *whole,
1252+
void *holder)
1253+
{
1254+
spin_lock(&bdev_lock);
1255+
BUG_ON(!bd_may_claim(bdev, whole, holder));
1256+
/*
1257+
* Note that for a whole device bd_holders will be incremented twice,
1258+
* and bd_holder will be set to bd_may_claim before being set to holder
1259+
*/
1260+
whole->bd_holders++;
1261+
whole->bd_holder = bd_may_claim;
1262+
bdev->bd_holders++;
1263+
bdev->bd_holder = holder;
1264+
bd_clear_claiming(whole, holder);
1265+
spin_unlock(&bdev_lock);
1266+
}
1267+
EXPORT_SYMBOL(bd_finish_claiming);
1268+
1269+
/**
1270+
* bd_abort_claiming - abort claiming of a block device
1271+
* @bdev: block device of interest
1272+
* @whole: whole block device (returned from bd_start_claiming())
1273+
* @holder: holder that has claimed @bdev
1274+
*
1275+
* Abort claiming of a block device when the exclusive open failed. This can be
1276+
* also used when exclusive open is not actually desired and we just needed
1277+
* to block other exclusive openers for a while.
1278+
*/
1279+
void bd_abort_claiming(struct block_device *bdev, struct block_device *whole,
1280+
void *holder)
1281+
{
1282+
spin_lock(&bdev_lock);
1283+
bd_clear_claiming(whole, holder);
1284+
spin_unlock(&bdev_lock);
1285+
}
1286+
EXPORT_SYMBOL(bd_abort_claiming);
12321287

12331288
#ifdef CONFIG_SYSFS
12341289
struct bd_holder_disk {
@@ -1698,29 +1753,7 @@ int blkdev_get(struct block_device *bdev, fmode_t mode, void *holder)
16981753

16991754
/* finish claiming */
17001755
mutex_lock(&bdev->bd_mutex);
1701-
spin_lock(&bdev_lock);
1702-
1703-
if (!res) {
1704-
BUG_ON(!bd_may_claim(bdev, whole, holder));
1705-
/*
1706-
* Note that for a whole device bd_holders
1707-
* will be incremented twice, and bd_holder
1708-
* will be set to bd_may_claim before being
1709-
* set to holder
1710-
*/
1711-
whole->bd_holders++;
1712-
whole->bd_holder = bd_may_claim;
1713-
bdev->bd_holders++;
1714-
bdev->bd_holder = holder;
1715-
}
1716-
1717-
/* tell others that we're done */
1718-
BUG_ON(whole->bd_claiming != holder);
1719-
whole->bd_claiming = NULL;
1720-
wake_up_bit(&whole->bd_claiming, 0);
1721-
1722-
spin_unlock(&bdev_lock);
1723-
1756+
bd_finish_claiming(bdev, whole, holder);
17241757
/*
17251758
* Block event polling for write claims if requested. Any
17261759
* write holder makes the write_holder state stick until

‎include/linux/fs.h

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2598,6 +2598,12 @@ extern struct block_device *blkdev_get_by_path(const char *path, fmode_t mode,
25982598
void *holder);
25992599
extern struct block_device *blkdev_get_by_dev(dev_t dev, fmode_t mode,
26002600
void *holder);
2601+
extern struct block_device *bd_start_claiming(struct block_device *bdev,
2602+
void *holder);
2603+
extern void bd_finish_claiming(struct block_device *bdev,
2604+
struct block_device *whole, void *holder);
2605+
extern void bd_abort_claiming(struct block_device *bdev,
2606+
struct block_device *whole, void *holder);
26012607
extern void blkdev_put(struct block_device *bdev, fmode_t mode);
26022608
extern int __blkdev_reread_part(struct block_device *bdev);
26032609
extern int blkdev_reread_part(struct block_device *bdev);

0 commit comments

Comments
 (0)