Re: [PATCH V3 8/9] fsverity: Move verity status check to fsverity_file_open

[Eric Biggers <ebiggers@kernel.org>]

Hi Chandan,

On Sat, Dec 08, 2018 at 12:21:43PM +0530, Chandan Rajendra wrote:
> Instead of conditionally checking for verity status of an inode before
> invoking fsverity_file_open(), this commit moves the check inside the
> definition of fsverity_file_open().
> 
> Signed-off-by: Chandan Rajendra <chandan@linux.vnet.ibm.com>
> ---
>  fs/ext4/file.c    | 8 +++-----
>  fs/f2fs/file.c    | 8 +++-----
>  fs/verity/setup.c | 3 +++
>  3 files changed, 9 insertions(+), 10 deletions(-)
> 
> diff --git a/fs/ext4/file.c b/fs/ext4/file.c
> index 30fbd663354f..b404a857cd48 100644
> --- a/fs/ext4/file.c
> +++ b/fs/ext4/file.c
> @@ -444,11 +444,9 @@ static int ext4_file_open(struct inode * inode, struct file * filp)
>  	if (ret)
>  		return ret;
>  
> -	if (IS_VERITY(inode)) {
> -		ret = fsverity_file_open(inode, 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
> diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
> index 2eb4821d95d1..925c0d9608da 100644
> --- a/fs/f2fs/file.c
> +++ b/fs/f2fs/file.c
> @@ -491,11 +491,9 @@ static int f2fs_file_open(struct inode *inode, struct file *filp)
>  	if (err)
>  		return err;
>  
> -	if (IS_VERITY(inode)) {
> -		err = fsverity_file_open(inode, filp);
> -		if (err)
> -			return err;
> -	}
> +	err = fsverity_file_open(inode, filp);
> +	if (err)
> +		return err;
>  
>  	filp->f_mode |= FMODE_NOWAIT;
>  
> diff --git a/fs/verity/setup.c b/fs/verity/setup.c
> index 08b609127531..bc463dc601b1 100644
> --- a/fs/verity/setup.c
> +++ b/fs/verity/setup.c
> @@ -771,6 +771,9 @@ static int setup_fsverity_info(struct inode *inode)
>   */
>  int fsverity_file_open(struct inode *inode, struct file *filp)
>  {
> +	if (!IS_VERITY(inode))
> +		return 0;
> +
>  	if (filp->f_mode & FMODE_WRITE) {
>  		pr_debug("Denying opening verity file (ino %lu) for write\n",
>  			 inode->i_ino);
> -- 
> 2.19.1
> 

This will break ext4 and f2fs when !CONFIG_FS_VERITY because then:

static inline int fsverity_file_open(struct inode *inode, struct file *filp)
{
	return -EOPNOTSUPP;
}

Can you please make it like fscrypt_file_open()?

static inline int fsverity_file_open(struct inode *inode, struct file *filp)
{
	if (IS_VERITY(inode))
		return -EOPNOTSUPP;
	return 0;
}

Same with fsverity_prepare_setattr().

- Eric