linux-ext4 - Re: [PATCH 3/6] fs,fsverity: handle fsverity in generic_file

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20260119-davon-krippenkind-78d683621491@brauner>
Date: Mon, 19 Jan 2026 11:02:37 +0100
From: Christian Brauner <brauner@...nel.org>
To: Christoph Hellwig <hch@....de>
Cc: Eric Biggers <ebiggers@...nel.org>, Al Viro <viro@...iv.linux.org.uk>, 
	Jan Kara <jack@...e.cz>, David Sterba <dsterba@...e.com>, Theodore Ts'o <tytso@....edu>, 
	Jaegeuk Kim <jaegeuk@...nel.org>, Chao Yu <chao@...nel.org>, 
	Andrey Albershteyn <aalbersh@...hat.com>, linux-fsdevel@...r.kernel.org, linux-btrfs@...r.kernel.org, 
	linux-ext4@...r.kernel.org, linux-f2fs-devel@...ts.sourceforge.net, fsverity@...ts.linux.dev
Subject: Re: [PATCH 3/6] fs,fsverity: handle fsverity in generic_file_open

On Mon, Jan 19, 2026 at 07:22:44AM +0100, Christoph Hellwig wrote:
> Call into fsverity_file_open from generic_file_open instead of requiring
> the file system to handle it explicitly.
> 
> Signed-off-by: Christoph Hellwig <hch@....de>
> ---
>  fs/btrfs/file.c          |  6 ------
>  fs/ext4/file.c           |  4 ----
>  fs/f2fs/file.c           |  4 ----
>  fs/open.c                |  8 +++++++-
>  fs/verity/open.c         | 10 ++++++++--
>  include/linux/fsverity.h | 32 +-------------------------------
>  6 files changed, 16 insertions(+), 48 deletions(-)
> 
> diff --git a/fs/btrfs/file.c b/fs/btrfs/file.c
> index 1abc7ed2990e..4b3a31b2b52e 100644
> --- a/fs/btrfs/file.c
> +++ b/fs/btrfs/file.c
> @@ -3808,16 +3808,10 @@ static loff_t btrfs_file_llseek(struct file *file, loff_t offset, int whence)
>  
>  static int btrfs_file_open(struct inode *inode, struct file *filp)
>  {
> -	int ret;
> -
>  	if (unlikely(btrfs_is_shutdown(inode_to_fs_info(inode))))
>  		return -EIO;
>  
>  	filp->f_mode |= FMODE_NOWAIT | FMODE_CAN_ODIRECT;
> -
> -	ret = fsverity_file_open(inode, filp);
> -	if (ret)
> -		return ret;
>  	return generic_file_open(inode, filp);
>  }
>  
> diff --git a/fs/ext4/file.c b/fs/ext4/file.c
> index 7a8b30932189..a7dc8c10273e 100644
> --- a/fs/ext4/file.c
> +++ b/fs/ext4/file.c
> @@ -906,10 +906,6 @@ static int ext4_file_open(struct inode *inode, struct file *filp)
>  	if (ret)
>  		return ret;
>  
> -	ret = fsverity_file_open(inode, filp);
> -	if (ret)
> -		return ret;
> -
>  	/*
>  	 * Set up the jbd2_inode if we are opening the inode for
>  	 * writing and the journal is present
> diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
> index da029fed4e5a..f1510ab657b6 100644
> --- a/fs/f2fs/file.c
> +++ b/fs/f2fs/file.c
> @@ -624,10 +624,6 @@ static int f2fs_file_open(struct inode *inode, struct file *filp)
>  	if (!f2fs_is_compress_backend_ready(inode))
>  		return -EOPNOTSUPP;
>  
> -	err = fsverity_file_open(inode, filp);
> -	if (err)
> -		return err;
> -
>  	filp->f_mode |= FMODE_NOWAIT;
>  	filp->f_mode |= FMODE_CAN_ODIRECT;
>  
> diff --git a/fs/open.c b/fs/open.c
> index f328622061c5..dea93bab8795 100644
> --- a/fs/open.c
> +++ b/fs/open.c
> @@ -10,6 +10,7 @@
>  #include <linux/file.h>
>  #include <linux/fdtable.h>
>  #include <linux/fsnotify.h>
> +#include <linux/fsverity.h>
>  #include <linux/module.h>
>  #include <linux/tty.h>
>  #include <linux/namei.h>
> @@ -1604,10 +1605,15 @@ SYSCALL_DEFINE0(vhangup)
>   * the caller didn't specify O_LARGEFILE.  On 64bit systems we force
>   * on this flag in sys_open.
>   */
> -int generic_file_open(struct inode * inode, struct file * filp)
> +int generic_file_open(struct inode *inode, struct file *filp)
>  {
>  	if (!(filp->f_flags & O_LARGEFILE) && i_size_read(inode) > MAX_NON_LFS)
>  		return -EOVERFLOW;
> +	if (IS_ENABLED(CONFIG_FS_VERITY) && IS_VERITY(inode)) {
> +		if (filp->f_mode & FMODE_WRITE)
> +			return -EPERM;
> +		return fsverity_file_open(inode, filp);
> +	}

This is the only one where I'm not happy about the location.
This hides the ordering requirement between fsverity and fscrypt. It's
easier to miss now. This also really saves very little compared to the
other changes. So I wonder whether it's really that big of a deal to
have the call located in the open routines of the filesystems.