Re: [PATCH v2 12/20] btrfs: start using fscrypt hooks.

David Sterba
2022-09-07T20:22:44Z

On Mon, Sep 05, 2022 at 08:35:27PM -0400, Sweet Tea Dorminy wrote:
> From: Omar Sandoval <osandov@osandov.com>
>
> In order to appropriately encrypt, create, open, rename, and various symlink
> operations must call fscrypt hooks. These determine whether the inode
> should be encrypted and do other preparatory actions. The superblock
> must have fscrypt operations registered, so implement the minimal set
> also.
>
> Signed-off-by: Omar Sandoval <osandov@osandov.com>
> Signed-off-by: Sweet Tea Dorminy <sweettea-kernel@dorminy.me>
> ---
> fs/btrfs/ctree.h | 1 +
> fs/btrfs/file.c | 3 ++
> fs/btrfs/fscrypt.c | 3 ++
> fs/btrfs/fscrypt.h | 1 +
> fs/btrfs/inode.c | 91 ++++++++++++++++++++++++++++++++++++++++------
> fs/btrfs/super.c | 3 ++
> 6 files changed, 90 insertions(+), 12 deletions(-)
>
> diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
> index 230537a007b6..2b9ba8d77861 100644
> --- a/fs/btrfs/ctree.h
> +++ b/fs/btrfs/ctree.h
> @@ -3416,6 +3416,7 @@ struct btrfs_new_inode_args {
> */
> struct posix_acl *default_acl;
> struct posix_acl *acl;
> + bool encrypt;
> };
> int btrfs_new_inode_prepare(struct btrfs_new_inode_args *args,
> unsigned int *trans_num_items);
> diff --git a/fs/btrfs/file.c b/fs/btrfs/file.c
> index 7216ac1f860c..929a0308676c 100644
> --- a/fs/btrfs/file.c
> +++ b/fs/btrfs/file.c
> @@ -3695,6 +3695,9 @@ static int btrfs_file_open(struct inode *inode, struct file *filp)
> int ret;
>
> filp->f_mode |= FMODE_NOWAIT | FMODE_BUF_RASYNC;
> + ret = fscrypt_file_open(inode, filp);
> + if (ret)
> + return ret;
>
> ret = fsverity_file_open(inode, filp);

Can fsverity and fscrypt can be used at the same time?

> if (ret)
> --- a/fs/btrfs/fscrypt.h
> +++ b/fs/btrfs/fscrypt.h
> @@ -22,4 +22,5 @@ static bool btrfs_fscrypt_match_name(const struct fscrypt_name *fname,
> }
> #endif
>
> +extern const struct fscrypt_operations btrfs_fscrypt_ops;

Please keep a blank line before the last #endif

> #endif /* BTRFS_FSCRYPT_H */
> @@ -9907,15 +9927,22 @@ static int btrfs_symlink(struct user_namespace *mnt_userns, struct inode *dir,
> };
> unsigned int trans_num_items;
> int err;
> - int name_len;
> int datasize;
> unsigned long ptr;
> struct btrfs_file_extent_item *ei;
> struct extent_buffer *leaf;
> + struct fscrypt_str disk_link;
> + u32 name_len = strlen(symname);
>
> - name_len = strlen(symname);
> - if (name_len > BTRFS_MAX_INLINE_DATA_SIZE(fs_info))
> - return -ENAMETOOLONG;
> + /*
> + * fscrypt sets disk_link.len to be len + 1, including a NULL terminator, but we
> + * don't store that NULL.

I think it should be referred to as NUL character, or as '\0'.

> + */
> + err = fscrypt_prepare_symlink(dir, symname, name_len,
> + BTRFS_MAX_INLINE_DATA_SIZE(fs_info) + 1,
> + &disk_link);
> + if (err)
> + return err;
>
> inode = new_inode(dir->i_sb);
> if (!inode)
> @@ -9994,6 +10035,29 @@ static int btrfs_symlink(struct user_namespace *mnt_userns, struct inode *dir,
> --- a/fs/btrfs/super.c
> +++ b/fs/btrfs/super.c
> @@ -47,6 +47,8 @@
> #include "tests/btrfs-tests.h"
> #include "block-group.h"
> #include "discard.h"
> +#include "fscrypt.h"

No newline please

> +
> #include "qgroup.h"
> #include "raid56.h"
> #define CREATE_TRACE_POINTS