On Thu, Jul 28, 2022 at 01:43:32PM +0200, David Sterba wrote:
> On Wed, Jul 27, 2022 at 04:46:22PM -0700, Boris Burkov wrote:
> > Preserve the fs-verity status of a btrfs file across send/recv.
> >
> > There is no facility for installing the Merkle tree contents directly on
> > the receiving filesystem, so we package up the parameters used to enable
> > verity found in the verity descriptor. This gives the receive side
> > enough information to properly enable verity again. Note that this means
> > that receive will have to re-compute the whole Merkle tree, similar to
> > how compression worked before encoded_write.
> >
> > Since the file becomes read-only after verity is enabled, it is
> > important that verity is added to the send stream after any file writes.
> > Therefore, when we process a verity item, merely note that it happened,
> > then actually create the command in the send stream during
> > 'finish_inode_if_needed'.
> >
> > This also creates V3 of the send stream format, without any format
> > changes besides adding the new commands and attributes.
> >
> > Signed-off-by: Boris Burkov <boris@xxxxxx>
> > ---
> > @@ -4886,6 +4889,80 @@ static int process_all_new_xattrs(struct send_ctx *sctx)
> > return ret;
> > }
> >
> > +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, 1 << desc->log_blocksize);
>
> bitshifts should be done on unsigned types
>
> 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, desc->sig_size);
> > +
> > + ret = send_cmd(sctx);
> > +
> > +tlv_put_failure:
> > +out:
> > + return ret;
> > +}
> > +
> > +static int process_new_verity(struct send_ctx *sctx)
> > +{
> > + int ret = 0;
> > + struct btrfs_fs_info *fs_info = sctx->send_root->fs_info;
> > + struct inode *inode;
> > + struct fsverity_descriptor *desc;
> > + struct fs_path *p;
> > +
> > + inode = btrfs_iget(fs_info->sb, sctx->cur_ino, sctx->send_root);
> > + if (IS_ERR(inode))
> > + return PTR_ERR(inode);
> > +
> > + ret = fs_info->sb->s_vop->get_verity_descriptor(inode, NULL, 0);
> > + if (ret < 0)
> > + goto iput;
> > +
> > + if (ret > FS_VERITY_MAX_DESCRIPTOR_SIZE) {
> > + ret = -EMSGSIZE;
> > + goto iput;
> > + }
> > + desc = kmalloc(ret, GFP_KERNEL);
>
> I think that once there's a file with verity record there will be more
> so it would make sense to cache the allocated memory, which means to
> allocate the full size.
>
> FS_VERITY_MAX_DESCRIPTOR_SIZE is 16K so this should be allocated by
> kvmalloc, like we have for other data during send.
>
> > + if (!desc) {
> > + ret = -ENOMEM;
> > + goto iput;
> > + }
> > +
> > + ret = fs_info->sb->s_vop->get_verity_descriptor(inode, desc, ret);
> > + if (ret < 0)
> > + goto free_desc;
> > +
> > + p = fs_path_alloc();
> > + if (!p) {
> > + ret = -ENOMEM;
> > + goto free_desc;
> > + }
> > + ret = get_cur_path(sctx, sctx->cur_ino, sctx->cur_inode_gen, p);
> > + if (ret < 0)
> > + goto free_path;
> > +
> > + ret = send_verity(sctx, p, desc);
> > + if (ret < 0)
> > + goto free_path;
> > +
> > +free_path:
> > + fs_path_free(p);
> > +free_desc:
> > + kfree(desc);
> > +iput:
> > + iput(inode);
> > + return ret;
> > +}
> > +
> > static inline u64 max_send_read_size(const struct send_ctx *sctx)
> > {
> > return sctx->send_max_size - SZ_16K;
> > --- a/fs/btrfs/send.h
> > +++ b/fs/btrfs/send.h
> > @@ -92,8 +92,11 @@ enum btrfs_send_cmd {
> > BTRFS_SEND_C_ENCODED_WRITE = 25,
> > BTRFS_SEND_C_MAX_V2 = 25,
> >
> > + /* Version 3 */
> > + BTRFS_SEND_C_ENABLE_VERITY = 26,
>
> I find the name confusing, it sounds like enabling it for the whole
> filesystem, but it affects only one file.
Your point about fs-wide vs file specific makes sense, but on the other
hand, that is the name of the ioctl and the cli command.
Would you prefer VERITY_ENABLE? or just VERITY? I am having trouble
thinking of a better name, but I also don't mind if you think of one
and rename it.
>
> > + BTRFS_SEND_C_MAX_V3 = 26,
> > /* End */
> > - BTRFS_SEND_C_MAX = 25,
> > + BTRFS_SEND_C_MAX = 26,
> > };
> >
> > /* attributes in send stream */
> > @@ -160,8 +163,14 @@ enum {
> > BTRFS_SEND_A_ENCRYPTION = 31,
> > BTRFS_SEND_A_MAX_V2 = 31,
> >
> > - /* End */
> > - BTRFS_SEND_A_MAX = 31,
> > + /* Version 3 */
> > + BTRFS_SEND_A_VERITY_ALGORITHM = 32,
> > + BTRFS_SEND_A_VERITY_BLOCK_SIZE = 33,
> > + BTRFS_SEND_A_VERITY_SALT_DATA = 34,
> > + BTRFS_SEND_A_VERITY_SIG_DATA = 35,
> > + BTRFS_SEND_A_MAX_V3 = 35,
> > +
> > + __BTRFS_SEND_A_MAX = 35,
> > };
|