1. Patchew
  2. linux
  3. View series

[RFC PATCH] vfs: exclude ntfs3 from file mode validation in may_open()

Tetsuo Handa posted 1 patch 1 month, 3 weeks ago 
fs/namei.c | 6 ++++++
1 file changed, 6 insertions(+)
[RFC PATCH] vfs: exclude ntfs3 from file mode validation in may_open()
Posted by Tetsuo Handa 1 month, 3 weeks ago 
Since ntfs_read_mft() not only accepts file modes which may_open() accepts
but also accepts

  (fname && fname->home.low == cpu_to_le32(MFT_REC_EXTEND) &&
   fname->home.seq == cpu_to_le16(MFT_REC_EXTEND)

case when the file mode is none of
S_IFDIR/S_IFLNK/S_IFREG/S_IFCHR/S_IFBLK/S_IFIFO/S_IFSOCK, may_open() cannot
unconditionally expect IS_ANON_FILE(inode) when the file mode is none of
S_IFDIR/S_IFLNK/S_IFREG/S_IFCHR/S_IFBLK/S_IFIFO/S_IFSOCK.

Treat as if S_IFREG when the inode is for NTFS3 filesystem.

Reported-by: syzbot <syzbot+895c23f6917da440ed0d@syzkaller.appspotmail.com>
Closes: https://syzkaller.appspot.com/bug?extid=895c23f6917da440ed0d
Fixes: af153bb63a33 ("vfs: catch invalid modes in may_open()")
Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
---
Is it possible to handle this problem on the NTFS3 side?

  --- a/fs/ntfs3/inode.c
  +++ b/fs/ntfs3/inode.c
  @@ -470,8 +470,9 @@ static struct inode *ntfs_read_mft(struct inode *inode,
          } else if (fname && fname->home.low == cpu_to_le32(MFT_REC_EXTEND) &&
                     fname->home.seq == cpu_to_le16(MFT_REC_EXTEND)) {
                  /* Records in $Extend are not a files or general directories. */
                  inode->i_op = &ntfs_file_inode_operations;
  +               mode = S_IFREG;
          } else {
                  err = -EINVAL;
                  goto out;
          }

I don't know what breaks if we pretend as if S_IFREG...

 fs/namei.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/fs/namei.c b/fs/namei.c
index cd43ff89fbaa..a66599754394 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -3471,6 +3471,12 @@ static int may_open(struct mnt_idmap *idmap, const struct path *path,
 			return -EACCES;
 		break;
 	default:
+		/* Special handling for ntfs_read_mft() case. */
+		if (inode->i_sb->s_magic == 0x7366746e) {
+			if ((acc_mode & MAY_EXEC) && path_noexec(path))
+				return -EACCES;
+			break;
+		}
 		VFS_BUG_ON_INODE(!IS_ANON_FILE(inode), inode);
 	}
 
-- 
2.50.1
Re: [RFC PATCH] vfs: exclude ntfs3 from file mode validation in may_open()
Posted by Christian Brauner 1 month, 3 weeks ago 
On Mon, Aug 11, 2025 at 04:05:51PM +0900, Tetsuo Handa wrote:
> Since ntfs_read_mft() not only accepts file modes which may_open() accepts
> but also accepts
> 
>   (fname && fname->home.low == cpu_to_le32(MFT_REC_EXTEND) &&
>    fname->home.seq == cpu_to_le16(MFT_REC_EXTEND)
> 
> case when the file mode is none of
> S_IFDIR/S_IFLNK/S_IFREG/S_IFCHR/S_IFBLK/S_IFIFO/S_IFSOCK, may_open() cannot
> unconditionally expect IS_ANON_FILE(inode) when the file mode is none of
> S_IFDIR/S_IFLNK/S_IFREG/S_IFCHR/S_IFBLK/S_IFIFO/S_IFSOCK.
> 
> Treat as if S_IFREG when the inode is for NTFS3 filesystem.
> 
> Reported-by: syzbot <syzbot+895c23f6917da440ed0d@syzkaller.appspotmail.com>
> Closes: https://syzkaller.appspot.com/bug?extid=895c23f6917da440ed0d
> Fixes: af153bb63a33 ("vfs: catch invalid modes in may_open()")
> Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
> ---
> Is it possible to handle this problem on the NTFS3 side?

Ugh, this is annoying.
@Konstantin, why do you leave a zero mode for these files?
Let's just make them regular files?

> 
>   --- a/fs/ntfs3/inode.c
>   +++ b/fs/ntfs3/inode.c
>   @@ -470,8 +470,9 @@ static struct inode *ntfs_read_mft(struct inode *inode,
>           } else if (fname && fname->home.low == cpu_to_le32(MFT_REC_EXTEND) &&
>                      fname->home.seq == cpu_to_le16(MFT_REC_EXTEND)) {
>                   /* Records in $Extend are not a files or general directories. */
>                   inode->i_op = &ntfs_file_inode_operations;
>   +               mode = S_IFREG;
>           } else {
>                   err = -EINVAL;
>                   goto out;
>           }
> 
> I don't know what breaks if we pretend as if S_IFREG...
> 
>  fs/namei.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/fs/namei.c b/fs/namei.c
> index cd43ff89fbaa..a66599754394 100644
> --- a/fs/namei.c
> +++ b/fs/namei.c
> @@ -3471,6 +3471,12 @@ static int may_open(struct mnt_idmap *idmap, const struct path *path,
>  			return -EACCES;
>  		break;
>  	default:
> +		/* Special handling for ntfs_read_mft() case. */
> +		if (inode->i_sb->s_magic == 0x7366746e) {
> +			if ((acc_mode & MAY_EXEC) && path_noexec(path))
> +				return -EACCES;

That's really unacceptable. We either need to drop the assert which
would be a shame because it keeps finding bugs or we fix that in ntfs3
which is my preferred solution...

> +			break;
> +		}
>  		VFS_BUG_ON_INODE(!IS_ANON_FILE(inode), inode);
>  	}
>  
> -- 
> 2.50.1
Re: [RFC PATCH] vfs: exclude ntfs3 from file mode validation in may_open()
Posted by Tetsuo Handa 1 month ago 
Konstantin, can you answer?

On 2025/08/11 22:50, Christian Brauner wrote:
>> Is it possible to handle this problem on the NTFS3 side?
> 
> Ugh, this is annoying.
> @Konstantin, why do you leave a zero mode for these files?
> Let's just make them regular files?
> 
>>
>>   --- a/fs/ntfs3/inode.c
>>   +++ b/fs/ntfs3/inode.c
>>   @@ -470,8 +470,9 @@ static struct inode *ntfs_read_mft(struct inode *inode,
>>           } else if (fname && fname->home.low == cpu_to_le32(MFT_REC_EXTEND) &&
>>                      fname->home.seq == cpu_to_le16(MFT_REC_EXTEND)) {
>>                   /* Records in $Extend are not a files or general directories. */
>>                   inode->i_op = &ntfs_file_inode_operations;
>>   +               mode = S_IFREG;
>>           } else {
>>                   err = -EINVAL;
>>                   goto out;
>>           }
>>
>> I don't know what breaks if we pretend as if S_IFREG...
[PATCH] ntfs3: pretend $Extend records as regular files
Posted by Tetsuo Handa 1 month ago 
Since commit af153bb63a33 ("vfs: catch invalid modes in may_open()")
requires any inode be one of S_IFDIR/S_IFLNK/S_IFREG/S_IFCHR/S_IFBLK/
S_IFIFO/S_IFSOCK type, use S_IFREG for $Extend records.

Reported-by: syzbot <syzbot+895c23f6917da440ed0d@syzkaller.appspotmail.com>
Closes: https://syzkaller.appspot.com/bug?extid=895c23f6917da440ed0d
Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
---
 fs/ntfs3/inode.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/fs/ntfs3/inode.c b/fs/ntfs3/inode.c
index 37cbbee7fa58..b08b00912165 100644
--- a/fs/ntfs3/inode.c
+++ b/fs/ntfs3/inode.c
@@ -471,6 +471,7 @@ static struct inode *ntfs_read_mft(struct inode *inode,
 		   fname->home.seq == cpu_to_le16(MFT_REC_EXTEND)) {
 		/* Records in $Extend are not a files or general directories. */
 		inode->i_op = &ntfs_file_inode_operations;
+		mode = S_IFREG;
 	} else {
 		err = -EINVAL;
 		goto out;
-- 
2.51.0
Re: [RFC PATCH] vfs: exclude ntfs3 from file mode validation in may_open()
Posted by Al Viro 1 month, 3 weeks ago 
On Mon, Aug 11, 2025 at 04:05:51PM +0900, Tetsuo Handa wrote:
> Since ntfs_read_mft() not only accepts file modes which may_open() accepts
> but also accepts
> 
>   (fname && fname->home.low == cpu_to_le32(MFT_REC_EXTEND) &&
>    fname->home.seq == cpu_to_le16(MFT_REC_EXTEND)
> 
> case when the file mode is none of
> S_IFDIR/S_IFLNK/S_IFREG/S_IFCHR/S_IFBLK/S_IFIFO/S_IFSOCK, may_open() cannot
> unconditionally expect IS_ANON_FILE(inode) when the file mode is none of
> S_IFDIR/S_IFLNK/S_IFREG/S_IFCHR/S_IFBLK/S_IFIFO/S_IFSOCK.
> 
> Treat as if S_IFREG when the inode is for NTFS3 filesystem.
> 
> Reported-by: syzbot <syzbot+895c23f6917da440ed0d@syzkaller.appspotmail.com>
> Closes: https://syzkaller.appspot.com/bug?extid=895c23f6917da440ed0d
> Fixes: af153bb63a33 ("vfs: catch invalid modes in may_open()")
> Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
> ---
> Is it possible to handle this problem on the NTFS3 side?
> 
>   --- a/fs/ntfs3/inode.c
>   +++ b/fs/ntfs3/inode.c
>   @@ -470,8 +470,9 @@ static struct inode *ntfs_read_mft(struct inode *inode,
>           } else if (fname && fname->home.low == cpu_to_le32(MFT_REC_EXTEND) &&
>                      fname->home.seq == cpu_to_le16(MFT_REC_EXTEND)) {
>                   /* Records in $Extend are not a files or general directories. */
>                   inode->i_op = &ntfs_file_inode_operations;
>   +               mode = S_IFREG;
>           } else {
>                   err = -EINVAL;
>                   goto out;
>           }
> 
> I don't know what breaks if we pretend as if S_IFREG...
> 
>  fs/namei.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/fs/namei.c b/fs/namei.c
> index cd43ff89fbaa..a66599754394 100644
> --- a/fs/namei.c
> +++ b/fs/namei.c
> @@ -3471,6 +3471,12 @@ static int may_open(struct mnt_idmap *idmap, const struct path *path,
>  			return -EACCES;
>  		break;
>  	default:
> +		/* Special handling for ntfs_read_mft() case. */
> +		if (inode->i_sb->s_magic == 0x7366746e) {
> +			if ((acc_mode & MAY_EXEC) && path_noexec(path))
> +				return -EACCES;
> +			break;
> +		}
>  		VFS_BUG_ON_INODE(!IS_ANON_FILE(inode), inode);
>  	}
>  

Bravo, but that's several months late - would be lovely for an AFD posting.
In the current form the patch is obviously unacceptable; if we do that
kind of special-casing, the proper place is in register_filesystem() where
we clearly ought to compare fs->name with "ntfs" and return an error.

All jokes aside, this kind of stuff is a non-starter.  Blacklist it on
syzbot side or fix fs/ntfs3; VFS is *NOT* the place for this kind of
special-casing.

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.