On Thu, Nov 11, 2021 at 07:10:56AM +0900, Tetsuo Handa wrote:
> Hello.
>
> Commit 87579e9b7d8dc36e ("loop: use worker per cgroup instead of kworker")
> is calling destroy_workqueue() with lo->lo_mutex held, and syzbot is
> reporting circular locking dependency
The commit changed the logic from a separate kthread + queue to a
workqueue. So I don't believe this changed anything regarding this
circular locking dependency, it's just that the dependency is now
clear via workqueue whereas it wasn't with the kthread.
>
> &disk->open_mutex => &lo->lo_mutex => (wq_completion)loop$M =>
> (work_completion)(&lo->rootcg_work) => sb_writers#$N => &p->lock =>
> &of->mutex => system_transition_mutex/1 => &disk->open_mutex
>
> . Can you somehow call destroy_workqueue() without holding a lock (e.g.
> breaking &lo->lo_mutex => (wq_completion)loop$M dependency chain by
> making sure that below change is safe) ?
It's really not clear to me - the lo_mutex protects a lot of entry
points (ioctls and others) and it's hard to tell if the observed state
will be sane if they can race in the middle of __loop_clr_fd.
>
> @@ -1365,7 +1365,9 @@ static int __loop_clr_fd(struct loop_device *lo, bool release)
> /* freeze request queue during the transition */
> blk_mq_freeze_queue(lo->lo_queue);
>
> + mutex_unlock(&lo->lo_mutex);
> destroy_workqueue(lo->workqueue);
> + mutex_lock(&lo->lo_mutex);
> spin_lock_irq(&lo->lo_work_lock);
> list_for_each_entry_safe(worker, pos, &lo->idle_worker_list,
> idle_list) {
>
> On 2021/11/11 2:00, syzbot wrote:
> > Hello,
> >
> > syzbot found the following issue on:
> >
> > HEAD commit: cb690f5238d7 Merge tag 'for-5.16/drivers-2021-11-09' of gi..
> > git tree: upstream
> > console output: https://syzkaller.appspot.com/x/log.txt?x=1611368ab00000
> > kernel config: https://syzkaller.appspot.com/x/.config?x=9d7259f0deb293aa
> > dashboard link: https://syzkaller.appspot.com/bug?extid=63614029dfb79abd4383
> > compiler: gcc (Debian 10.2.1-6) 10.2.1 20210110, GNU ld (GNU Binutils for Debian) 2.35.2
>
