Message-ID: <CANp29Y55bJ_3qg+y4jgwuUu7nwvtrfhFEvStzFuoWzS=Xm=3uw@mail.gmail.com>
Date: Tue, 20 May 2025 19:17:17 +0200
From: Aleksandr Nogikh <nogikh@...gle.com>
To: Al Viro <viro@...iv.linux.org.uk>
Cc: Edward Adam Davis <eadavis@...com>, syzbot+321477fad98ea6dd35b7@...kaller.appspotmail.com, 
	brauner@...nel.org, jack@...e.cz, linux-fsdevel@...r.kernel.org, 
	linux-kernel@...r.kernel.org, syzkaller-bugs@...glegroups.com, 
	syzkaller <syzkaller@...glegroups.com>
Subject: Re: [pox on syzbot - again][exfat] exfat_mkdir() breakage on
 corrupted image

Hi Al,

I've only just seen this email as it landed in my Spam folder for some reason.

On Sat, May 17, 2025 at 1:20 AM Al Viro <viro@...iv.linux.org.uk> wrote:
>
> On Fri, May 16, 2025 at 08:31:22PM +0100, Al Viro wrote:
> > On Wed, May 14, 2025 at 06:39:40AM +0800, Edward Adam Davis wrote:
> > > In the reproducer, when calling renameat2(), olddirfd and newdirfd passed
> > > are the same value r0, see [1]. This situation should be avoided.
> > >
> > > [1]
> > > renameat2(r0, &(0x7f0000000240)='./bus/file0\x00', r0, &(0x7f00000001c0)='./file0\x00', 0x0)
> > >
> > > Reported-by: syzbot+321477fad98ea6dd35b7@...kaller.appspotmail.com
> > > Closes: https://syzkaller.appspot.com/bug?extid=321477fad98ea6dd35b7
> > > Tested-by: syzbot+321477fad98ea6dd35b7@...kaller.appspotmail.com
> > > Signed-off-by: Edward Adam Davis <eadavis@...com>
> > > ---
> > >  fs/namei.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/fs/namei.c b/fs/namei.c
> > > index 84a0e0b0111c..ff843007ca94 100644
> > > --- a/fs/namei.c
> > > +++ b/fs/namei.c
> > > @@ -5013,7 +5013,7 @@ int vfs_rename(struct renamedata *rd)
> > >     struct name_snapshot old_name;
> > >     bool lock_old_subdir, lock_new_subdir;
> > >
> > > -   if (source == target)
> > > +   if (source == target || old_dir == target)
> > >             return 0;
> >
> > What the hell?
> >
> > 1) olddirfd and newdirfd have nothing to do with vfs_rename() - they are
> > bloody well gone by the time we get there.
> >
> > 2) there's nothing wrong with having the same value passed in both -
> > and it's certainly not a "quietly do nothing".
> >
> > 3) the check added in this patch is... odd.  You are checking essentically
> > for rename("foo/bar", "foo").  It should fail (-ENOTEMPTY or -EINVAL, depending
> > upon RENAME_EXCHANGE in flags) without having reached vfs_rename().
>
> 4) it's definitely an exfat bug, since we are getting
>         old_dentry->d_parent != target
>         old_dentry->d_parent->d_inode == target->d_inode
>         S_ISDIR(target->d_inode->i_mode)
> All objects involved are on the same super_block, which has "exfat" for
> ->s_type->name, so it's exfat ending up with multiple dentries for
> the same directory inode, and once that kind of thing has happened,
> the system is FUBAR.
>
> As for the root cause, almost certainly their ->mkdir() is deciding
> that it has just created a new inode - and ending up with existing one,
> already in icache and already with a dentry attached to it.
>
> <adds BUG_ON(!hlist_empty(&inode->i_dentry)) into exfat_mkdir()>
>
>    [   84.780875] exFAT-fs (loop0): Volume was not properly unmounted. Some data may be corrupt. Please run fsck.
>    [   84.781411] exFAT-fs (loop0): Medium has reported failures. Some data may be lost.
>    [   84.782209] exFAT-fs (loop0): failed to load upcase table (idx : 0x00010000, chksum : 0xe62de5da, utbl_chksum : 0xe619d30d)
>    [   84.783272] ------------[ cut here ]------------
>    [   84.783546] kernel BUG at fs/exfat/namei.c:881!
>
> ... and there we go.  exfat_mkdir() getting an existing in-core inode
> and attaching an alias to it, with expected fun results.
>
> For crying out loud, how many times do syzbot folks need to be told that
> getting report to attention of relevant filesystem folks is important?
>
> Subject: [syzbot] [fs?] INFO: task hung in vfs_rename (2)
>
> mentionings of anything exfat-related: 0.
>
> Cc: brauner@...nel.org, jack@...e.cz, linux-fsdevel@...r.kernel.org,
>          linux-kernel@...r.kernel.org, syzkaller-bugs@...glegroups.com,
>                  viro@...iv.linux.org.uk
>
> mentionings of anything exfat-related: 0.
>
> In message body:
>   fsck result: failed (log: https://syzkaller.appspot.com/x/fsck.log?x=12655204580000)
>
> Why, that does sound like some filesystem bug might be involved
> and presumably the damn thing knows which type had it been.
> <start browser, cut'n'paste the sodding link>
> ... and the very first line is
> fsck.exfat -n exited with status code 4
>
> Result: 3 weeks later it *STILL* hasn't reached the relevant fs
> maintainers.  Could that be a sufficient evidence to convince the
> fine fellows working on syzbot that "you just need to click a few
> links" DOES NOT WORK?
>
> We'd been there several times already.  For relatively polite example,
> see https://lore.kernel.org/all/Y5ZDjuSNuSLJd8Mn@ZenIV/ - I can't be arsed
> to explain that again and again, and you don't seem to mind following
> links in email, so...
>

I've checked the code, and there was indeed a bug in our
classification rules, specifically concerning the recognition of the
`syz_mount_image$exfat` call as an indicator for the "exfat"
subsystem. The fix will reach syzbot soon.

Sorry for the inconvenience it has caused.

-- 
Aleksandr

Powered by blists - more mailing lists