I hoped that below patch fixes the problem, but it did not ( https://syzkaller.appspot.com/text?tag=CrashReport&x=111bc8f7d00000 ). I guess we need to somehow serialize loop_add() and loop_remove() if we can't use loop_ctl_mutex... >From 2dfbd8e4a10c7883a85a2d4c32d83c93b2c3d485 Mon Sep 17 00:00:00 2001 From: Tetsuo Handa <penguin-kernel@xxxxxxxxxxxxxxxxxxx> Date: Fri, 11 Jun 2021 05:54:25 +0000 Subject: [not-yet-a-PATCH] loop: drop loop_ctl_mutex before calling del_gendisk() syzbot is reporting circular locking dependency between loop_ctl_mutex and bdev->bd_mutex [1] due to commit c76f48eb5c084b1e ("block: take bd_mutex around delete_partitions in del_gendisk"). Since the lo becomes unreachable before del_gendisk() is called by loop_remove(lo) from loop_control_ioctl(LOOP_CTL_REMOVE), we can drop loop_ctl_mutex before calling loop_remove(). Holding loop_ctl_mutex in loop_exit() is pointless, for module's refcount being 0 (unless forced module unload is requested) guarantees that nobody is using /dev/loop* interface. Link: https://syzkaller.appspot.com/bug?extid=61e04e51b7ac86930589 [1] Reported-by: syzbot <syzbot+61e04e51b7ac86930589@xxxxxxxxxxxxxxxxxxxxxxxxx> Fixes: c76f48eb5c084b1e ("block: take bd_mutex around delete_partitions in del_gendisk") --- drivers/block/loop.c | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/drivers/block/loop.c b/drivers/block/loop.c index d58d68f3c7cd..3045bcb9f7ed 100644 --- a/drivers/block/loop.c +++ b/drivers/block/loop.c @@ -2079,6 +2079,7 @@ static const struct blk_mq_ops loop_mq_ops = { .complete = lo_complete_rq, }; +/* Must be called with loop_ctl_mutex held. */ static int loop_add(struct loop_device **l, int i) { struct loop_device *lo; @@ -2186,6 +2187,7 @@ static int loop_add(struct loop_device **l, int i) return err; } +/* Must not be called with loop_ctl_mutex or lo->lo_mutex held. */ static void loop_remove(struct loop_device *lo) { del_gendisk(lo->lo_disk); @@ -2288,8 +2290,9 @@ static long loop_control_ioctl(struct file *file, unsigned int cmd, lo->lo_disk->private_data = NULL; mutex_unlock(&lo->lo_mutex); idr_remove(&loop_index_idr, lo->lo_number); + mutex_unlock(&loop_ctl_mutex); loop_remove(lo); - break; + return 0; case LOOP_CTL_GET_FREE: ret = loop_lookup(&lo, -1); if (ret >= 0) @@ -2397,16 +2400,12 @@ static int loop_exit_cb(int id, void *ptr, void *data) static void __exit loop_exit(void) { - mutex_lock(&loop_ctl_mutex); - idr_for_each(&loop_index_idr, &loop_exit_cb, NULL); idr_destroy(&loop_index_idr); unregister_blkdev(LOOP_MAJOR, "loop"); misc_deregister(&loop_misc); - - mutex_unlock(&loop_ctl_mutex); } module_init(loop_init); -- 2.25.1
