Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:18 as permitted sender) client-ip=2620:137:e000::1:18;
Date:   Wed, 23 Feb 2022 15:59:29 -0800
From:   Eric Biggers <ebiggers@kernel.org>
To:     Mimi Zohar <zohar@linux.ibm.com>
Cc:     linux-integrity@vger.kernel.org,
        Stefan Berger <stefanb@linux.ibm.com>,
        linux-fscrypt@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v5 3/8] fs-verity: define a function to return the
 integrity protected file digest
Message-ID: <YhbKYZcWxmi4auJU@sol.localdomain>
References: <20220211214310.119257-1-zohar@linux.ibm.com>
 <20220211214310.119257-4-zohar@linux.ibm.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20220211214310.119257-4-zohar@linux.ibm.com>
Precedence: bulk

On Fri, Feb 11, 2022 at 04:43:05PM -0500, Mimi Zohar wrote:
> +/**
> + * fsverity_get_digest() - get a verity file's digest
> + * @inode: inode to get digest of
> + * @digest: (out) pointer to the digest
> + * @alg: (out) pointer to the hash algorithm enumeration
> + *
> + * Return the file hash algorithm and digest of an fsverity protected file.
> + *
> + * Return: 0 on success, -errno on failure
> + */
> +int fsverity_get_digest(struct inode *inode,
> +			u8 digest[FS_VERITY_MAX_DIGEST_SIZE],
> +			enum hash_algo *alg)
> +{
> +	const struct fsverity_info *vi;
> +	const struct fsverity_hash_alg *hash_alg;
> +	int i;
> +
> +	vi = fsverity_get_info(inode);
> +	if (!vi)
> +		return -ENODATA; /* not a verity file */

Sorry for the slow reviews; I'm taking a look again now.  One question about
something I missed earlier: is the file guaranteed to have been opened before
this is called?  fsverity_get_info() only returns a non-NULL value if the file
has been opened at least once since the inode has been loaded into memory.  If
the inode has just been loaded into memory without being opened, for example due
to a call to stat(), then fsverity_get_info() will return NULL.

If the file is guaranteed to have been opened, then the code is fine, but the
comment for fsverity_get_digest() perhaps should be updated to mention this
assumption, given that it takes a struct inode rather than a struct file.

If the file is *not* guaranteed to have been opened, then it would be necessary
to make fsverity_get_digest() call ensure_verity_info() to set up the
fsverity_info.

- Eric