On 2022/03/25 19:42, Tetsuo Handa wrote: > On 2022/03/25 15:39, Christoph Hellwig wrote: >> Ensure that the lo_device which is stored in the gendisk private >> data is valid until the gendisk is freed. Currently the loop driver >> uses a lot of effort to make sure a device is not freed when it is >> still in use, but to to fix a potential deadlock this will be relaxed >> a bit soon. > > This patch breaks blk_cleanup_disk() into blk_cleanup_queue() and put_disk() on > loop_remove() side only. But there is blk_cleanup_disk() in the error path of > loop_add() side. Don't we need to rewrite the error path of loop_add() side, for > put_disk() from blk_cleanup_disk() from loop_add() calls kfree() via lo_free_disk() > but out_cleanup_disk: label falls through to blk_mq_free_tag_set() (which seems to > be UAF read) and kfree() (which seems to be double kfree()) ? > Ah, since set_bit(GD_ADDED, &disk->state) is not called unless device_add_disk() from add_disk() succeeds, disk->fops->free_disk will not be called unless add_disk() succeeds. Thus, it is OK for the error path of loop_add(). Tricky call...
