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

[David Sterba <dsterba@suse.cz>]

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