From: Hou Tao houtao1@huawei.com

hulk inclusion category: bugfix bugzilla: 55097 CVE: NA

-------------------------------------------------

This reverts commit 375e23ab3d24e67b13df536dc6f933c628f78c41.

The patch set for partition symlink cleanup will introduce deadlock for nbd, loop and xen-blkfront driver, so revert it.

Signed-off-by: Hou Tao houtao1@huawei.com Reviewed-by: Jason Yan yanaijie@huawei.com Signed-off-by: Yang Yingliang yangyingliang@huawei.com --- block/genhd.c | 2 +- block/ioctl.c | 4 ---- block/partition-generic.c | 3 --- 3 files changed, 1 insertion(+), 8 deletions(-)

diff --git a/block/genhd.c b/block/genhd.c index be938088c440f..1bf24be80cca9 100644 --- a/block/genhd.c +++ b/block/genhd.c @@ -782,7 +782,6 @@ void del_gendisk(struct gendisk *disk) bdev = bdget_disk(disk, 0); if (bdev) mutex_lock(&bdev->bd_mutex); - disk->flags &= ~GENHD_FL_UP; /* invalidate stuff */ disk_part_iter_init(&piter, disk, DISK_PITER_INCL_EMPTY | DISK_PITER_REVERSE); @@ -798,6 +797,7 @@ void del_gendisk(struct gendisk *disk) invalidate_partition(disk, 0); bdev_unhash_inode(disk_devt(disk)); set_capacity(disk, 0); + disk->flags &= ~GENHD_FL_UP; up_write(&disk->lookup_sem);

if (!(disk->flags & GENHD_FL_HIDDEN)) diff --git a/block/ioctl.c b/block/ioctl.c index 3bcfc8dc32fad..899ffd50a7c6b 100644 --- a/block/ioctl.c +++ b/block/ioctl.c @@ -50,10 +50,6 @@ static int blkpg_ioctl(struct block_device *bdev, struct blkpg_ioctl_arg __user return -EINVAL;

mutex_lock(&bdev->bd_mutex); - if (!(disk->flags & GENHD_FL_UP)) { - mutex_unlock(&bdev->bd_mutex); - return -ENXIO; - }

/* overlap? */ disk_part_iter_init(&piter, disk, diff --git a/block/partition-generic.c b/block/partition-generic.c index ddaa5d41139c6..63b82df5bbb40 100644 --- a/block/partition-generic.c +++ b/block/partition-generic.c @@ -525,9 +525,6 @@ int rescan_partitions(struct gendisk *disk, struct block_device *bdev) struct parsed_partitions *state = NULL; struct hd_struct *part; int p, highest, res; - - if (!(disk->flags & GENHD_FL_UP)) - return -ENXIO; rescan: if (state && !IS_ERR(state)) { free_partitions(state);

From: Hou Tao houtao1@huawei.com

hulk inclusion category: bugfix bugzilla: 55097 CVE: NA

-------------------------------------------------

Now there no protection between partition operations (e.g, partition rescan) and delete_partition() in del_gendisk(), so the following scenario is possible:

CPU 1

blkdev_ioctl del_gendisk blkdev_reread_part lock bd_mutex drop_partitions check_partition lock lookup_sem // for each partition deletion_partion

// for each partition add_partition

The newly added partitions, the device files (e.g, /dev/sdXN) and the symlinks in /sys/class/block will be left behind. If the deleted disk is online again, the scan of partition will fail with the following error:

sysfs: cannot create duplicate filename '/class/block/sdaN' sdX: pN could not be added: 17

Vanilla kernel tries to fix that by commit c76f48eb5c08 ("block: take bd_mutex around delete_partitions in del_gendisk"), but it introduces dead-lock for nbd/loop/xen-frontblk drivers. These in-tree drivers can be fixed, but there may be other affected block drivers, especially the out-of-tree ones, so fixing it in another way.

Two methods are considered. The first is waiting for the end of partition operations in del_gendisk(). It is OK but it needs adding new fields in gendisk (bool & wait_queue_head_t). The second is reusing lookup_sem and GENHD_FL_UP to serialize partition operations and del_gendisk(). Now the latter is chose and here are the details.

There are six partition operations:

(1) add_partition() in blkpg_ioctl() (2) deletion_partion() in blkpg_ioctl() (3) resize in blkpg_ioctl() (4) partition rescan in __blkdev_reread_part() (5) partition revalidate in bdev_disk_changed() (6) deletion_partion() in del_gendisk()

op (1)~(5) already take bd_mutex, so using down_read() to serialize with down_write() in del_gendisk() is OK. op (3) only updates the values in hd_struct, so no lock is needed, because it already increase the ref of hd_struct.

lookup_sem is used to prevent a newly-created blocking device inode from associating with a deleting gendisk, and the locking order is:

part->bd_mutex -> disk->lookup_sem or whole->bd_mutex -> disk->lookup_sem

Now it is also used to serialize the partition operations and the new locking order will be:

part->bd_mutex -> whole->bd_mutex -> disk->lookup_sem

and it is OK.

Signed-off-by: Hou Tao houtao1@huawei.com Reviewed-by: Jason Yan yanaijie@huawei.com Signed-off-by: Yang Yingliang yangyingliang@huawei.com --- block/ioctl.c | 19 ++++++++++++++++++- fs/block_dev.c | 11 ++++++++++- 2 files changed, 28 insertions(+), 2 deletions(-)

diff --git a/block/ioctl.c b/block/ioctl.c index 899ffd50a7c6b..93939b0cbf03e 100644 --- a/block/ioctl.c +++ b/block/ioctl.c @@ -64,9 +64,16 @@ static int blkpg_ioctl(struct block_device *bdev, struct blkpg_ioctl_arg __user } disk_part_iter_exit(&piter);

+ down_read(&disk->lookup_sem); + if (!(disk->flags & GENHD_FL_UP)) { + up_read(&disk->lookup_sem); + mutex_unlock(&bdev->bd_mutex); + return -ENXIO; + } /* all seems OK */ part = add_partition(disk, partno, start, length, ADDPART_FLAG_NONE, NULL); + up_read(&disk->lookup_sem); mutex_unlock(&bdev->bd_mutex); return PTR_ERR_OR_ZERO(part); case BLKPG_DEL_PARTITION: @@ -90,7 +97,9 @@ static int blkpg_ioctl(struct block_device *bdev, struct blkpg_ioctl_arg __user invalidate_bdev(bdevp);

mutex_lock_nested(&bdev->bd_mutex, 1); + down_read(&disk->lookup_sem); delete_partition(disk, partno); + up_read(&disk->lookup_sem); mutex_unlock(&bdev->bd_mutex); mutex_unlock(&bdevp->bd_mutex); bdput(bdevp); @@ -162,6 +171,7 @@ static int blkpg_ioctl(struct block_device *bdev, struct blkpg_ioctl_arg __user int __blkdev_reread_part(struct block_device *bdev) { struct gendisk *disk = bdev->bd_disk; + int err;

if (!disk_part_scan_enabled(disk) || bdev != bdev->bd_contains) return -EINVAL; @@ -170,7 +180,14 @@ int __blkdev_reread_part(struct block_device *bdev)

lockdep_assert_held(&bdev->bd_mutex);

- return rescan_partitions(disk, bdev); + down_read(&disk->lookup_sem); + if (disk->flags & GENHD_FL_UP) + err = rescan_partitions(disk, bdev); + else + err = -ENXIO; + up_read(&disk->lookup_sem); + + return err; } EXPORT_SYMBOL(__blkdev_reread_part);

diff --git a/fs/block_dev.c b/fs/block_dev.c index 77dbfc832bb9f..8b299347f2aa9 100644 --- a/fs/block_dev.c +++ b/fs/block_dev.c @@ -1499,11 +1499,20 @@ static void __blkdev_put(struct block_device *bdev, fmode_t mode, int for_part);

static void bdev_disk_changed(struct block_device *bdev, bool invalidate) { - if (disk_part_scan_enabled(bdev->bd_disk)) { + struct gendisk *disk = bdev->bd_disk; + + if (disk_part_scan_enabled(disk)) { + down_read(&disk->lookup_sem); + if (!(disk->flags & GENHD_FL_UP)) { + up_read(&disk->lookup_sem); + return; + } + if (invalidate) invalidate_partitions(bdev->bd_disk, bdev); else rescan_partitions(bdev->bd_disk, bdev); + up_read(&disk->lookup_sem); } else { check_disk_size_change(bdev->bd_disk, bdev, !invalidate); bdev->bd_invalidated = 0;