[ 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);