On Tue, Aug 08, 2023 at 01:12:19PM -0400, Sweet Tea Dorminy wrote:
> This change actually saves and loads the extent contexts created and
> freed by the last change.
>
> Signed-off-by: Sweet Tea Dorminy <sweettea-kernel@xxxxxxxxxx>
> ---
> fs/btrfs/file-item.c | 7 +++
> fs/btrfs/fscrypt.c | 108 ++++++++++++++++++++++++++++++++++++++++++-
> fs/btrfs/fscrypt.h | 35 ++++++++++++++
> fs/btrfs/inode.c | 37 +++++++++++++--
> fs/btrfs/tree-log.c | 14 +++++-
> 5 files changed, 194 insertions(+), 7 deletions(-)
>
> diff --git a/fs/btrfs/file-item.c b/fs/btrfs/file-item.c
> index f83f7020ed89..880fb7810152 100644
> --- a/fs/btrfs/file-item.c
> +++ b/fs/btrfs/file-item.c
> @@ -1299,6 +1299,13 @@ void btrfs_extent_item_to_extent_map(struct btrfs_inode *inode,
>
> ctxsize = btrfs_file_extent_ctxsize_from_item(leaf, path);
> ASSERT(ctxsize == btrfs_file_extent_encryption_ctxsize(leaf, fi));
> +
> + if (ctxsize) {
> + unsigned long ptr = (unsigned long)fi->encryption_context;
> + int res = btrfs_fscrypt_load_extent_info(inode, leaf, ptr,
> + ctxsize, &em->fscrypt_info);
> + ASSERT(res == 0);
> + }
> } else if (type == BTRFS_FILE_EXTENT_INLINE) {
> em->block_start = EXTENT_MAP_INLINE;
> em->start = extent_start;
> diff --git a/fs/btrfs/fscrypt.c b/fs/btrfs/fscrypt.c
> index 5508cbc6bccb..7324375af0ac 100644
> --- a/fs/btrfs/fscrypt.c
> +++ b/fs/btrfs/fscrypt.c
> @@ -75,6 +75,65 @@ bool btrfs_fscrypt_match_name(struct fscrypt_name *fname,
> return !memcmp(digest, nokey_name->sha256, sizeof(digest));
> }
>
> +int btrfs_fscrypt_fill_extent_context(struct btrfs_inode *inode,
> + struct fscrypt_info *info,
> + u8 *context_buffer, size_t *context_len)
> +{
> + struct btrfs_fs_info *fs_info = inode->root->fs_info;
> + int ret;
> +
> + if (!IS_ENCRYPTED(&inode->vfs_inode))
> + return 0;
> +
> +
> + ret = fscrypt_set_extent_context(info, context_buffer + sizeof(u32),
> + FSCRYPT_SET_CONTEXT_MAX_SIZE);
> + if (ret < 0) {
> + btrfs_err(fs_info, "fscrypt context could not be saved");
> + return ret;
> + }
> +
> + /* the return value, if nonnegative, is the fscrypt context size */
> + ret += sizeof(u32);
> +
> + put_unaligned_le32(ret, context_buffer);
> +
> + *context_len = ret;
> + return 0;
> +}
> +
> +int btrfs_fscrypt_load_extent_info(struct btrfs_inode *inode,
> + struct extent_buffer *leaf,
> + unsigned long ptr,
> + u8 ctxsize,
> + struct fscrypt_info **info_ptr)
> +{
> + struct btrfs_fs_info *fs_info = inode->root->fs_info;
> + u8 context[BTRFS_FSCRYPT_EXTENT_CONTEXT_MAX_SIZE];
> + int res;
> + unsigned int nofs_flags;
> + u32 len;
> +
> + read_extent_buffer(leaf, context, ptr, ctxsize);
> +
> + nofs_flags = memalloc_nofs_save();
> + res = fscrypt_load_extent_info(&inode->vfs_inode,
> + context + sizeof(u32),
> + ctxsize - sizeof(u32), info_ptr);
> + memalloc_nofs_restore(nofs_flags);
> +
> + if (res)
> + btrfs_err(fs_info, "Unable to load fscrypt info: %d", res);
We are we not returning an error here? Seems like we could end up with garbage
in context and then we'd just turn the error into -EINVAL below.
> +
> + len = get_unaligned_le32(context);
> + if (len != ctxsize) {
> + res = -EINVAL;
> + btrfs_err(fs_info, "fscrypt info size mismatches");
> + }
> +
> + return res;
> +}
> +
> static int btrfs_fscrypt_get_context(struct inode *inode, void *ctx, size_t len)
> {
> struct btrfs_key key = {
> @@ -138,11 +197,14 @@ static int btrfs_fscrypt_set_context(struct inode *inode, const void *ctx,
>
> if (!trans)
> trans = btrfs_start_transaction(BTRFS_I(inode)->root, 1);
> - if (IS_ERR(trans))
> + if (IS_ERR(trans)) {
> + btrfs_free_path(path);
> return PTR_ERR(trans);
> + }
>
> ret = btrfs_search_slot(trans, BTRFS_I(inode)->root, &key, path, 0, 1);
> if (ret < 0) {
> + btrfs_free_path(path);
> btrfs_abort_transaction(trans, ret);
> return ret;
> }
> @@ -151,12 +213,13 @@ static int btrfs_fscrypt_set_context(struct inode *inode, const void *ctx,
> btrfs_release_path(path);
> ret = btrfs_insert_empty_item(trans, BTRFS_I(inode)->root, path, &key, len);
> if (ret) {
> + btrfs_free_path(path);
> btrfs_abort_transaction(trans, ret);
> return ret;
> }
> }
> -
> btrfs_fscrypt_update_context(path, ctx, len);
> + btrfs_free_path(path);
>
These are unrelated changes, but my earlier comment was about re-working this
function, so I assume this will go away once you implement the changes I asked
for. Thanks,
Josef
|