> + const u64 end_pos = min(pos + length, vi->tree_params.tree_size);
> + struct backing_dev_info *bdi = inode->i_sb->s_bdi;
> + const u64 max_ra_bytes = min((u64)bdi->io_pages << PAGE_SHIFT,
> + ULONG_MAX);
> + const struct merkle_tree_params *params = &vi->tree_params;
bdi->io_pages is really a VM readahead concept. I know this is existing
code, but can we rething why this is even used here?
> + unsigned int offs_in_block = pos & (params->block_size - 1);
> int retval = 0;
> int err = 0;
>
> + * Iterate through each Merkle tree block in the requested range and
> + * copy the requested portion to userspace. Note that we are returning
> + * a byte stream.
> */
> + while (pos < end_pos) {
> + unsigned long ra_bytes;
> + unsigned int bytes_to_copy;
> + struct fsverity_blockbuf block = { };
>
> + ra_bytes = min_t(unsigned long, end_pos - pos, max_ra_bytes);
> + bytes_to_copy = min_t(u64, end_pos - pos,
> + params->block_size - offs_in_block);
> +
> + err = fsverity_read_merkle_tree_block(inode, &vi->tree_params,
> + pos - offs_in_block,
> + ra_bytes, &block);
Maybe it's just me, but isn't passing a byte offset to a read...block
routine a bit weird and this should operate on the block number instead?
> + if (copy_to_user(buf, block.kaddr + offs_in_block, bytes_to_copy)) {
And the returned/passed value should be a kernel pointer to the start
of the in-memory copy of the block?
to
> +static bool is_hash_block_verified(struct inode *inode,
> + struct fsverity_blockbuf *block,
> unsigned long hblock_idx)
Other fsverify code seems to use the (IMHO) much more readable
two-tab indentation for prototype continuations, maybe stick to that?
>
> {
> + struct fsverity_info *vi = inode->i_verity_info;
> + struct page *hpage = (struct page *)block->context;
block->context is a void pointer, no need for casting it.
> + for (; level > 0; level--)
> + fsverity_drop_merkle_tree_block(inode, &hblocks[level - 1].block);
Overlh long line here. But the loop kinda looks odd anyway with the
exta one off in the body instead of the loop.
