Re: [PATCH v3] btrfs: send: add support for fs-verity

[Eric Biggers <ebiggers@kernel.org>]

On Mon, Aug 01, 2022 at 11:54:40AM -0700, Boris Burkov wrote:
> +#ifdef CONFIG_FS_VERITY
> +static int send_verity(struct send_ctx *sctx, struct fs_path *path,
> +		       struct fsverity_descriptor *desc)
> +{
> +	int ret;
> +
> +	ret = begin_cmd(sctx, BTRFS_SEND_C_ENABLE_VERITY);
> +	if (ret < 0)
> +		goto out;
> +
> +	TLV_PUT_PATH(sctx, BTRFS_SEND_A_PATH, path);
> +	TLV_PUT_U8(sctx, BTRFS_SEND_A_VERITY_ALGORITHM, desc->hash_algorithm);
> +	TLV_PUT_U32(sctx, BTRFS_SEND_A_VERITY_BLOCK_SIZE, 1U << desc->log_blocksize);
> +	TLV_PUT(sctx, BTRFS_SEND_A_VERITY_SALT_DATA, desc->salt, desc->salt_size);
> +	TLV_PUT(sctx, BTRFS_SEND_A_VERITY_SIG_DATA, desc->signature, (int)desc->sig_size);

le32_to_cpu(desc->sig_size)

> +
> +	ret = send_cmd(sctx);
> +
> +tlv_put_failure:
> +out:
> +	return ret;
> +}

The 'out' label is unnecessary.

> +
> +static int process_new_verity(struct send_ctx *sctx)

What does "new verity" mean in this context?  The other functions called by
finish_inode_if_needed() have names like send_chown(), send_chmod(), etc., so
this name seems inconsistent (although I'm not familiar with this code).

> +
> +	ret = send_verity(sctx, p, sctx->verity_descriptor);
> +	if (ret < 0)
> +		goto free_path;
> +
> +free_path:
> +	fs_path_free(p);

The goto above is unnecessary.

> +static int process_new_verity(struct send_ctx *sctx)
> +{
> +	int ret = 0;
> +	struct send_ctx tmp;
> +
> +	return -EPERM;
> +	/* avoid unused TLV_PUT_U8 build warning without CONFIG_FS_VERITY */
> +	TLV_PUT_U8(&tmp, 0, 0);
> +tlv_put_failure:
> +	return -EPERM;
> +}
> +#endif

How about adding __maybe_unused to tlv_put_u##bits instead?

> @@ -8036,6 +8148,8 @@ long btrfs_ioctl_send(struct inode *inode, struct btrfs_ioctl_send_args *arg)
>  		kvfree(sctx->clone_roots);
>  		kfree(sctx->send_buf_pages);
>  		kvfree(sctx->send_buf);
> +		if (sctx->verity_descriptor)
> +			kvfree(sctx->verity_descriptor);

There's no need to check for NULL before calling kvfree().

- Eric