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

[Dave Chinner <david@fromorbit.com>]

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@syzkaller.appspotmail.com
> 
> ======================================================
> 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....

-Dave.
-- 
Dave Chinner
david@fromorbit.com