On Sun, Jul 16, 2023 at 11:52:46PM -0400, Sweet Tea Dorminy wrote:
> This puts the long-preserved 1-byte encryption field to work, storing
> whether the extent is encrypted and the length of its fscrypt_context.
> Since there is no way for a btrfs inode to be encrypted right now, we
> set context length to be 0 in all locations. We then update all the
> locations which check the size of an extent item to expect the size to
> agree with the context length.
>
> The 1-byte field packs the encryption type and context length together,
> to avoid a format change. Right now we don't anticipate very many
> encryption policies, so 2 bits should be ample; similarly right now we
> can't imagine a context larger than fscrypt's current contexts, which
> are 40 bytes, so 6 bits is ample to store the context's size; and
> therefore we can pack these together into the one-byte encryption field
> without touching other space reserved for future use.
>
I don't like this design, I think it doesn't give us enough flexibility.
I think I'd rather use the whole u8 to have the encryption policy, and then the
first u32 of the extent context is the size. So something like
struct btrfs_encryption_context {
__le32 len;
__u8 context[0];
};
struct btrfs_file_extent {
struct btrfs_encryption_context fscrypt_context[0];
};
It takes up a little more space but is more flexible and enables us more
flexibility in the future.
> Signed-off-by: Sweet Tea Dorminy <sweettea-kernel@xxxxxxxxxx>
> ---
> fs/btrfs/accessors.h | 31 +++++++++++++++++++++++++++++++
> fs/btrfs/file-item.c | 8 ++++++++
> fs/btrfs/fscrypt.h | 24 ++++++++++++++++++++++++
> fs/btrfs/inode.c | 26 ++++++++++++++++++++++----
> fs/btrfs/tree-checker.c | 37 +++++++++++++++++++++++++++++--------
> fs/btrfs/tree-log.c | 3 +++
> 6 files changed, 117 insertions(+), 12 deletions(-)
>
> diff --git a/fs/btrfs/accessors.h b/fs/btrfs/accessors.h
> index ceadfc5d6c66..fbee45b9d9c2 100644
> --- a/fs/btrfs/accessors.h
> +++ b/fs/btrfs/accessors.h
> @@ -3,6 +3,8 @@
> #ifndef BTRFS_ACCESSORS_H
> #define BTRFS_ACCESSORS_H
>
> +#include "fscrypt.h"
> +
I don't want to pull other dependencies into accessors.h, I want to make syncing
*something* into btrfs-progs easy.
> struct btrfs_map_token {
> struct extent_buffer *eb;
> char *kaddr;
> @@ -936,6 +938,16 @@ BTRFS_SETGET_STACK_FUNCS(stack_file_extent_disk_num_bytes,
> struct btrfs_file_extent_item, disk_num_bytes, 64);
> BTRFS_SETGET_STACK_FUNCS(stack_file_extent_compression,
> struct btrfs_file_extent_item, compression, 8);
> +BTRFS_SETGET_STACK_FUNCS(stack_file_extent_encryption,
> + struct btrfs_file_extent_item, encryption, 8);
> +static inline u8 btrfs_stack_file_extent_encryption_ctxsize(
> + struct btrfs_file_extent_item *e)
> +{
> + u8 ctxsize;
> +
> + btrfs_unpack_encryption(e->encryption, NULL, &ctxsize);
> + return ctxsize;
> +}
This helper can go into fscrypt with the other related helpers.
>
>
> BTRFS_SETGET_FUNCS(file_extent_type, struct btrfs_file_extent_item, type, 8);
> @@ -958,6 +970,25 @@ BTRFS_SETGET_FUNCS(file_extent_encryption, struct btrfs_file_extent_item,
> BTRFS_SETGET_FUNCS(file_extent_other_encoding, struct btrfs_file_extent_item,
> other_encoding, 16);
>
> +static inline u8
> +btrfs_file_extent_encryption_ctxsize(const struct extent_buffer *eb,
> + struct btrfs_file_extent_item *e)
> +{
> + u8 ctxsize;
> +
> + btrfs_unpack_encryption(btrfs_file_extent_encryption(eb, e),
> + NULL, &ctxsize);
> + return ctxsize;
> +}
Same for this one. Thanks,
Josef
|