Re: [PATCH v2 05/14] fscrypt: setup leaf inodes for extent encryption

[Josef Bacik <josef@toxicpanda.com>]

On Sun, Jul 09, 2023 at 02:53:38PM -0400, Sweet Tea Dorminy wrote:
> For extent-based encryption, leaf/regular file inodes are special: it's
> useful to set their i_crypt_info field so that it's easy to inherit
> their encryption policy for a new extent, but they never need to do any
> encyption themselves. Additionally, since encryption can only be set up
> on a directory, not a single file, their encryption policy can always
> duplicate their parent inode's policy.
> 
> Signed-off-by: Sweet Tea Dorminy <sweettea-kernel@dorminy.me>
> ---
>  fs/crypto/fscrypt_private.h | 17 +++++++++++++
>  fs/crypto/keysetup.c        | 49 ++++++++++++++++++++++++++-----------
>  2 files changed, 52 insertions(+), 14 deletions(-)
> 
> diff --git a/fs/crypto/fscrypt_private.h b/fs/crypto/fscrypt_private.h
> index c04454c289fd..260635e8b558 100644
> --- a/fs/crypto/fscrypt_private.h
> +++ b/fs/crypto/fscrypt_private.h
> @@ -308,6 +308,23 @@ fscrypt_get_lblk_info(const struct inode *inode, u64 lblk, u64 *offset,
>  	return inode->i_crypt_info;
>  }
>  
> +/**
> + * fscrypt_uses_extent_encryption() -- whether an inode uses per-extent
> + *				       encryption
> + *
> + * @inode: the inode in question
> + *
> + * Return: true if the inode uses per-extent fscrypt_infos, false otherwise
> + */
> +static inline bool fscrypt_uses_extent_encryption(const struct inode *inode)
> +{
> +	// Non-regular files don't have extents
> +	if (!S_ISREG(inode->i_mode))
> +		return false;
> +
> +	// No filesystems currently use per-extent infos
> +	return false;

Wrong comment format.

> +}
>  
>  /* crypto.c */
>  extern struct kmem_cache *fscrypt_info_cachep;
> diff --git a/fs/crypto/keysetup.c b/fs/crypto/keysetup.c
> index c5f68cf65a6f..7469b2d8ac87 100644
> --- a/fs/crypto/keysetup.c
> +++ b/fs/crypto/keysetup.c
> @@ -747,27 +747,48 @@ fscrypt_setup_encryption_info(struct inode *inode,
>  int fscrypt_get_encryption_info(struct inode *inode, bool allow_unsupported)
>  {
>  	int res;
> -	union fscrypt_context ctx;
> +	union fscrypt_context ctx = { 0 };
>  	union fscrypt_policy policy;
>  
>  	if (fscrypt_has_encryption_key(inode))
>  		return 0;
>  
> -	res = inode->i_sb->s_cop->get_context(inode, &ctx, sizeof(ctx));
> -	if (res < 0) {
> -		if (res == -ERANGE && allow_unsupported)
> -			return 0;
> -		fscrypt_warn(inode, "Error %d getting encryption context", res);
> -		return res;
> -	}
> +	if (fscrypt_uses_extent_encryption(inode)) {
> +		/*
> +		 * Nothing will be encrypted with this info, so we can borrow
> +		 * the parent (dir) inode's policy and use a zero nonce.
> +		 */
> +		struct dentry *dentry = d_find_any_alias(inode);
> +		struct dentry *parent_dentry = dget_parent(dentry);
> +		struct inode *dir = parent_dentry->d_inode;
> +		bool found = false;
>  

Can this be extracted to a helper to keep this function cleaner?  Thanks,

Josef