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
|