From: Josef Bacik <josef@toxicpanda.com>
To: Sweet Tea Dorminy <sweettea-kernel@dorminy.me>
Cc: "Theodore Y. Ts'o" <tytso@mit.edu>,
Jaegeuk Kim <jaegeuk@kernel.org>,
Eric Biggers <ebiggers@kernel.org>, Chris Mason <clm@fb.com>,
David Sterba <dsterba@suse.com>,
linux-fscrypt@vger.kernel.org, linux-btrfs@vger.kernel.org,
kernel-team@meta.com
Subject: Re: [PATCH v2 17/17] btrfs: save and load fscrypt extent contexts
Date: Mon, 17 Jul 2023 14:15:25 -0400 [thread overview]
Message-ID: <20230717181525.GQ691303@perftesting> (raw)
In-Reply-To: <d3849d039673b6583291c29c5d36140357e1f1dc.1689564024.git.sweettea-kernel@dorminy.me>
On Sun, Jul 16, 2023 at 11:52:48PM -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@dorminy.me>
> ---
> fs/btrfs/file-item.c | 21 ++++++++++++++++
> fs/btrfs/fscrypt.c | 36 +++++++++++++++++++++++++++
> fs/btrfs/fscrypt.h | 6 +++++
> fs/btrfs/inode.c | 44 ++++++++++++++++++++++++++++++---
> fs/btrfs/tree-log.c | 24 ++++++++++++++++--
> include/uapi/linux/btrfs_tree.h | 5 ++++
> 6 files changed, 130 insertions(+), 6 deletions(-)
>
> diff --git a/fs/btrfs/file-item.c b/fs/btrfs/file-item.c
> index 8095fc2e7ca1..ccc2d12faba3 100644
> --- a/fs/btrfs/file-item.c
> +++ b/fs/btrfs/file-item.c
> @@ -1302,6 +1302,27 @@ 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));
> +
> +#ifdef CONFIG_FS_ENCRYPTION
> + if (ctxsize) {
> + u8 context[FSCRYPT_SET_CONTEXT_MAX_SIZE];
> + int res;
> + unsigned int nofs_flag;
> +
> + read_extent_buffer(leaf, context,
> + (unsigned long)fi->fscrypt_context,
> + ctxsize);
> + nofs_flag = memalloc_nofs_save();
> + res = fscrypt_load_extent_info(&inode->vfs_inode,
> + context, ctxsize,
> + &em->fscrypt_info);
> + memalloc_nofs_restore(nofs_flag);
> + if (res)
> + btrfs_err(fs_info,
> + "Unable to load fscrypt info: %d",
> + res);
> + }
> +#endif /* CONFIG_FS_ENCRYPTION */
> } 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 6875108f4363..30dab7d06589 100644
> --- a/fs/btrfs/fscrypt.c
> +++ b/fs/btrfs/fscrypt.c
> @@ -166,6 +166,41 @@ static bool btrfs_fscrypt_empty_dir(struct inode *inode)
> return inode->i_size == BTRFS_EMPTY_DIR_SIZE;
> }
>
> +int btrfs_fscrypt_get_extent_info(const struct inode *inode,
> + u64 lblk_num,
> + struct fscrypt_info **info_ptr,
> + u64 *extent_offset,
> + u64 *extent_length)
> +{
> + u64 offset = lblk_num << inode->i_blkbits;
> + struct extent_map *em;
> +
> + /* Since IO must be in progress on this extent, this must succeed */
> + em = btrfs_get_extent(BTRFS_I(inode), NULL, 0, offset, PAGE_SIZE);
> + if (!em)
> + return -EINVAL;
> +
> + if (em->block_start == EXTENT_MAP_HOLE) {
> + btrfs_info(BTRFS_I(inode)->root->fs_info,
> + "extent context requested for block %llu of inode %lu without an extent",
> + lblk_num, inode->i_ino);
> + free_extent_map(em);
> + return -ENOENT;
> + }
> +
> + *info_ptr = em->fscrypt_info;
> +
> + if (extent_offset)
> + *extent_offset
> + = (offset - em->start) >> inode->i_blkbits;
> +
> + if (extent_length)
> + *extent_length = em->len >> inode->i_blkbits;
> +
> + free_extent_map(em);
> + return 0;
> +}
> +
> static struct block_device **btrfs_fscrypt_get_devices(struct super_block *sb,
> unsigned int *num_devs)
> {
> @@ -206,6 +241,7 @@ const struct fscrypt_operations btrfs_fscrypt_ops = {
> .get_context = btrfs_fscrypt_get_context,
> .set_context = btrfs_fscrypt_set_context,
> .empty_dir = btrfs_fscrypt_empty_dir,
> + .get_extent_info = btrfs_fscrypt_get_extent_info,
> .get_devices = btrfs_fscrypt_get_devices,
> .key_prefix = "btrfs:"
> };
> diff --git a/fs/btrfs/fscrypt.h b/fs/btrfs/fscrypt.h
> index 2d405d54cbc7..1cab721a64e5 100644
> --- a/fs/btrfs/fscrypt.h
> +++ b/fs/btrfs/fscrypt.h
> @@ -50,6 +50,12 @@ static inline bool btrfs_fscrypt_match_name(struct fscrypt_name *fname,
> }
> #endif /* CONFIG_FS_ENCRYPTION */
>
> +int btrfs_fscrypt_get_extent_info(const struct inode *inode,
> + u64 lblk_num,
> + struct fscrypt_info **info_ptr,
> + u64 *extent_offset,
> + u64 *extent_length);
> +
> extern const struct fscrypt_operations btrfs_fscrypt_ops;
>
> #endif /* BTRFS_FSCRYPT_H */
> diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
> index 83098779dad2..92a193785a21 100644
> --- a/fs/btrfs/inode.c
> +++ b/fs/btrfs/inode.c
> @@ -3036,17 +3036,46 @@ static int insert_reserved_file_extent(struct btrfs_trans_handle *trans,
> u64 num_bytes = btrfs_stack_file_extent_num_bytes(stack_fi);
> u64 ram_bytes = btrfs_stack_file_extent_ram_bytes(stack_fi);
> struct btrfs_drop_extents_args drop_args = { 0 };
> - size_t fscrypt_context_size =
> - btrfs_stack_file_extent_encryption(stack_fi) ?
> - FSCRYPT_SET_CONTEXT_MAX_SIZE : 0;
> + size_t fscrypt_context_size = 0;
> +#ifdef CONFIG_FS_ENCRYPTION
> + u8 context[FSCRYPT_SET_CONTEXT_MAX_SIZE];
> +#endif /* CONFIG_FS_ENCRYPTION */
> +
> int ret;
>
> path = btrfs_alloc_path();
> if (!path)
> return -ENOMEM;
>
> +#ifdef CONFIG_FS_ENCRYPTION
> + if (IS_ENCRYPTED(&inode->vfs_inode)) {
> + u8 encryption;
> + struct fscrypt_info *fscrypt_info;
> + u64 lblk_num = file_pos >> root->fs_info->sectorsize_bits;
> +
> + ret = btrfs_fscrypt_get_extent_info(&inode->vfs_inode,
> + lblk_num, &fscrypt_info,
> + NULL, NULL);
> + if (ret) {
> + btrfs_err(root->fs_info, "No fscrypt context found");
> + goto out;
> + }
> +
> + fscrypt_context_size =
> + fscrypt_set_extent_context(fscrypt_info, context,
> + FSCRYPT_SET_CONTEXT_MAX_SIZE);
> + if (fscrypt_context_size < 0) {
> + ret = fscrypt_context_size;
> + goto out;
> + }
> + encryption = btrfs_pack_encryption(BTRFS_ENCRYPTION_FSCRYPT,
> + fscrypt_context_size);
> + btrfs_set_stack_file_extent_encryption(stack_fi, encryption);
> + }
> +#endif /* CONFIG_FS_ENCRYPTION */
Make this into a helper so we're not cluttering the normal code path with the
ifdefs.
> +
> /*
> - * we may be replacing one extent in the tree with another.
> + * We may be replacing one extent in the tree with another.
> * The new extent is pinned in the extent map, and we don't want
> * to drop it from the cache until it is completely in the btree.
> *
> @@ -3079,6 +3108,13 @@ static int insert_reserved_file_extent(struct btrfs_trans_handle *trans,
> btrfs_item_ptr_offset(leaf, path->slots[0]),
> sizeof(struct btrfs_file_extent_item));
>
> +#ifdef CONFIG_FS_ENCRYPTION
> + write_extent_buffer(leaf, context,
> + btrfs_item_ptr_offset(leaf, path->slots[0]) +
> + sizeof(struct btrfs_file_extent_item),
> + fscrypt_context_size);
> +#endif /* CONFIG_FS_ENCRYPTION */
> +
> btrfs_mark_buffer_dirty(leaf);
> btrfs_release_path(path);
>
> diff --git a/fs/btrfs/tree-log.c b/fs/btrfs/tree-log.c
> index 82c91097672b..f0ad281170c5 100644
> --- a/fs/btrfs/tree-log.c
> +++ b/fs/btrfs/tree-log.c
> @@ -4634,8 +4634,22 @@ static int log_one_extent(struct btrfs_trans_handle *trans,
> u64 extent_offset = em->start - em->orig_start;
> u64 block_len;
> int ret;
> - u8 encryption = btrfs_pack_encryption(IS_ENCRYPTED(&inode->vfs_inode) ?
> - BTRFS_ENCRYPTION_FSCRYPT : 0, 0);
> + u8 encryption = 0;
> + size_t fscrypt_context_size = 0;
> +#ifdef CONFIG_FS_ENCRYPTION
> + u8 context[FSCRYPT_SET_CONTEXT_MAX_SIZE];
> +
> + if (em->fscrypt_info) {
> + fscrypt_context_size =
> + fscrypt_set_extent_context(em->fscrypt_info, context,
> + FSCRYPT_SET_CONTEXT_MAX_SIZE);
> + if (fscrypt_context_size < 0)
> + return fscrypt_context_size;
> +
> + encryption = btrfs_pack_encryption(BTRFS_ENCRYPTION_FSCRYPT,
> + fscrypt_context_size);
> + }
Same here, looks like this can be it's own helper, and the code above can use
this helper and do the other thing it needs to. Thanks,
Josef
prev parent reply other threads:[~2023-07-17 18:16 UTC|newest]
Thread overview: 33+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-07-17 3:52 [PATCH v2 00/17] btrfs: add encryption feature Sweet Tea Dorminy
2023-07-17 3:52 ` [PATCH v2 01/17] btrfs: disable various operations on encrypted inodes Sweet Tea Dorminy
2023-07-17 3:52 ` [PATCH v2 02/17] btrfs: disable verity " Sweet Tea Dorminy
2023-07-17 3:52 ` [PATCH v2 03/17] fscrypt: expose fscrypt_nokey_name Sweet Tea Dorminy
2023-07-17 3:52 ` [PATCH v2 04/17] btrfs: start using fscrypt hooks Sweet Tea Dorminy
2023-07-17 15:34 ` Luís Henriques
2023-07-17 17:28 ` David Sterba
2023-07-18 8:36 ` Luís Henriques
2023-07-17 3:52 ` [PATCH v2 05/17] btrfs: add inode encryption contexts Sweet Tea Dorminy
2023-07-17 15:41 ` Josef Bacik
2023-07-17 3:52 ` [PATCH v2 06/17] btrfs: add new FEATURE_INCOMPAT_ENCRYPT flag Sweet Tea Dorminy
2023-07-17 15:42 ` Josef Bacik
2023-07-17 3:52 ` [PATCH v2 07/17] btrfs: adapt readdir for encrypted and nokey names Sweet Tea Dorminy
2023-07-17 15:34 ` Luís Henriques
2023-07-17 17:46 ` Josef Bacik
2023-07-17 3:52 ` [PATCH v2 08/17] btrfs: use correct name hash for " Sweet Tea Dorminy
2023-07-17 3:52 ` [PATCH v2 09/17] btrfs: implement fscrypt ioctls Sweet Tea Dorminy
2023-07-17 3:52 ` [PATCH v2 10/17] btrfs: add encryption to CONFIG_BTRFS_DEBUG Sweet Tea Dorminy
2023-07-17 3:52 ` [PATCH v2 11/17] btrfs: add get_devices hook for fscrypt Sweet Tea Dorminy
2023-07-17 17:51 ` Josef Bacik
2023-07-17 3:52 ` [PATCH v2 12/17] btrfs: turn on inlinecrypt mount option for encrypt Sweet Tea Dorminy
2023-07-17 15:34 ` Luís Henriques
2023-07-17 17:55 ` Josef Bacik
2023-07-17 3:52 ` [PATCH v2 13/17] btrfs: turn on the encryption ioctls Sweet Tea Dorminy
2023-07-17 3:52 ` [PATCH v2 14/17] btrfs: create and free extent fscrypt_infos Sweet Tea Dorminy
2023-07-17 17:58 ` Josef Bacik
2023-07-17 3:52 ` [PATCH v2 15/17] btrfs: start tracking extent encryption context info Sweet Tea Dorminy
2023-07-17 18:11 ` Josef Bacik
2023-07-17 3:52 ` [PATCH v2 16/17] btrfs: explicitly track file extent length and encryption Sweet Tea Dorminy
2023-07-17 15:30 ` Josef Bacik
2023-07-17 18:12 ` Josef Bacik
2023-07-17 3:52 ` [PATCH v2 17/17] btrfs: save and load fscrypt extent contexts Sweet Tea Dorminy
2023-07-17 18:15 ` Josef Bacik [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20230717181525.GQ691303@perftesting \
--to=josef@toxicpanda.com \
--cc=clm@fb.com \
--cc=dsterba@suse.com \
--cc=ebiggers@kernel.org \
--cc=jaegeuk@kernel.org \
--cc=kernel-team@meta.com \
--cc=linux-btrfs@vger.kernel.org \
--cc=linux-fscrypt@vger.kernel.org \
--cc=sweettea-kernel@dorminy.me \
--cc=tytso@mit.edu \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox