block: fix use-after-free on gendisk

BugLink: https://bugs.launchpad.net/bugs/1838700
[ 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>
Signed-off-by: Kamal Mostafa <kamal@canonical.com>
Signed-off-by: Khalid Elmously <khalid.elmously@canonical.com>

diff --git a/block/genhd.c b/block/genhd.c
index f878da09d658043c6ec5acdef560c68e7731604f..76fa1e96ebd3bb0963970d980566f115423a2960 100644 (file)
--- a/block/genhd.c
+++ b/block/genhd.c
@@ -515,6 +515,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) {
@@ -745,6 +757,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 cc5ea110bf0767ddd66430441c5452acac42998f..afde86374081ef9d4b574a55fd6a1c4d43c74b52 100644 (file)
--- 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 dc0b34ac4b380f52a558425b93657b8190fc033f..cbdf003e759e0ee3106a9cf0f05b0cf326883be7 100644 (file)
--- a/include/linux/genhd.h
+++ b/include/linux/genhd.h
@@ -581,6 +581,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);