Re: [syzbot] [exfat?] possible deadlock in exfat_page_mkwrite

[Namjae Jeon]

2024년 4월 17일 (수) 오전 8:02, Dave Chinner <david@xxxxxxxxxxxxx>님이 작성:
>
> On Tue, Apr 16, 2024 at 06:14:20AM -0700, syzbot wrote:
> > Hello,
> >
> > syzbot found the following issue on:
> >
> > HEAD commit:    66e4190e92ce Add linux-next specific files for 20240416
> > git tree:       linux-next
> > console output: https://syzkaller.appspot.com/x/log.txt?x=15817767180000
> > kernel config:  https://syzkaller.appspot.com/x/.config?x=c247afaa437e6409
> > dashboard link: https://syzkaller.appspot.com/bug?extid=d88216a7af9446d57d59
> > compiler:       Debian clang version 15.0.6, GNU ld (GNU Binutils for Debian) 2.40
> >
> > Unfortunately, I don't have any reproducer for this issue yet.
> >
> > Downloadable assets:
> > disk image: https://storage.googleapis.com/syzbot-assets/86891dae5f9c/disk-66e4190e.raw.xz
> > vmlinux: https://storage.googleapis.com/syzbot-assets/1ca383660bf2/vmlinux-66e4190e.xz
> > kernel image: https://storage.googleapis.com/syzbot-assets/bf6ff37d3fcc/bzImage-66e4190e.xz
> >
> > IMPORTANT: if you fix the issue, please add the following tag to the commit:
> > Reported-by: syzbot+d88216a7af9446d57d59@xxxxxxxxxxxxxxxxxxxxxxxxx
> >
> > ======================================================
> > WARNING: possible circular locking dependency detected
> > 6.9.0-rc4-next-20240416-syzkaller #0 Not tainted
> > ------------------------------------------------------
> > syz-executor.0/17125 is trying to acquire lock:
> > ffff88805e616b38 (&sb->s_type->i_mutex_key#24){++++}-{3:3}, at: inode_lock include/linux/fs.h:791 [inline]
> > ffff88805e616b38 (&sb->s_type->i_mutex_key#24){++++}-{3:3}, at: exfat_page_mkwrite+0x43a/0xea0 fs/exfat/file.c:629
>
> exfat_page_mkwrite() is taking the inode_lock() in the page fault
> handler:
>
>         folio_lock(folio);
>         .....
>         if (ei->valid_size < folio_pos(folio)) {
>                 inode_lock(inode);
>                 err = exfat_extend_valid_size(file, ei->valid_size, folio_pos(folio));
>                 inode_unlock(inode);
>                 if (err < 0) {
>                         ret = vmf_fs_error(err);
>                         goto out;
>                 }
>         }
>
> This is can deadlock in a couple of ways:
>
> 1. page faults nest inside the inode lock (e.g. read/write IO path)
> 2. folio locks nest inside the inode lock (e.g. truncate)
> 3. IIUC, exfat_extend_valid_size() will allocate, lock and zero new
> folios and call balance_dirty_pages_ratelimited(). None of these
> things should be done with some other folio already held locked.
>
> As I've previously said: doing sparse file size extension in page
> fault context is complex and difficult to do correctly. It is far
> easier and safer to do it when the file is actually extended, and in
> that case the context doing the extension takes the perf penalty of
> allocaiton and zeroing, not the downstream application doing mmap()
> operations on the (extended) file....
Okay, Let me take a look.
Thanks for your comment.
>
> -Dave.
> --
> Dave Chinner
> david@xxxxxxxxxxxxx