On Tue, 2019-04-02 at 20:06 +-0800, Yufen Yu wrote: +AD4 commit 2da78092dda +ACI-block: Fix dev+AF8-t minor allocation lifetime+ACI +AD4 specifically moved blk+AF8-free+AF8-devt(dev-+AD4-devt) call to part+AF8-release() +AD4 to avoid reallocating device number before the device is fully +AD4 shutdown. +AD4 +AD4 However, it can cause use-after-free on gendisk in get+AF8-gendisk(). +AD4 We use md device as example to show the race scenes: +AD4 +AD4 Process1 Worker Process2 +AD4 md+AF8-free +AD4 blkdev+AF8-open +AD4 del+AF8-gendisk +AD4 add delete+AF8-partition+AF8-work+AF8-fn() to wq +AD4 +AF8AXw-blkdev+AF8-get +AD4 get+AF8-gendisk +AD4 put+AF8-disk +AD4 disk+AF8-release +AD4 kfree(disk) +AD4 find part from ext+AF8-devt+AF8-idr +AD4 get+AF8-disk+AF8-and+AF8-module(disk) +AD4 cause use after free +AD4 +AD4 delete+AF8-partition+AF8-work+AF8-fn +AD4 put+AF8-device(part) +AD4 part+AF8-release +AD4 remove part from ext+AF8-devt+AF8-idr +AD4 +AD4 Before +ADw-devt, hd+AF8-struct pointer+AD4 is removed from ext+AF8-devt+AF8-idr by +AD4 delete+AF8-partition+AF8-work+AF8-fn(), we can find the devt and then access +AD4 gendisk by hd+AF8-struct pointer. But, if we access the gendisk after +AD4 it have been freed, it can cause in use-after-freeon gendisk in +AD4 get+AF8-gendisk(). +AD4 +AD4 We fix this by adding a new helper blk+AF8-invalidate+AF8-devt() in +AD4 delete+AF8-partition() and del+AF8-gendisk(). It replaces hd+AF8-struct +AD4 pointer in idr with value 'NULL', and deletes the entry from +AD4 idr in part+AF8-release() as we do now. +AD4 +AD4 Thanks to Jan Kara for providing the solution and more clear comments +AD4 for the code. Nice work. Reviewed-by: Bart Van Assche +ADw-bvanassche+AEA-acm.org+AD4
