Re: [syzbot] [f2fs?] WARNING in rcu_sync_dtor

[Christian Brauner]

On Fri, Jul 26, 2024 at 08:23:02AM GMT, syzbot wrote:
> syzbot has bisected this issue to:
> 
> commit b62e71be2110d8b52bf5faf3c3ed7ca1a0c113a5
> Author: Chao Yu <chao@xxxxxxxxxx>
> Date:   Sun Apr 23 15:49:15 2023 +0000
> 
>     f2fs: support errors=remount-ro|continue|panic mountoption
> 
> bisection log:  https://syzkaller.appspot.com/x/bisect.txt?x=119745f1980000
> start commit:   1722389b0d86 Merge tag 'net-6.11-rc1' of git://git.kernel...
> git tree:       upstream
> final oops:     https://syzkaller.appspot.com/x/report.txt?x=139745f1980000
> console output: https://syzkaller.appspot.com/x/log.txt?x=159745f1980000
> kernel config:  https://syzkaller.appspot.com/x/.config?x=b698a1b2fcd7ef5f
> dashboard link: https://syzkaller.appspot.com/bug?extid=20d7e439f76bbbd863a7
> syz repro:      https://syzkaller.appspot.com/x/repro.syz?x=1237a1f1980000
> C reproducer:   https://syzkaller.appspot.com/x/repro.c?x=115edac9980000
> 
> Reported-by: syzbot+20d7e439f76bbbd863a7@xxxxxxxxxxxxxxxxxxxxxxxxx
> Fixes: b62e71be2110 ("f2fs: support errors=remount-ro|continue|panic mountoption")
> 
> For information about bisection process see: https://goo.gl/tpsmEJ#bisection

Thanks to Paul and Oleg for point me in the right direction and
explaining that rcu sync warning.

That patch here is remounting a superblock read-only directly by raising
SB_RDONLY without the involvement of the VFS at all. That's pretty
broken and is likely to cause trouble if done wrong. The rough order of
operations to transition rw->ro usualy include checking that the
filsystem is unfrozen, and marking all mounts read-only, then calling
into the filesystem so it can do whatever it wants to do.

In any case, all of this requires holding sb->s_umount. Not holding
sb->s_umount will end up confusing freeze_super() (Thanks to Oleg for
noticing!). When freeze_super() is called on a non-ro filesystem it will
acquire
percpu_down_write(SB_FREEZE_WRITE+SB_FREEZE_PAGEFAULT+SB_FREEZE_FS) and
thaw_super() needs to call
sb_freeze_unlock(SB_FREEZE_FS+SB_FREEZE_PAGEFAULT+SB_FREEZE_WRITE) but
because you just raise SB_RDONLY you end up causing thaw_super() to skip
that step causing the bug in rcu_sync_dtor() to be noticed.

Btw, ext4 has similar logic where it raises SB_RDONLY without checking
whether the filesystem is frozen.

So I guess, this is technically ok as long as that emergency SB_RDONLY raising
in sb->s_flags is not done while the fs is already frozen. I think ext4 can
probably never do that. Jan?

My guess is that something in f2fs can end up raising SB_RDONLY after
the filesystem is frozen and so it causes this bug. I suspect this is coming
from the gc_thread() which might issue a f2fs_stop_checkpoint() while the fs is
already about to be frozen but before the gc thread is stopped as part of the
freeze.