| From 20e57e22ae4d7506f30778d0a8e1b33da4d1110b Mon Sep 17 00:00:00 2001 |
| From: Yufen Yu <yuyufen@huawei.com> |
| Date: Tue, 2 Apr 2019 20:06:34 +0800 |
| Subject: block: fix use-after-free on gendisk |
| |
| [ Upstream commit 2c88e3c7ec32d7a40cc7c9b4a487cf90e4671bdd ] |
| |
| commit 2da78092dda "block: Fix dev_t minor allocation lifetime" |
| specifically moved blk_free_devt(dev->devt) call to part_release() |
| to avoid reallocating device number before the device is fully |
| shutdown. |
| |
| However, it can cause use-after-free on gendisk in get_gendisk(). |
| We use md device as example to show the race scenes: |
| |
| Process1 Worker Process2 |
| md_free |
| blkdev_open |
| del_gendisk |
| add delete_partition_work_fn() to wq |
| __blkdev_get |
| get_gendisk |
| put_disk |
| disk_release |
| kfree(disk) |
| find part from ext_devt_idr |
| get_disk_and_module(disk) |
| cause use after free |
| |
| delete_partition_work_fn |
| put_device(part) |
| part_release |
| remove part from ext_devt_idr |
| |
| Before <devt, hd_struct pointer> is removed from ext_devt_idr by |
| delete_partition_work_fn(), we can find the devt and then access |
| gendisk by hd_struct pointer. But, if we access the gendisk after |
| it have been freed, it can cause in use-after-freeon gendisk in |
| get_gendisk(). |
| |
| We fix this by adding a new helper blk_invalidate_devt() in |
| delete_partition() and del_gendisk(). It replaces hd_struct |
| pointer in idr with value 'NULL', and deletes the entry from |
| idr in part_release() as we do now. |
| |
| Thanks to Jan Kara for providing the solution and more clear comments |
| for the code. |
| |
| Fixes: 2da78092dda1 ("block: Fix dev_t minor allocation lifetime") |
| Cc: Al Viro <viro@zeniv.linux.org.uk> |
| Reviewed-by: Bart Van Assche <bvanassche@acm.org> |
| Reviewed-by: Keith Busch <keith.busch@intel.com> |
| Reviewed-by: Jan Kara <jack@suse.cz> |
| Suggested-by: Jan Kara <jack@suse.cz> |
| Signed-off-by: Yufen Yu <yuyufen@huawei.com> |
| Signed-off-by: Jens Axboe <axboe@kernel.dk> |
| Signed-off-by: Sasha Levin <sashal@kernel.org> |
| --- |
| block/genhd.c | 19 +++++++++++++++++++ |
| block/partition-generic.c | 7 +++++++ |
| include/linux/genhd.h | 1 + |
| 3 files changed, 27 insertions(+) |
| |
| diff --git a/block/genhd.c b/block/genhd.c |
| index 703267865f14d..d8dff0b21f7d1 100644 |
| --- a/block/genhd.c |
| +++ b/block/genhd.c |
| @@ -531,6 +531,18 @@ void blk_free_devt(dev_t devt) |
| } |
| } |
| |
| +/** |
| + * We invalidate devt by assigning NULL pointer for devt in idr. |
| + */ |
| +void blk_invalidate_devt(dev_t devt) |
| +{ |
| + if (MAJOR(devt) == BLOCK_EXT_MAJOR) { |
| + spin_lock_bh(&ext_devt_lock); |
| + idr_replace(&ext_devt_idr, NULL, blk_mangle_minor(MINOR(devt))); |
| + spin_unlock_bh(&ext_devt_lock); |
| + } |
| +} |
| + |
| static char *bdevt_str(dev_t devt, char *buf) |
| { |
| if (MAJOR(devt) <= 0xff && MINOR(devt) <= 0xff) { |
| @@ -793,6 +805,13 @@ void del_gendisk(struct gendisk *disk) |
| |
| if (!(disk->flags & GENHD_FL_HIDDEN)) |
| blk_unregister_region(disk_devt(disk), disk->minors); |
| + /* |
| + * Remove gendisk pointer from idr so that it cannot be looked up |
| + * while RCU period before freeing gendisk is running to prevent |
| + * use-after-free issues. Note that the device number stays |
| + * "in-use" until we really free the gendisk. |
| + */ |
| + blk_invalidate_devt(disk_devt(disk)); |
| |
| kobject_put(disk->part0.holder_dir); |
| kobject_put(disk->slave_dir); |
| diff --git a/block/partition-generic.c b/block/partition-generic.c |
| index 8e596a8dff321..aee643ce13d15 100644 |
| --- a/block/partition-generic.c |
| +++ b/block/partition-generic.c |
| @@ -285,6 +285,13 @@ void delete_partition(struct gendisk *disk, int partno) |
| kobject_put(part->holder_dir); |
| device_del(part_to_dev(part)); |
| |
| + /* |
| + * Remove gendisk pointer from idr so that it cannot be looked up |
| + * while RCU period before freeing gendisk is running to prevent |
| + * use-after-free issues. Note that the device number stays |
| + * "in-use" until we really free the gendisk. |
| + */ |
| + blk_invalidate_devt(part_devt(part)); |
| hd_struct_kill(part); |
| } |
| |
| diff --git a/include/linux/genhd.h b/include/linux/genhd.h |
| index 06c0fd594097d..69db1affedb0b 100644 |
| --- a/include/linux/genhd.h |
| +++ b/include/linux/genhd.h |
| @@ -610,6 +610,7 @@ struct unixware_disklabel { |
| |
| extern int blk_alloc_devt(struct hd_struct *part, dev_t *devt); |
| extern void blk_free_devt(dev_t devt); |
| +extern void blk_invalidate_devt(dev_t devt); |
| extern dev_t blk_lookup_devt(const char *name, int partno); |
| extern char *disk_name (struct gendisk *hd, int partno, char *buf); |
| |
| -- |
| 2.20.1 |
| |