Re: [syzbot] [ntfs?] WARNING in do_open_execat

Re: [syzbot] [ntfs?] WARNING in do_open_execat

[Kees Cook]

On Fri, Aug 18, 2023 at 11:26:51AM -0500, Eric W. Biederman wrote:
> syzbot <syzbot+6ec38f7a8db3b3fb1002@syzkaller.appspotmail.com> writes:
> 
> > Hello,
> >
> > syzbot found the following issue on:
> 
> Not an issue.
> Nothing to do with ntfs.
> 
> The code is working as designed and intended.
> 
> syzbot generated a malformed exec and the kernel made it
> well formed and warned about it.
> 
> Human beings who run syzbot please mark this as not an issue in your
> system.  The directions don't have a way to say that the code is working
> as expected and designed.

WARN and BUG should not be reachable from userspace, so if this can be
tripped we should take a closer look and likely fix it...

> > HEAD commit:    16931859a650 Merge tag 'nfsd-6.5-4' of git://git.kernel.or..
> > git tree:       upstream
> > console+strace: https://syzkaller.appspot.com/x/log.txt?x=13e2673da80000
> > kernel config:  https://syzkaller.appspot.com/x/.config?x=aa796b6080b04102
> > dashboard link: https://syzkaller.appspot.com/bug?extid=6ec38f7a8db3b3fb1002
> > compiler:       gcc (Debian 12.2.0-14) 12.2.0, GNU ld (GNU Binutils for Debian) 2.40
> > syz repro:      https://syzkaller.appspot.com/x/repro.syz?x=17cdbc65a80000
> > C reproducer:   https://syzkaller.appspot.com/x/repro.c?x=1262d8cfa80000
> >
> > Downloadable assets:
> > disk image: https://storage.googleapis.com/syzbot-assets/eecc010800b4/disk-16931859.raw.xz
> > vmlinux: https://storage.googleapis.com/syzbot-assets/f45ae06377a7/vmlinux-16931859.xz
> > kernel image: https://storage.googleapis.com/syzbot-assets/68891896edba/bzImage-16931859.xz
> > mounted in repro: https://storage.googleapis.com/syzbot-assets/4b6ab78b223a/mount_0.gz
> >
> > IMPORTANT: if you fix the issue, please add the following tag to the commit:
> > Reported-by: syzbot+6ec38f7a8db3b3fb1002@syzkaller.appspotmail.com
> >
> > ntfs: volume version 3.1.
> > process 'syz-executor300' launched './file1' with NULL argv: empty string added
> > ------------[ cut here ]------------
> > WARNING: CPU: 0 PID: 5020 at fs/exec.c:933 do_open_execat+0x18f/0x3f0 fs/exec.c:933

This is a double-check I left in place, since it shouldn't have been reachable:

        /*
         * may_open() has already checked for this, so it should be
         * impossible to trip now. But we need to be extra cautious
         * and check again at the very end too.
         */
        err = -EACCES;
        if (WARN_ON_ONCE(!S_ISREG(file_inode(file)->i_mode) ||
                         path_noexec(&file->f_path)))
                goto exit;

So yes, let's figure this out...

-- 
Kees Cook

Re: [syzbot] [ntfs?] WARNING in do_open_execat

[Matthew Wilcox]

On Fri, Aug 18, 2023 at 10:33:26AM -0700, Kees Cook wrote:
> On Fri, Aug 18, 2023 at 11:26:51AM -0500, Eric W. Biederman wrote:
> > syzbot <syzbot+6ec38f7a8db3b3fb1002@syzkaller.appspotmail.com> writes:
> > 
> > > Hello,
> > >
> > > syzbot found the following issue on:
> > 
> > Not an issue.
> > Nothing to do with ntfs.
> > 
> > The code is working as designed and intended.
> > 
> > syzbot generated a malformed exec and the kernel made it
> > well formed and warned about it.
> > 
> > Human beings who run syzbot please mark this as not an issue in your
> > system.  The directions don't have a way to say that the code is working
> > as expected and designed.
> 
> WARN and BUG should not be reachable from userspace, so if this can be
> tripped we should take a closer look and likely fix it...
> 
> > > HEAD commit:    16931859a650 Merge tag 'nfsd-6.5-4' of git://git.kernel.or..
> > > git tree:       upstream
> > > console+strace: https://syzkaller.appspot.com/x/log.txt?x=13e2673da80000
> > > kernel config:  https://syzkaller.appspot.com/x/.config?x=aa796b6080b04102
> > > dashboard link: https://syzkaller.appspot.com/bug?extid=6ec38f7a8db3b3fb1002
> > > compiler:       gcc (Debian 12.2.0-14) 12.2.0, GNU ld (GNU Binutils for Debian) 2.40
> > > syz repro:      https://syzkaller.appspot.com/x/repro.syz?x=17cdbc65a80000
> > > C reproducer:   https://syzkaller.appspot.com/x/repro.c?x=1262d8cfa80000
> > >
> > > Downloadable assets:
> > > disk image: https://storage.googleapis.com/syzbot-assets/eecc010800b4/disk-16931859.raw.xz
> > > vmlinux: https://storage.googleapis.com/syzbot-assets/f45ae06377a7/vmlinux-16931859.xz
> > > kernel image: https://storage.googleapis.com/syzbot-assets/68891896edba/bzImage-16931859.xz
> > > mounted in repro: https://storage.googleapis.com/syzbot-assets/4b6ab78b223a/mount_0.gz
> > >
> > > IMPORTANT: if you fix the issue, please add the following tag to the commit:
> > > Reported-by: syzbot+6ec38f7a8db3b3fb1002@syzkaller.appspotmail.com
> > >
> > > ntfs: volume version 3.1.
> > > process 'syz-executor300' launched './file1' with NULL argv: empty string added
> > > ------------[ cut here ]------------
> > > WARNING: CPU: 0 PID: 5020 at fs/exec.c:933 do_open_execat+0x18f/0x3f0 fs/exec.c:933
> 
> This is a double-check I left in place, since it shouldn't have been reachable:
> 
>         /*
>          * may_open() has already checked for this, so it should be
>          * impossible to trip now. But we need to be extra cautious
>          * and check again at the very end too.
>          */
>         err = -EACCES;
>         if (WARN_ON_ONCE(!S_ISREG(file_inode(file)->i_mode) ||
>                          path_noexec(&file->f_path)))
>                 goto exit;
> 
> So yes, let's figure this out...

When trying to figure it out, remember that ntfs corrupts random memory,
so all reports from syzbot that have "ntfs" in them should be discarded.
I tried to tell them that all this work they're doing testing ntfs3 is
pointless, but they won't listen.

Re: [syzbot] [ntfs?] WARNING in do_open_execat

[Kees Cook]

On Fri, Aug 18, 2023 at 06:43:11PM +0100, Matthew Wilcox wrote:
> On Fri, Aug 18, 2023 at 10:33:26AM -0700, Kees Cook wrote:
> > On Fri, Aug 18, 2023 at 11:26:51AM -0500, Eric W. Biederman wrote:
> > > syzbot <syzbot+6ec38f7a8db3b3fb1002@syzkaller.appspotmail.com> writes:
> > > 
> > > > Hello,
> > > >
> > > > syzbot found the following issue on:
> > > 
> > > Not an issue.
> > > Nothing to do with ntfs.
> > > 
> > > The code is working as designed and intended.
> > > 
> > > syzbot generated a malformed exec and the kernel made it
> > > well formed and warned about it.
> > > 
> > > Human beings who run syzbot please mark this as not an issue in your
> > > system.  The directions don't have a way to say that the code is working
> > > as expected and designed.
> > 
> > WARN and BUG should not be reachable from userspace, so if this can be
> > tripped we should take a closer look and likely fix it...
> > 
> > > > HEAD commit:    16931859a650 Merge tag 'nfsd-6.5-4' of git://git.kernel.or..
> > > > git tree:       upstream
> > > > console+strace: https://syzkaller.appspot.com/x/log.txt?x=13e2673da80000
> > > > kernel config:  https://syzkaller.appspot.com/x/.config?x=aa796b6080b04102
> > > > dashboard link: https://syzkaller.appspot.com/bug?extid=6ec38f7a8db3b3fb1002
> > > > compiler:       gcc (Debian 12.2.0-14) 12.2.0, GNU ld (GNU Binutils for Debian) 2.40
> > > > syz repro:      https://syzkaller.appspot.com/x/repro.syz?x=17cdbc65a80000
> > > > C reproducer:   https://syzkaller.appspot.com/x/repro.c?x=1262d8cfa80000
> > > >
> > > > Downloadable assets:
> > > > disk image: https://storage.googleapis.com/syzbot-assets/eecc010800b4/disk-16931859.raw.xz
> > > > vmlinux: https://storage.googleapis.com/syzbot-assets/f45ae06377a7/vmlinux-16931859.xz
> > > > kernel image: https://storage.googleapis.com/syzbot-assets/68891896edba/bzImage-16931859.xz
> > > > mounted in repro: https://storage.googleapis.com/syzbot-assets/4b6ab78b223a/mount_0.gz
> > > >
> > > > IMPORTANT: if you fix the issue, please add the following tag to the commit:
> > > > Reported-by: syzbot+6ec38f7a8db3b3fb1002@syzkaller.appspotmail.com
> > > >
> > > > ntfs: volume version 3.1.
> > > > process 'syz-executor300' launched './file1' with NULL argv: empty string added
> > > > ------------[ cut here ]------------
> > > > WARNING: CPU: 0 PID: 5020 at fs/exec.c:933 do_open_execat+0x18f/0x3f0 fs/exec.c:933
> > 
> > This is a double-check I left in place, since it shouldn't have been reachable:
> > 
> >         /*
> >          * may_open() has already checked for this, so it should be
> >          * impossible to trip now. But we need to be extra cautious
> >          * and check again at the very end too.
> >          */
> >         err = -EACCES;
> >         if (WARN_ON_ONCE(!S_ISREG(file_inode(file)->i_mode) ||
> >                          path_noexec(&file->f_path)))
> >                 goto exit;
> > 
> > So yes, let's figure this out...
> 
> When trying to figure it out, remember that ntfs corrupts random memory,

!! Oh. Well, then yeah, that's not great. :(

-Kees

> so all reports from syzbot that have "ntfs" in them should be discarded.
> I tried to tell them that all this work they're doing testing ntfs3 is
> pointless, but they won't listen.

-- 
Kees Cook

Re: [syzbot] [ntfs?] WARNING in do_open_execat

[Mateusz Guzik]

On Fri, Aug 18, 2023 at 10:33:26AM -0700, Kees Cook wrote:
> This is a double-check I left in place, since it shouldn't have been reachable:
> 
>         /*
>          * may_open() has already checked for this, so it should be
>          * impossible to trip now. But we need to be extra cautious
>          * and check again at the very end too.
>          */
>         err = -EACCES;
>         if (WARN_ON_ONCE(!S_ISREG(file_inode(file)->i_mode) ||
>                          path_noexec(&file->f_path)))
>                 goto exit;
> 

As I mentioned in my other e-mail, the check is racy -- an unlucky
enough remounting with noexec should trip over it, and probably a chmod
too.

However, that's not what triggers the warn in this case.

The ntfs image used here is intentionally corrupted and the inode at
hand has a mode of 777 (as in type not specified).

Then the type check in may_open():
        switch (inode->i_mode & S_IFMT) {

fails to match anything.

This debug printk:
diff --git a/fs/namei.c b/fs/namei.c
index e56ff39a79bc..05652e8a1069 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -3259,6 +3259,10 @@ static int may_open(struct mnt_idmap *idmap, const struct path *path,
                if ((acc_mode & MAY_EXEC) && path_noexec(path))
                        return -EACCES;
                break;
+       default:
+               /* bogus mode! */
+               printk(KERN_EMERG "got bogus mode inode!\n");
+               return -EACCES;
        }

        error = inode_permission(idmap, inode, MAY_OPEN | acc_mode);

catches it.

All that said, I think adding a WARN_ONCE here is prudent, but I
don't know if denying literally all opts is the way to go.

Do other filesystems have provisions to prevent inodes like this from
getting here?

Re: [syzbot] [ntfs?] WARNING in do_open_execat

[Christian Brauner]

On Fri, Aug 18, 2023 at 09:12:39PM +0200, Mateusz Guzik wrote:
> On Fri, Aug 18, 2023 at 10:33:26AM -0700, Kees Cook wrote:
> > This is a double-check I left in place, since it shouldn't have been reachable:
> > 
> >         /*
> >          * may_open() has already checked for this, so it should be
> >          * impossible to trip now. But we need to be extra cautious
> >          * and check again at the very end too.
> >          */
> >         err = -EACCES;
> >         if (WARN_ON_ONCE(!S_ISREG(file_inode(file)->i_mode) ||
> >                          path_noexec(&file->f_path)))
> >                 goto exit;
> > 
> 
> As I mentioned in my other e-mail, the check is racy -- an unlucky
> enough remounting with noexec should trip over it, and probably a chmod
> too.
> 
> However, that's not what triggers the warn in this case.
> 
> The ntfs image used here is intentionally corrupted and the inode at
> hand has a mode of 777 (as in type not specified).
> 
> Then the type check in may_open():
>         switch (inode->i_mode & S_IFMT) {
> 
> fails to match anything.
> 
> This debug printk:
> diff --git a/fs/namei.c b/fs/namei.c
> index e56ff39a79bc..05652e8a1069 100644
> --- a/fs/namei.c
> +++ b/fs/namei.c
> @@ -3259,6 +3259,10 @@ static int may_open(struct mnt_idmap *idmap, const struct path *path,
>                 if ((acc_mode & MAY_EXEC) && path_noexec(path))
>                         return -EACCES;
>                 break;
> +       default:
> +               /* bogus mode! */
> +               printk(KERN_EMERG "got bogus mode inode!\n");
> +               return -EACCES;
>         }
> 
>         error = inode_permission(idmap, inode, MAY_OPEN | acc_mode);
> 
> catches it.
> 
> All that said, I think adding a WARN_ONCE here is prudent, but I
> don't know if denying literally all opts is the way to go.
> 
> Do other filesystems have provisions to prevent inodes like this from
> getting here?

Bugs reported against the VFS from ntfs/ntfs3 are to be treated with
extreme caution. Frankly, if it isn't reproducible without a corrupted
ntfs/ntfs3 image it is to be dismissed until further notice.

In this case it simply seems that ntfs is failing at ensuring that its
own inodes it reads from disk have a well-defined type.

If ntfs fails to validate that its own inodes it puts into the icache
are correctly initialized then the vfs doesn't need to try and taper
over this.

If ntfs fails at that, there's no guarantee that it doesn't also fail at
setting the correct i_ops for that inode. At which point we can check
the type in may_open() but we already used bogus i_ops the whole time on
some other inodes.

We're not here to make up for silly bugs like this. That WARN belongs
into ntfs not the vfs.

Re: [syzbot] [ntfs?] WARNING in do_open_execat

[Mateusz Guzik]

On 8/19/23, Christian Brauner <brauner@kernel.org> wrote:
> On Fri, Aug 18, 2023 at 09:12:39PM +0200, Mateusz Guzik wrote:
>> On Fri, Aug 18, 2023 at 10:33:26AM -0700, Kees Cook wrote:
>> > This is a double-check I left in place, since it shouldn't have been
>> > reachable:
>> >
>> >         /*
>> >          * may_open() has already checked for this, so it should be
>> >          * impossible to trip now. But we need to be extra cautious
>> >          * and check again at the very end too.
>> >          */
>> >         err = -EACCES;
>> >         if (WARN_ON_ONCE(!S_ISREG(file_inode(file)->i_mode) ||
>> >                          path_noexec(&file->f_path)))
>> >                 goto exit;
>> >
>>
>> As I mentioned in my other e-mail, the check is racy -- an unlucky
>> enough remounting with noexec should trip over it, and probably a chmod
>> too.
>>
>> However, that's not what triggers the warn in this case.
>>
>> The ntfs image used here is intentionally corrupted and the inode at
>> hand has a mode of 777 (as in type not specified).
>>
>> Then the type check in may_open():
>>         switch (inode->i_mode & S_IFMT) {
>>
>> fails to match anything.
>>
>> This debug printk:
>> diff --git a/fs/namei.c b/fs/namei.c
>> index e56ff39a79bc..05652e8a1069 100644
>> --- a/fs/namei.c
>> +++ b/fs/namei.c
>> @@ -3259,6 +3259,10 @@ static int may_open(struct mnt_idmap *idmap, const
>> struct path *path,
>>                 if ((acc_mode & MAY_EXEC) && path_noexec(path))
>>                         return -EACCES;
>>                 break;
>> +       default:
>> +               /* bogus mode! */
>> +               printk(KERN_EMERG "got bogus mode inode!\n");
>> +               return -EACCES;
>>         }
>>
>>         error = inode_permission(idmap, inode, MAY_OPEN | acc_mode);
>>
>> catches it.
>>
>> All that said, I think adding a WARN_ONCE here is prudent, but I
>> don't know if denying literally all opts is the way to go.
>>
>> Do other filesystems have provisions to prevent inodes like this from
>> getting here?
>
> Bugs reported against the VFS from ntfs/ntfs3 are to be treated with
> extreme caution. Frankly, if it isn't reproducible without a corrupted
> ntfs/ntfs3 image it is to be dismissed until further notice.
>
> In this case it simply seems that ntfs is failing at ensuring that its
> own inodes it reads from disk have a well-defined type.
>
> If ntfs fails to validate that its own inodes it puts into the icache
> are correctly initialized then the vfs doesn't need to try and taper
> over this.
>
> If ntfs fails at that, there's no guarantee that it doesn't also fail at
> setting the correct i_ops for that inode. At which point we can check
> the type in may_open() but we already used bogus i_ops the whole time on
> some other inodes.
>
> We're not here to make up for silly bugs like this. That WARN belongs
> into ntfs not the vfs.
>

Given the triggered WARN_ON it seemed to me this would be the
operating procedure, I am happy it is not ;)

Per your description and the one provided by Theodore I take it
filesystems must not ship botched inodes like this one.

While in this case this is a clear-cut bug in ntfs, I would argue the
entire ordeal exposes a deficiency in VFS -- it should have a
debug-only mechanism which catches cases like this early on. For
example there could be a mandatory function to call when the
filesystem claims it constructed the inode to assert a bunch on it --
it would not catch all possible problems, but would definitely catch
this one (and VFS would have to detect the call was not made).

Perhaps I should write a separate e-mail about this, but I'm surprised
there is no debug-only (as in not present in production kernels)
support for asserting the state. To give one example which makes me
itchy see inode destruction. There are few checks in clear_inode, but
past that there is almost nothing.

Similarly there are quite a few comments how the caller is required to
hold a given lock, which should have been converted to lockdep asserts
years ago.

I'm going to write something up later.

-- 
Mateusz Guzik <mjguzik gmail.com>

Re: [syzbot] [ntfs?] WARNING in do_open_execat

[Theodore Ts'o]

On Fri, Aug 18, 2023 at 09:12:39PM +0200, Mateusz Guzik wrote:
> 
> The ntfs image used here is intentionally corrupted and the inode at
> hand has a mode of 777 (as in type not specified).
> 
> Then the type check in may_open():
>         switch (inode->i_mode & S_IFMT) {
> 
> fails to match anything.
> ...
> 
> Do other filesystems have provisions to prevent inodes like this from
> getting here?

Well, what ext4 does is that we do a bunch of basic validity checks in
ext4_iget(), and if the inode is bad (for example the type is not
specified), the following gets executed:

	} else {
		ret = -EFSCORRUPTED;
		ext4_error_inode(inode, function, line, 0,
				 "iget: bogus i_mode (%o)", inode->i_mode);
		goto bad_inode;
       ...

bad_inode:
	brelse(iloc.bh);
	iget_failed(inode);
	return ERR_PTR(ret);
       
iget_failed() takes the inode under construction (returned by
iget_locked), and marks it as a bad/"dead" inode.  So subsequent
attempts to do anything with the inode, including opening it, will
fail at the VFS level, and you never get to the file system's open
function.

The ext4_error_inode() function is reponsible for logging the error,
and if userspace is using fsnotify and are subscribed FS_ERROR,
notifying user space that the file system is corrupted.  Depending on
the file system settings, we may also remount the file system
read-only, or force a panic to reboot the system (so that a failover
backup server can take over), or just log the message and continuing.

       	      	       	      	      	      - Ted

Re: [syzbot] [ntfs?] WARNING in do_open_execat

[Eric W. Biederman]

Mateusz Guzik <mjguzik@gmail.com> writes:

> On Fri, Aug 18, 2023 at 11:26:51AM -0500, Eric W. Biederman wrote:
>> syzbot <syzbot+6ec38f7a8db3b3fb1002@syzkaller.appspotmail.com> writes:
>> 
>> > Hello,
>> >
>> > syzbot found the following issue on:
>> 
>> Not an issue.
>> Nothing to do with ntfs.
>> 
>> The code is working as designed and intended.
>> 
>> syzbot generated a malformed exec and the kernel made it
>> well formed and warned about it.
>> 
>
> There is definitely an issue here.
>
> The warn on comes from:
>         /*
>          * may_open() has already checked for this, so it should be
>          * impossible to trip now. But we need to be extra cautious
>          * and check again at the very end too.
>          */
>         err = -EACCES;
>         if (WARN_ON_ONCE(!S_ISREG(file_inode(file)->i_mode) ||
>                          path_noexec(&file->f_path)))
>                 goto exit;
>
> Where path_noexec is:
>         return (path->mnt->mnt_flags & MNT_NOEXEC) ||
>                (path->mnt->mnt_sb->s_iflags & SB_I_NOEXEC);

My confusion.

I was seeing the message from
	if (retval == 0)
		pr_warn_once("process '%s' launched '%s' with NULL argv: empty string added\n",
			     current->comm, bprm->filename);

I made the mistake of assuming that that was generating the backtrace.
The lack of args to execveat appears to be working fine.

I see you tracked this down to a non-exhaustive check in may_open.
Apologies for the noise.

Eric

Re: [syzbot] [ntfs3?] WARNING in do_open_execat

[Jan Kara]

On Mon 11-03-24 11:04:04, syzbot wrote:
> syzbot suspects this issue was fixed by commit:
> 
> commit 6f861765464f43a71462d52026fbddfc858239a5
> Author: Jan Kara <jack@suse.cz>
> Date:   Wed Nov 1 17:43:10 2023 +0000
> 
>     fs: Block writes to mounted block devices
> 
> bisection log:  https://syzkaller.appspot.com/x/bisect.txt?x=17e3f58e180000
> start commit:   eb3479bc23fa Merge tag 'kbuild-fixes-v6.7' of git://git.ke..
> git tree:       upstream
> kernel config:  https://syzkaller.appspot.com/x/.config?x=bdf178b2f20f99b0
> dashboard link: https://syzkaller.appspot.com/bug?extid=6ec38f7a8db3b3fb1002
> syz repro:      https://syzkaller.appspot.com/x/repro.syz?x=15073fd4e80000
> C reproducer:   https://syzkaller.appspot.com/x/repro.c?x=17b20b8f680000
> 
> If the result looks correct, please mark the issue as fixed by replying with:
 
#syz fix: fs: Block writes to mounted block devices

								Honza
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

Re: [syzbot] [ntfs3?] WARNING in do_open_execat

[Mateusz Guzik]

On 3/11/24, Jan Kara <jack@suse.cz> wrote:
> On Mon 11-03-24 11:04:04, syzbot wrote:
>> syzbot suspects this issue was fixed by commit:
>>
>> commit 6f861765464f43a71462d52026fbddfc858239a5
>> Author: Jan Kara <jack@suse.cz>
>> Date:   Wed Nov 1 17:43:10 2023 +0000
>>
>>     fs: Block writes to mounted block devices
>>
>> bisection log:
>> https://syzkaller.appspot.com/x/bisect.txt?x=17e3f58e180000
>> start commit:   eb3479bc23fa Merge tag 'kbuild-fixes-v6.7' of
>> git://git.ke..
>> git tree:       upstream
>> kernel config:
>> https://syzkaller.appspot.com/x/.config?x=bdf178b2f20f99b0
>> dashboard link:
>> https://syzkaller.appspot.com/bug?extid=6ec38f7a8db3b3fb1002
>> syz repro:
>> https://syzkaller.appspot.com/x/repro.syz?x=15073fd4e80000
>> C reproducer:   https://syzkaller.appspot.com/x/repro.c?x=17b20b8f680000
>>
>> If the result looks correct, please mark the issue as fixed by replying
>> with:
>
> #syz fix: fs: Block writes to mounted block devices
>

I don't think that's correct.

The bug is ntfs instantiating an inode with bogus type (based on an
intentionally corrupted filesystem), violating the api contract with
vfs, which in turn results in the warning way later.

It may be someone sorted out ntfs doing this in the meantime, I have
not checked.

With this in mind I don't believe your patch fixed it, at best it
happened to neuter the reproducer.

vfs could definitely be patched to catch this when I_NEW is getting
cleared (only when  running with debug), not in the spot which
generates the warn.

-- 
Mateusz Guzik <mjguzik gmail.com>

Re: [syzbot] [ntfs3?] WARNING in do_open_execat

[Jan Kara]

On Mon 11-03-24 20:01:14, Mateusz Guzik wrote:
> On 3/11/24, Jan Kara <jack@suse.cz> wrote:
> > On Mon 11-03-24 11:04:04, syzbot wrote:
> >> syzbot suspects this issue was fixed by commit:
> >>
> >> commit 6f861765464f43a71462d52026fbddfc858239a5
> >> Author: Jan Kara <jack@suse.cz>
> >> Date:   Wed Nov 1 17:43:10 2023 +0000
> >>
> >>     fs: Block writes to mounted block devices
> >>
> >> bisection log:
> >> https://syzkaller.appspot.com/x/bisect.txt?x=17e3f58e180000
> >> start commit:   eb3479bc23fa Merge tag 'kbuild-fixes-v6.7' of
> >> git://git.ke..
> >> git tree:       upstream
> >> kernel config:
> >> https://syzkaller.appspot.com/x/.config?x=bdf178b2f20f99b0
> >> dashboard link:
> >> https://syzkaller.appspot.com/bug?extid=6ec38f7a8db3b3fb1002
> >> syz repro:
> >> https://syzkaller.appspot.com/x/repro.syz?x=15073fd4e80000
> >> C reproducer:   https://syzkaller.appspot.com/x/repro.c?x=17b20b8f680000
> >>
> >> If the result looks correct, please mark the issue as fixed by replying
> >> with:
> >
> > #syz fix: fs: Block writes to mounted block devices
> >
> 
> I don't think that's correct.
> 
> The bug is ntfs instantiating an inode with bogus type (based on an
> intentionally corrupted filesystem), violating the api contract with
> vfs, which in turn results in the warning way later.
> 
> It may be someone sorted out ntfs doing this in the meantime, I have
> not checked.
> 
> With this in mind I don't believe your patch fixed it, at best it
> happened to neuter the reproducer.

OK, I didn't dig deep into the bug. I've just seen there are no working
reproducers and given this is ntfs3 which doesn't really have great
maintenance effort put into it, I've opted for closing the bug. If there's
a way to tickle the bug without writing to mounted block device, syzbot
should eventually find it and create a new issue... But if you want to look
into this feel free to :) Thanks for sharing the info.

								Honza

-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

Re: [syzbot] [ntfs3?] WARNING in do_open_execat

[Mateusz Guzik]

On 3/12/24, Jan Kara <jack@suse.cz> wrote:
> On Mon 11-03-24 20:01:14, Mateusz Guzik wrote:
>> On 3/11/24, Jan Kara <jack@suse.cz> wrote:
>> > On Mon 11-03-24 11:04:04, syzbot wrote:
>> >> syzbot suspects this issue was fixed by commit:
>> >>
>> >> commit 6f861765464f43a71462d52026fbddfc858239a5
>> >> Author: Jan Kara <jack@suse.cz>
>> >> Date:   Wed Nov 1 17:43:10 2023 +0000
>> >>
>> >>     fs: Block writes to mounted block devices
>> >>
>> >> bisection log:
>> >> https://syzkaller.appspot.com/x/bisect.txt?x=17e3f58e180000
>> >> start commit:   eb3479bc23fa Merge tag 'kbuild-fixes-v6.7' of
>> >> git://git.ke..
>> >> git tree:       upstream
>> >> kernel config:
>> >> https://syzkaller.appspot.com/x/.config?x=bdf178b2f20f99b0
>> >> dashboard link:
>> >> https://syzkaller.appspot.com/bug?extid=6ec38f7a8db3b3fb1002
>> >> syz repro:
>> >> https://syzkaller.appspot.com/x/repro.syz?x=15073fd4e80000
>> >> C reproducer:
>> >> https://syzkaller.appspot.com/x/repro.c?x=17b20b8f680000
>> >>
>> >> If the result looks correct, please mark the issue as fixed by
>> >> replying
>> >> with:
>> >
>> > #syz fix: fs: Block writes to mounted block devices
>> >
>>
>> I don't think that's correct.
>>
>> The bug is ntfs instantiating an inode with bogus type (based on an
>> intentionally corrupted filesystem), violating the api contract with
>> vfs, which in turn results in the warning way later.
>>
>> It may be someone sorted out ntfs doing this in the meantime, I have
>> not checked.
>>
>> With this in mind I don't believe your patch fixed it, at best it
>> happened to neuter the reproducer.
>
> OK, I didn't dig deep into the bug. I've just seen there are no working
> reproducers and given this is ntfs3 which doesn't really have great
> maintenance effort put into it, I've opted for closing the bug. If there's
> a way to tickle the bug without writing to mounted block device, syzbot
> should eventually find it and create a new issue... But if you want to look
> into this feel free to :) Thanks for sharing the info.
>

Maybe I'll get around to future-proofing by adding validation before
the inode escapes the filesystem code, but I'm definitely NOT patching
ntfs. 8->

-- 
Mateusz Guzik <mjguzik gmail.com>