Re: [PATCH v3] block: fix use-after-free on gendisk

[Keith Busch <kbusch@kernel.org>]

On Tue, Apr 02, 2019 at 05:06:34AM -0700, Yufen Yu wrote:
> 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>
> Cc: Bart Van Assche <bart.vanassche@wdc.com>
> Cc: Keith Busch <keith.busch@intel.com>
> Suggested-by: Jan Kara <jack@suse.cz>
> Signed-off-by: Yufen Yu <yuyufen@huawei.com>

Looks good to me.

Reviewed-by: Keith Busch <keith.busch@intel.com>