On Sat, Apr 7, 2018 at 9:27 AM, Tetsuo Handa <penguin-ker...@i-love.sakura.ne.jp> wrote: > Omar Sandoval wrote: >> From: Omar Sandoval <osan...@fb.com> >> >> Commit 2d1d4c1e591f made loop_get_status() drop lo_ctx_mutex before >> returning, but the loop_get_status_old(), loop_get_status64(), and >> loop_get_status_compat() wrappers don't call loop_get_status() if the >> passed argument is NULL. The callers expect that the lock is dropped, so >> make sure we drop it in that case, too. >> >> Reported-by: syzbot+31e8daa8b3fc129e7...@syzkaller.appspotmail.com > > Well, it is me who reported this bug before syzbot reports it. ;-)
Robots are taking our jobs! :) We could notify syzbot with just "#syz fix" tag in the report email, rather than putting it into Reported-by. >> Fixes: 2d1d4c1e591f ("loop: don't call into filesystem while holding >> lo_ctl_mutex") > > But I feel we should revert 2d1d4c1e591f rather than applying this patch. > >> Signed-off-by: Omar Sandoval <osan...@fb.com> > > If the reason of dropping the lock is to avoid deadlock caused by recursing > into the same lock from vfs_getattr(), it is correct thing to drop the lock. > > But when the reason is that vfs_getattr() cannot return when NFS server is > dead, there is no point with dropping the lock. Anybody who tries to call > loop_get_status() will get stuck. It is commit 3148ffbdb9162baa ("loop: > use killable lock in ioctls") which actually helps minimizing number of > stuck processes when NFS server is dead if we didn't drop the lock. > If we drop the lock before calling vfs_getattr(), all threads who called > loop_get_status() will reach vfs_getattr() and get stuck, won't it?