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