On Wed, 2024-08-28 at 15:26 +0000, Jaegeuk Kim wrote: > On 08/27, Julian Sun wrote: > > Hi, all. > > > > Recently syzbot reported a bug as following: > > > > kernel BUG at fs/f2fs/inode.c:896! > > CPU: 1 UID: 0 PID: 5217 Comm: syz-executor605 Not tainted 6.11.0- > > rc4-syzkaller-00033-g872cf28b8df9 #0 > > RIP: 0010:f2fs_evict_inode+0x1598/0x15c0 fs/f2fs/inode.c:896 > > Call Trace: > > <TASK> > > evict+0x532/0x950 fs/inode.c:704 > > dispose_list fs/inode.c:747 [inline] > > evict_inodes+0x5f9/0x690 fs/inode.c:797 > > generic_shutdown_super+0x9d/0x2d0 fs/super.c:627 > > kill_block_super+0x44/0x90 fs/super.c:1696 > > kill_f2fs_super+0x344/0x690 fs/f2fs/super.c:4898 > > deactivate_locked_super+0xc4/0x130 fs/super.c:473 > > cleanup_mnt+0x41f/0x4b0 fs/namespace.c:1373 > > task_work_run+0x24f/0x310 kernel/task_work.c:228 > > ptrace_notify+0x2d2/0x380 kernel/signal.c:2402 > > ptrace_report_syscall include/linux/ptrace.h:415 [inline] > > ptrace_report_syscall_exit include/linux/ptrace.h:477 [inline] > > syscall_exit_work+0xc6/0x190 kernel/entry/common.c:173 > > syscall_exit_to_user_mode_prepare kernel/entry/common.c:200 > > [inline] > > __syscall_exit_to_user_mode_work kernel/entry/common.c:205 > > [inline] > > syscall_exit_to_user_mode+0x279/0x370 kernel/entry/common.c:218 > > do_syscall_64+0x100/0x230 arch/x86/entry/common.c:89 > > entry_SYSCALL_64_after_hwframe+0x77/0x7f > > > > The syzbot constructed the following scenario: concurrently > > creating directories and setting the file system to read-only. > > In this case, while f2fs was making dir, the filesystem switched to > > readonly, and when it tried to clear the dirty flag, it triggered > > this > > code path: f2fs_mkdir()-> f2fs_sync_fs()->f2fs_write_checkpoint() > > ->f2fs_readonly(). This resulted FI_DIRTY_INODE flag not being > > cleared, > > which eventually led to a bug being triggered during the > > FI_DIRTY_INODE > > check in f2fs_evict_inode(). > > > > In this case, we cannot do anything further, so if filesystem is > > readonly, > > do not trigger the BUG. Instead, clean up resources to the best of > > our > > ability to prevent triggering subsequent resource leak checks. > > > > If there is anything important I'm missing, please let me know, > > thanks. > > > > Reported-by: syzbot+ebea2790904673d7c618@xxxxxxxxxxxxxxxxxxxxxxxxx > > Closes: > > https://syzkaller.appspot.com/bug?extid=ebea2790904673d7c618 > > Fixes: ca7d802a7d8e ("f2fs: detect dirty inode in evict_inode") > > CC: stable@xxxxxxxxxxxxxxx > > Signed-off-by: Julian Sun <sunjunchao2870@xxxxxxxxx> > > --- > > fs/f2fs/inode.c | 8 ++++++-- > > 1 file changed, 6 insertions(+), 2 deletions(-) > > > > diff --git a/fs/f2fs/inode.c b/fs/f2fs/inode.c > > index aef57172014f..52d273383ec2 100644 > > --- a/fs/f2fs/inode.c > > +++ b/fs/f2fs/inode.c > > @@ -892,8 +892,12 @@ void f2fs_evict_inode(struct inode *inode) > > atomic_read(&fi->i_compr_blocks)); > > > > if (likely(!f2fs_cp_error(sbi) && > > - !is_sbi_flag_set(sbi, > > SBI_CP_DISABLED))) > > - f2fs_bug_on(sbi, is_inode_flag_set(inode, > > FI_DIRTY_INODE)); > > + !is_sbi_flag_set(sbi, > > SBI_CP_DISABLED))) { > > + if (!f2fs_readonly(sbi->sb)) > > + f2fs_bug_on(sbi, is_inode_flag_set(inode, > > FI_DIRTY_INODE)); > > + else > > + f2fs_inode_synced(inode); > > + } > > else > > f2fs_inode_synced(inode); > > What about: > > if (likely(!f2fs_cp_error(sbi) && > !is_sbi_flag_set(sbi, SBI_CP_DISABLED)) && > !f2fs_readonly(sbi->sb))) > f2fs_bug_on(sbi, is_inode_flag_set(inode, > FI_DIRTY_INODE)); > else > f2fs_inode_synced(inode); Hi, Jaegeuk, thanks for your review. Yeah, it is semantically identical, and the code is clearer. I will fix it in patch v2. > > > > > > > > -- > > 2.39.2
