Re: [PATCH v2 13/14] fscrypt: save session key credentials for extent infos

[Josef Bacik <josef@toxicpanda.com>]

On Sun, Jul 09, 2023 at 02:53:46PM -0400, Sweet Tea Dorminy wrote:
> For v1 encryption policies using per-session keys, the thread which
> opens the inode and therefore initializes the encryption info is part of
> the session, so it can get the key from the session keyring. However,
> for extent encryption, the extent infos are likely loaded from a
> different thread, which does not have access to the session keyring.
> This change saves the credentials of the inode opening thread and reuses
> those credentials temporarily when dealing with extent infos, allowing
> finding the encryption key correctly.
> 
> v1 encryption policies using per-session keys should probably not exist
> for new usages such as extent encryption, but this makes more tests
> work without change; maybe the right answer is to disallow v1 session
> keys plus extent encryption and deal with editing tests to not use v1
> session encryption so much.
> 
> Signed-off-by: Sweet Tea Dorminy <sweettea-kernel@dorminy.me>
> ---
>  fs/crypto/fscrypt_private.h |  8 ++++++++
>  fs/crypto/keysetup.c        | 14 ++++++++++++++
>  fs/crypto/keysetup_v1.c     |  1 +
>  3 files changed, 23 insertions(+)
> 
> diff --git a/fs/crypto/fscrypt_private.h b/fs/crypto/fscrypt_private.h
> index 6e6020f7746c..a1c484511ba3 100644
> --- a/fs/crypto/fscrypt_private.h
> +++ b/fs/crypto/fscrypt_private.h
> @@ -231,6 +231,14 @@ struct fscrypt_info {
>  	 */
>  	bool ci_inlinecrypt;
>  #endif
> +	/* Credential struct from the thread which created this info. This is
> +	 * only used in v1 session keyrings with extent encryption; it allows
> +	 * the thread creating extents for an inode to join the session
> +	 * keyring temporarily, since otherwise the thread is usually part of
> +	 * kernel writeback and therefore unrelated to the thread with the
> +	 * right session key.
> +	 */
> +	struct cred *ci_session_creds;
>  
>  	/*
>  	 * Encryption mode used for this inode.  It corresponds to either the
> diff --git a/fs/crypto/keysetup.c b/fs/crypto/keysetup.c
> index 3b80e7061039..9c56ef8d2eb6 100644
> --- a/fs/crypto/keysetup.c
> +++ b/fs/crypto/keysetup.c
> @@ -646,6 +646,8 @@ static void put_crypt_info(struct fscrypt_info *ci)
>  
>  		fscrypt_put_master_key_activeref(ci->ci_sb, mk);
>  	}
> +	if (ci->ci_session_creds)
> +		abort_creds(ci->ci_session_creds);
>  	memzero_explicit(ci, sizeof(*ci));
>  	kmem_cache_free(fscrypt_info_cachep, ci);
>  }
> @@ -662,6 +664,7 @@ fscrypt_setup_encryption_info(struct inode *inode,
>  	struct fscrypt_master_key *mk = NULL;
>  	int res;
>  	bool info_for_extent = !!info_ptr;
> +	const struct cred *creds = NULL;
>  
>  	if (!info_ptr)
>  		info_ptr = &inode->i_crypt_info;
> @@ -705,7 +708,18 @@ fscrypt_setup_encryption_info(struct inode *inode,
>  	if (res)
>  		goto out;
>  
> +	if (info_for_extent && inode->i_crypt_info->ci_session_creds) {
> +		 creds = override_creds(inode->i_crypt_info->ci_session_creds);

Whitespace.  Thanks,

Josef