NTFS3 file system kernel mode driver
 help / color / mirror / Atom feed
From: Al Viro <viro@zeniv.linux.org.uk>
To: Tetsuo Handa <penguin-kernel@i-love.sakura.ne.jp>
Cc: Christian Brauner <brauner@kernel.org>, Jan Kara <jack@suse.cz>,
	Mateusz Guzik <mjguzik@gmail.com>,
	Konstantin Komarov <almaz.alexandrovich@paragon-software.com>,
	linux-fsdevel <linux-fsdevel@vger.kernel.org>,
	ntfs3@lists.linux.dev, LKML <linux-kernel@vger.kernel.org>
Subject: Re: [RFC PATCH] vfs: exclude ntfs3 from file mode validation in may_open()
Date: Mon, 11 Aug 2025 14:38:48 +0100	[thread overview]
Message-ID: <20250811133848.GR222315@ZenIV> (raw)
In-Reply-To: <fb3888fb-11b8-481b-86a6-766bbbab5c81@I-love.SAKURA.ne.jp>

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.

  reply	other threads:[~2025-08-11 13:38 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-08-11  7:05 [RFC PATCH] vfs: exclude ntfs3 from file mode validation in may_open() Tetsuo Handa
2025-08-11 13:38 ` Al Viro [this message]
2025-08-11 13:50 ` Christian Brauner
2025-08-29 13:37   ` Tetsuo Handa
2025-09-02 10:43     ` [PATCH] ntfs3: pretend $Extend records as regular files Tetsuo Handa

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20250811133848.GR222315@ZenIV \
    --to=viro@zeniv.linux.org.uk \
    --cc=almaz.alexandrovich@paragon-software.com \
    --cc=brauner@kernel.org \
    --cc=jack@suse.cz \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mjguzik@gmail.com \
    --cc=ntfs3@lists.linux.dev \
    --cc=penguin-kernel@i-love.sakura.ne.jp \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox