Hi Eric,
On 2018/8/27 1:04, Eric Biggers wrote:
> Hi Chuck,
>
> On Sun, Aug 26, 2018 at 11:55:57AM -0400, Chuck Lever wrote:
>>> +
>>> +/**
>>> + * fsverity_verify_page - verify a data page
>>> + *
>>> + * Verify a page that has just been read from a file against that file's Merkle
>>> + * tree. The page is assumed to be a pagecache page.
>>> + *
>>> + * Return: true if the page is valid, else false.
>>> + */
>>> +bool fsverity_verify_page(struct page *data_page)
>>> +{
>>> + struct inode *inode = data_page->mapping->host;
>>> + const struct fsverity_info *vi = get_fsverity_info(inode);
>>> + struct ahash_request *req;
>>> + bool valid;
>>> +
>>> + req = ahash_request_alloc(vi->hash_alg->tfm, GFP_KERNEL);
Some minor suggestions occurred to me after I saw this part of code
again before sleeping...
1) How about introducing an iterator callback to avoid too many
ahash_request_alloc and ahash_request_free... (It seems too many pages
and could be some slower than fsverity_verify_bio...)
2) How about adding a gfp_t input argument since I don't know whether
GFP_KERNEL is suitable for all use cases...
It seems there could be more fsverity_verify_page users as well as
fsverity_verify_bio ;)
Sorry for interruption...
Thanks,
Gao Xiang
>>> + if (unlikely(!req))
>>> + return false;
>>> +
>>> + valid = verify_page(inode, vi, req, data_page);
>>> +
>>> + ahash_request_free(req);
>>> +
>>> + return valid;
>>> +}
>>> +EXPORT_SYMBOL_GPL(fsverity_verify_page);
>>> +
>>> +/**
>>> + * fsverity_verify_bio - verify a 'read' bio that has just completed
>>> + *
>>> + * Verify a set of pages that have just been read from a file against that
>>> + * file's Merkle tree. The pages are assumed to be pagecache pages. Pages that
>>> + * fail verification are set to the Error state. Verification is skipped for
>>> + * pages already in the Error state, e.g. due to fscrypt decryption failure.
>>> + */
>>> +void fsverity_verify_bio(struct bio *bio)
>>
>> Hi Eric-
>>
>> This kind of API won't work for remote filesystems, which do not use
>> "struct bio" to do their I/O. Could a remote filesystem solely use
>> fsverity_verify_page instead?
>>
>
> Yes, filesystems don't have to use fsverity_verify_bio(). They can call
> fsverity_verify_page() on each page instead. I will clarify this in the next
> revision of the patchset.
>
> - Eric
>
