summaryrefslogtreecommitdiff
path: root/block
diff options
context:
space:
mode:
authorYufen Yu <yuyufen@huawei.com>2019-04-02 20:06:34 +0800
committerJens Axboe <axboe@kernel.dk>2019-04-22 09:48:12 -0600
commit6fcc44d1d77fea3c7230e4d109b37f6977aa675a (patch)
tree2fb40867aa291075313719bd9bb5f8874c5bca96 /block
parent5c61ee2cd5860e41c8ab98837761ffaa93eb4dfe (diff)
block: fix use-after-free on gendisk
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>
Diffstat (limited to 'block')
-rw-r--r--block/genhd.c19
-rw-r--r--block/partition-generic.c7
2 files changed, 26 insertions, 0 deletions
diff --git a/block/genhd.c b/block/genhd.c
index 1d0d25f7b0fe..83f5c33d1e80 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 8e596a8dff32..aee643ce13d1 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);
}