[PATCH openEuler-1.0-LTS 1/3] nbd: get config_lock before sock_shutdown
From: Zhong Jinghua <zhongjinghua@huaweicloud.com> hulk inclusion category: bugfix bugzilla: 188799, https://gitee.com/openeuler/kernel/issues/I79QWO CVE: NA ---------------------------------------- Config->socks in sock_shutdown may trigger a UAF problem. The reason is that sock_shutdown does not hold the config_lock, so that nbd_ioctl can release config->socks at this time. T0: NBD_SET_SOCK T1: NBD_DO_IT T0 T1 nbd_ioctl mutex_lock(&nbd->config_lock) // get lock __nbd_ioctl nbd_start_device_ioctl nbd_start_device mutex_unlock(&nbd->config_lock) // relase lock wait_event_interruptible (kill, enter sock_shutdown) sock_shutdown nbd_ioctl mutex_lock(&nbd->config_lock) // get lock __nbd_ioctl nbd_add_socket krealloc kfree(p) //config->socks is NULL nbd_sock *nsock = config->socks // error Fix it by moving config_lock up before sock_shutdown. Signed-off-by: Zhong Jinghua <zhongjinghua@huawei.com> Reviewed-by: Yu Kuai <yukuai3@huawei.com> Reviewed-by: Hou Tao <houtao1@huawei.com> Signed-off-by: Yongqiang Liu <liuyongqiang13@huawei.com> --- drivers/block/nbd.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c index 551b2f7da8ef..a08f35946718 100644 --- a/drivers/block/nbd.c +++ b/drivers/block/nbd.c @@ -1347,11 +1347,16 @@ static int nbd_start_device_ioctl(struct nbd_device *nbd, struct block_device *b mutex_unlock(&nbd->config_lock); ret = wait_event_interruptible(config->recv_wq, atomic_read(&config->recv_threads) == 0); + + /* + * recv_work in flush_workqueue will not get this lock, because nbd_open + * will hold nbd->config_refs + */ + mutex_lock(&nbd->config_lock); if (ret) sock_shutdown(nbd); flush_workqueue(nbd->recv_workq); - mutex_lock(&nbd->config_lock); nbd_bdev_reset(bdev); /* user requested, ignore socket errors */ if (test_bit(NBD_RT_DISCONNECT_REQUESTED, &config->runtime_flags)) -- 2.25.1
From: Christoph Hellwig <hch@lst.de> mainline inclusion from mainline-v5.8-rc1 commit fa9156ae597c244df4e12891dc8329f649970d9a category: bugfix bugzilla: 187268, https://gitee.com/openeuler/kernel/issues/I76JDY CVE: NA ---------------------------------------- Split each sub-command out into a separate helper, and move those helpers to block/partitions/core.c instead of having a lot of partition manipulation logic open coded in block/ioctl.c. Signed-off-by: Christoph Hellwig <hch@lst.de Signed-off-by: Jens Axboe <axboe@kernel.dk> conflicts: block/ioctl.c block/partition-generic.c include/linux/genhd.h Signed-off-by: Zhong Jinghua <zhongjinghua@huawei.com> Reviewed-by: Hou Tao <houtao1@huawei.com> Signed-off-by: Yongqiang Liu <liuyongqiang13@huawei.com> --- block/ioctl.c | 159 +++++++------------------------------- block/partition-generic.c | 124 ++++++++++++++++++++++++++++- include/linux/genhd.h | 10 +-- 3 files changed, 154 insertions(+), 139 deletions(-) diff --git a/block/ioctl.c b/block/ioctl.c index 911887eefc29..5ae04e2c6f0b 100644 --- a/block/ioctl.c +++ b/block/ioctl.c @@ -14,14 +14,9 @@ static int blkpg_ioctl(struct block_device *bdev, struct blkpg_ioctl_arg __user *arg) { - struct block_device *bdevp; - struct gendisk *disk; - struct hd_struct *part, *lpart; struct blkpg_ioctl_arg a; struct blkpg_partition p; - struct disk_part_iter piter; long long start, length; - int partno; if (!capable(CAP_SYS_ADMIN)) return -EACCES; @@ -29,139 +24,37 @@ static int blkpg_ioctl(struct block_device *bdev, struct blkpg_ioctl_arg __user return -EFAULT; if (copy_from_user(&p, a.data, sizeof(struct blkpg_partition))) return -EFAULT; - disk = bdev->bd_disk; if (bdev != bdev->bd_contains) return -EINVAL; - partno = p.pno; - if (partno <= 0) + + if (p.pno <= 0) return -EINVAL; + + if (a.op == BLKPG_DEL_PARTITION) + return bdev_del_partition(bdev, p.pno); + + start = p.start >> SECTOR_SHIFT; + length = p.length >> SECTOR_SHIFT; + + /* check for fit in a hd_struct */ + if (sizeof(sector_t) < sizeof(long long)) { + long pstart = start, plength = length; + + if (pstart != start || plength != length || pstart < 0 || + plength < 0 || p.pno > 65535) + return -EINVAL; + } + switch (a.op) { - case BLKPG_ADD_PARTITION: - start = p.start >> 9; - length = p.length >> 9; - /* check for fit in a hd_struct */ - if (sizeof(sector_t) == sizeof(long) && - sizeof(long long) > sizeof(long)) { - long pstart = start, plength = length; - if (pstart != start || plength != length - || pstart < 0 || plength < 0 || partno > 65535) - return -EINVAL; - } - /* check if partition is aligned to blocksize */ - if (p.start & (bdev_logical_block_size(bdev) - 1)) - return -EINVAL; - - mutex_lock(&bdev->bd_mutex); - - /* overlap? */ - disk_part_iter_init(&piter, disk, - DISK_PITER_INCL_EMPTY); - while ((part = disk_part_iter_next(&piter))) { - if (!(start + length <= part->start_sect || - start >= part->start_sect + part->nr_sects)) { - disk_part_iter_exit(&piter); - mutex_unlock(&bdev->bd_mutex); - return -EBUSY; - } - } - 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: - part = disk_get_part(disk, partno); - if (!part) - return -ENXIO; - - bdevp = bdget(part_devt(part)); - disk_put_part(part); - if (!bdevp) - return -ENOMEM; - - mutex_lock(&bdevp->bd_mutex); - if (bdevp->bd_openers) { - mutex_unlock(&bdevp->bd_mutex); - bdput(bdevp); - return -EBUSY; - } - /* all seems OK */ - fsync_bdev(bdevp); - 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); - - return 0; - case BLKPG_RESIZE_PARTITION: - start = p.start >> 9; - /* new length of partition in bytes */ - length = p.length >> 9; - /* check for fit in a hd_struct */ - if (sizeof(sector_t) == sizeof(long) && - sizeof(long long) > sizeof(long)) { - long pstart = start, plength = length; - if (pstart != start || plength != length - || pstart < 0 || plength < 0) - return -EINVAL; - } - part = disk_get_part(disk, partno); - if (!part) - return -ENXIO; - bdevp = bdget(part_devt(part)); - if (!bdevp) { - disk_put_part(part); - return -ENOMEM; - } - mutex_lock(&bdevp->bd_mutex); - mutex_lock_nested(&bdev->bd_mutex, 1); - if (start != part->start_sect) { - mutex_unlock(&bdevp->bd_mutex); - mutex_unlock(&bdev->bd_mutex); - bdput(bdevp); - disk_put_part(part); - return -EINVAL; - } - /* overlap? */ - disk_part_iter_init(&piter, disk, - DISK_PITER_INCL_EMPTY); - while ((lpart = disk_part_iter_next(&piter))) { - if (lpart->partno != partno && - !(start + length <= lpart->start_sect || - start >= lpart->start_sect + lpart->nr_sects) - ) { - disk_part_iter_exit(&piter); - mutex_unlock(&bdevp->bd_mutex); - mutex_unlock(&bdev->bd_mutex); - bdput(bdevp); - disk_put_part(part); - return -EBUSY; - } - } - disk_part_iter_exit(&piter); - part_nr_sects_write(part, (sector_t)length); - i_size_write(bdevp->bd_inode, p.length); - mutex_unlock(&bdevp->bd_mutex); - mutex_unlock(&bdev->bd_mutex); - bdput(bdevp); - disk_put_part(part); - return 0; - default: + case BLKPG_ADD_PARTITION: + /* check if partition is aligned to blocksize */ + if (p.start & (bdev_logical_block_size(bdev) - 1)) return -EINVAL; + return bdev_add_partition(bdev, p.pno, start, length); + case BLKPG_RESIZE_PARTITION: + return bdev_resize_partition(bdev, p.pno, start, length); + default: + return -EINVAL; } } diff --git a/block/partition-generic.c b/block/partition-generic.c index 2261566741f4..b1277629856c 100644 --- a/block/partition-generic.c +++ b/block/partition-generic.c @@ -321,7 +321,7 @@ static DEVICE_ATTR(whole_disk, 0444, whole_disk_show, NULL); * Must be called either with bd_mutex held, before a disk can be opened or * after all disk users are gone. */ -struct hd_struct *add_partition(struct gendisk *disk, int partno, +static struct hd_struct *add_partition(struct gendisk *disk, int partno, sector_t start, sector_t len, int flags, struct partition_meta_info *info) { @@ -442,6 +442,128 @@ struct hd_struct *add_partition(struct gendisk *disk, int partno, return ERR_PTR(err); } +static bool partition_overlaps(struct gendisk *disk, sector_t start, + sector_t length, int skip_partno) +{ + struct disk_part_iter piter; + struct hd_struct *part; + bool overlap = false; + + disk_part_iter_init(&piter, disk, DISK_PITER_INCL_EMPTY); + while ((part = disk_part_iter_next(&piter))) { + if (part->partno == skip_partno || + start >= part->start_sect + part->nr_sects || + start + length <= part->start_sect) + continue; + overlap = true; + break; + } + + disk_part_iter_exit(&piter); + return overlap; +} + +int bdev_add_partition(struct block_device *bdev, int partno, + sector_t start, sector_t length) +{ + struct hd_struct *part; + + mutex_lock(&bdev->bd_mutex); + if (partition_overlaps(bdev->bd_disk, start, length, -1)) { + mutex_unlock(&bdev->bd_mutex); + return -EBUSY; + } + + down_read(&bdev->bd_disk->lookup_sem); + if (!(bdev->bd_disk->flags & GENHD_FL_UP)) { + up_read(&bdev->bd_disk->lookup_sem); + mutex_unlock(&bdev->bd_mutex); + return -ENXIO; + } + part = add_partition(bdev->bd_disk, partno, start, length, + ADDPART_FLAG_NONE, NULL); + up_read(&bdev->bd_disk->lookup_sem); + mutex_unlock(&bdev->bd_mutex); + return PTR_ERR_OR_ZERO(part); +} + +int bdev_del_partition(struct block_device *bdev, int partno) +{ + struct block_device *bdevp; + struct hd_struct *part; + int ret = 0; + + part = disk_get_part(bdev->bd_disk, partno); + if (!part) + return -ENXIO; + + bdevp = bdget(part_devt(part)); + disk_put_part(part); + if (!bdevp) + return -ENOMEM; + + mutex_lock(&bdevp->bd_mutex); + + ret = -EBUSY; + if (bdevp->bd_openers) + goto out_unlock; + + fsync_bdev(bdevp); + invalidate_bdev(bdevp); + + mutex_lock_nested(&bdev->bd_mutex, 1); + down_read(&bdev->bd_disk->lookup_sem); + delete_partition(bdev->bd_disk, partno); + up_read(&bdev->bd_disk->lookup_sem); + mutex_unlock(&bdev->bd_mutex); + + ret = 0; +out_unlock: + mutex_unlock(&bdevp->bd_mutex); + bdput(bdevp); + return ret; +} + +int bdev_resize_partition(struct block_device *bdev, int partno, + sector_t start, sector_t length) +{ + struct block_device *bdevp; + struct hd_struct *part; + int ret = 0; + + part = disk_get_part(bdev->bd_disk, partno); + if (!part) + return -ENXIO; + + ret = -ENOMEM; + bdevp = bdget(part_devt(part)); + if (!bdevp) + goto out_put_part; + + mutex_lock(&bdevp->bd_mutex); + mutex_lock_nested(&bdev->bd_mutex, 1); + + ret = -EINVAL; + if (start != part->start_sect) + goto out_unlock; + + ret = -EBUSY; + if (partition_overlaps(bdev->bd_disk, start, length, partno)) + goto out_unlock; + + part_nr_sects_write(part, (sector_t)length); + i_size_write(bdevp->bd_inode, length << SECTOR_SHIFT); + + ret = 0; +out_unlock: + mutex_unlock(&bdevp->bd_mutex); + mutex_unlock(&bdev->bd_mutex); + bdput(bdevp); +out_put_part: + disk_put_part(part); + return ret; +} + static bool disk_unlock_native_capacity(struct gendisk *disk) { const struct block_device_operations *bdops = disk->fops; diff --git a/include/linux/genhd.h b/include/linux/genhd.h index 58a819484fb4..3073613538a0 100644 --- a/include/linux/genhd.h +++ b/include/linux/genhd.h @@ -632,13 +632,13 @@ extern char *disk_name (struct gendisk *hd, int partno, char *buf); extern int disk_expand_part_tbl(struct gendisk *disk, int target); extern int rescan_partitions(struct gendisk *disk, struct block_device *bdev); extern int invalidate_partitions(struct gendisk *disk, struct block_device *bdev); -extern struct hd_struct * __must_check add_partition(struct gendisk *disk, - int partno, sector_t start, - sector_t len, int flags, - struct partition_meta_info - *info); extern void __delete_partition(struct percpu_ref *); extern void delete_partition(struct gendisk *, int); +int bdev_add_partition(struct block_device *bdev, int partno, + sector_t start, sector_t length); +int bdev_del_partition(struct block_device *bdev, int partno); +int bdev_resize_partition(struct block_device *bdev, int partno, + sector_t start, sector_t length); extern void printk_all_partitions(void); extern struct gendisk *__alloc_disk_node(int minors, int node_id); -- 2.25.1
From: Zhong Jinghua <zhongjinghua@huawei.com> hulk inclusion category: bugfix bugzilla: 187268, https://gitee.com/openeuler/kernel/issues/I76JDY CVE: NA ---------------------------------------- In the block_ioctl, we can pass in the unsigned number 0x8000000000000000 as an input parameter, like below: block_ioctl blkdev_ioctl blkpg_ioctl blkpg_do_ioctl copy_from_user bdev_add_partition add_partition p->start_sect = start; // start = 0x8000000000000000 Then, there was an warning when submit bio: WARNING: CPU: 0 PID: 382 at fs/iomap/apply.c:54 Call trace: iomap_apply+0x644/0x6e0 __iomap_dio_rw+0x5cc/0xa24 iomap_dio_rw+0x4c/0xcc ext4_dio_read_iter ext4_file_read_iter ext4_file_read_iter+0x318/0x39c call_read_iter lo_rw_aio.isra.0+0x748/0x75c do_req_filebacked+0x2d4/0x370 loop_handle_cmd loop_queue_work+0x94/0x23c kthread_worker_fn+0x160/0x6bc loop_kthread_worker_fn+0x3c/0x50 kthread+0x20c/0x25c ret_from_fork+0x10/0x18 Stack: submit_bio_noacct submit_bio_checks blk_partition_remap bio->bi_iter.bi_sector += p->start_sect // bio->bi_iter.bi_sector = 0xffc0000000000000 + 65408 .. loop_queue_work loop_handle_cmd do_req_filebacked pos = ((loff_t) blk_rq_pos(rq) << 9) + lo->lo_offset // pos < 0 lo_rw_aio call_read_iter ext4_dio_read_iter __iomap_dio_rw iomap_apply ext4_iomap_begin map.m_lblk = offset >> blkbits ext4_set_iomap iomap->offset = (u64) map->m_lblk << blkbits // iomap->offset = 64512 WARN_ON(iomap.offset > pos) // iomap.offset = 64512 and pos < 0 This is unreasonable for start + length > disk->part0.nr_sects. There is already a similar check in blk_add_partition(). Fix it by adding a check in bdev_add_partition(). Signed-off-by: Zhong Jinghua <zhongjinghua@huawei.com> Reviewed-by: Hou Tao <houtao1@huawei.com> Signed-off-by: Yongqiang Liu <liuyongqiang13@huawei.com> --- block/ioctl.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/block/ioctl.c b/block/ioctl.c index 5ae04e2c6f0b..e5da2482342a 100644 --- a/block/ioctl.c +++ b/block/ioctl.c @@ -33,9 +33,16 @@ static int blkpg_ioctl(struct block_device *bdev, struct blkpg_ioctl_arg __user if (a.op == BLKPG_DEL_PARTITION) return bdev_del_partition(bdev, p.pno); + if (p.start < 0 || p.length <= 0 || p.start + p.length < 0) + return -EINVAL; + start = p.start >> SECTOR_SHIFT; length = p.length >> SECTOR_SHIFT; + /* length may be equal to 0 after right shift */ + if (!length || start + length > get_capacity(bdev->bd_disk)) + return -EINVAL; + /* check for fit in a hd_struct */ if (sizeof(sector_t) < sizeof(long long)) { long pstart = start, plength = length; -- 2.25.1
participants (1)
-
Yongqiang Liu