On 8/29/21 7:47 AM, Tetsuo Handa wrote:
> syzbot is reporting circular locking problem at __loop_clr_fd() [1], for
> commit a160c6159d4a0cf8 ("block: add an optional probe callback to
> major_names") is calling the module's probe function with major_names_lock
> held.
>
> Fortunately, since commit 990e78116d38059c ("block: loop: fix deadlock
> between open and remove") stopped holding loop_ctl_mutex in lo_open(),
> current role of loop_ctl_mutex is to serialize access to loop_index_idr
> and loop_add()/loop_remove(); in other words, management of id for IDR.
> To avoid holding loop_ctl_mutex during whole add/remove operation, use
> a bool flag to indicate whether the loop device is ready for use.
>
> loop_unregister_transfer() which is called from cleanup_cryptoloop()
> currently has possibility of use-after-free access due to lack of
> serialization between kfree() from loop_remove() from loop_control_remove()
> and mutex_lock() from unregister_transfer_cb(). But since lo->lo_encryption
> should be already NULL when this function is called due to module unload,
> and commit 222013f9ac30b9ce ("cryptoloop: add a deprecation warning")
> indicates that we will remove this function shortly, this patch updates
> this function to emit warning instead of checking lo->lo_encryption.
>
> Holding loop_ctl_mutex in loop_exit() is pointless, for all users must
> close /dev/loop-control and /dev/loop$num (in order to drop module's
> refcount to 0) before loop_exit() starts, and nobody can open
> /dev/loop-control or /dev/loop$num afterwards.
Applied, thanks.
--
Jens Axboe
