linux-ext4 - Re: [PATCH V3 8/9] fsverity: Move verity status check to fsverity_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]

Date:   Mon, 10 Dec 2018 09:26:11 -0800
From:   Eric Biggers <ebiggers@...nel.org>
To:     Chandan Rajendra <chandan@...ux.vnet.ibm.com>
Cc:     linux-ext4@...r.kernel.org, linux-f2fs-devel@...ts.sourceforge.net,
        linux-doc@...r.kernel.org, linux-mips@...ux-mips.org,
        linux-s390@...r.kernel.org, linux-mtd@...ts.infradead.org,
        linux-fsdevel@...r.kernel.org, tytso@....edu,
        adilger.kernel@...ger.ca, jaegeuk@...nel.org, yuchao0@...wei.com,
        corbet@....net, ralf@...ux-mips.org, paul.burton@...s.com,
        jhogan@...nel.org, green.hu@...il.com, deanbo422@...il.com,
        schwidefsky@...ibm.com, heiko.carstens@...ibm.com, richard@....at,
        dedekind1@...il.com, adrian.hunter@...el.com,
        viro@...iv.linux.org.uk
Subject: Re: [PATCH V3 8/9] fsverity: Move verity status check to
 fsverity_file_open

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@...ux.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