Re: [PATCH 11/14] loop: implement ->free_disk

[Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>]

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...