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;
Message-ID: <c6d3fcf09a56ad3b3fcc011b7fd3b8207675ad8e.camel@linux.ibm.com>
Subject: Re: [PATCH v5 3/8] fs-verity: define a function to return the
 integrity protected file digest
From:   Mimi Zohar <zohar@linux.ibm.com>
To:     Eric Biggers <ebiggers@kernel.org>
Cc:     linux-integrity@vger.kernel.org,
        Stefan Berger <stefanb@linux.ibm.com>,
        linux-fscrypt@vger.kernel.org, linux-kernel@vger.kernel.org
Date:   Wed, 23 Feb 2022 20:27:49 -0500
In-Reply-To: <YhbeuQpGuDxEmi9o@sol.localdomain>
References: <20220211214310.119257-1-zohar@linux.ibm.com>
         <20220211214310.119257-4-zohar@linux.ibm.com>
         <YhbKYZcWxmi4auJU@sol.localdomain>
         <f322ae351dde71b92d7d4037d78190c7338ca710.camel@linux.ibm.com>
         <YhbeuQpGuDxEmi9o@sol.localdomain>
Content-Type: text/plain; charset="ISO-8859-15"
Mime-Version: 1.0
Content-Transfer-Encoding: 7bit
Precedence: bulk

On Wed, 2022-02-23 at 17:26 -0800, Eric Biggers wrote:
> On Wed, Feb 23, 2022 at 08:21:01PM -0500, Mimi Zohar wrote:
> > On Wed, 2022-02-23 at 15:59 -0800, Eric Biggers wrote:
> > > 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.
> > 
> > Yes, fsverity_get_digest() is called as a result of a syscall - open,
> > execve, mmap, etc.   
> > Refer to the LSM hooks security_bprm_check() and security_mmap_file().
> > ima_file_check() is called directly in do_open().
> 
> stat() is a syscall too, so the question is not whether this is being called as
> a result of a syscall, but rather whether it's only being called while the file
> is open (or at least previously opened).  Is the answer to that "yes"?

yes