Re: [f2fs-dev] [PATCH] f2fs: Do not check the FI_DIRTY_INODE flag when umounting a ro fs.

[Julian Sun <sunjunchao2870@gmail.com>]

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@syzkaller.appspotmail.com
> > Closes:
> > https://syzkaller.appspot.com/bug?extid=ebea2790904673d7c618
> > Fixes: ca7d802a7d8e ("f2fs: detect dirty inode in evict_inode")
> > CC: stable@vger.kernel.org
> > Signed-off-by: Julian Sun <sunjunchao2870@gmail.com>
> > ---
> >  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



_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel