Re: [PATCH v2 1/2] fscrypt: new helper function - fscrypt_prepare_atomic_open()

[Xiubo Li <xiubli@redhat.com>]

On 14/03/2023 18:39, Luís Henriques wrote:
> This patch introduces a new helper function which prepares an atomic_open.
> Because atomic open can act as a lookup if handed a dentry that is negative,
> we need to set DCACHE_NOKEY_NAME if the key for the parent isn't available.
>
> The reason for getting the encryption info before checking if the directory has
> the encryption key is because we may have the key available but the encryption
> info isn't yet set (maybe due to a drop_caches).  The regular open path will
> call fscrypt_file_open() which uses function fscrypt_require_key() for setting
> the encryption info if needed.  The atomic open needs to do something similar.
>
> Signed-off-by: Luís Henriques <lhenriques@suse.de>
> ---
>   fs/crypto/hooks.c       | 33 +++++++++++++++++++++++++++++++++
>   include/linux/fscrypt.h |  7 +++++++
>   2 files changed, 40 insertions(+)
>
> diff --git a/fs/crypto/hooks.c b/fs/crypto/hooks.c
> index 7b8c5a1104b5..165ddf01bf9f 100644
> --- a/fs/crypto/hooks.c
> +++ b/fs/crypto/hooks.c
> @@ -117,6 +117,39 @@ int __fscrypt_prepare_readdir(struct inode *dir)
>   }
>   EXPORT_SYMBOL_GPL(__fscrypt_prepare_readdir);
>   
> +/**
> + * fscrypt_prepare_atomic_open() - prepare an atomic open on an encrypted directory
> + * @dir: inode of parent directory
> + * @dentry: dentry being open
> + *
> + * Because atomic open can act as a lookup if handed a dentry that is negative,
> + * we need to set DCACHE_NOKEY_NAME if the key for the parent isn't available.
> + *
> + * The reason for getting the encryption info before checking if the directory
> + * has the encryption key is because the key may be available but the encryption
> + * info isn't yet set (maybe due to a drop_caches).  The regular open path will
> + * call fscrypt_file_open() which uses function fscrypt_require_key() for
> + * setting the encryption info if needed.  The atomic open needs to do something
> + * similar.
> + *
> + * Return: 0 on success, or an error code if fscrypt_get_encryption_info()
> + * fails.
> + */
> +int fscrypt_prepare_atomic_open(struct inode *dir, struct dentry *dentry)
> +{
> +	int err;
> +
> +	err = fscrypt_get_encryption_info(dir, true);
> +	if (!err && !fscrypt_has_encryption_key(dir)) {
> +		spin_lock(&dentry->d_lock);
> +		dentry->d_flags |= DCACHE_NOKEY_NAME;
> +		spin_unlock(&dentry->d_lock);
> +	}
> +
> +	return err;
> +}
> +EXPORT_SYMBOL_GPL(fscrypt_prepare_atomic_open);
> +
>   int __fscrypt_prepare_setattr(struct dentry *dentry, struct iattr *attr)
>   {
>   	if (attr->ia_valid & ATTR_SIZE)
> diff --git a/include/linux/fscrypt.h b/include/linux/fscrypt.h
> index 4f5f8a651213..c70acb2a737a 100644
> --- a/include/linux/fscrypt.h
> +++ b/include/linux/fscrypt.h
> @@ -362,6 +362,7 @@ int __fscrypt_prepare_rename(struct inode *old_dir, struct dentry *old_dentry,
>   int __fscrypt_prepare_lookup(struct inode *dir, struct dentry *dentry,
>   			     struct fscrypt_name *fname);
>   int __fscrypt_prepare_readdir(struct inode *dir);
> +int fscrypt_prepare_atomic_open(struct inode *dir, struct dentry *dentry);
>   int __fscrypt_prepare_setattr(struct dentry *dentry, struct iattr *attr);
>   int fscrypt_prepare_setflags(struct inode *inode,
>   			     unsigned int oldflags, unsigned int flags);
> @@ -688,6 +689,12 @@ static inline int __fscrypt_prepare_readdir(struct inode *dir)
>   	return -EOPNOTSUPP;
>   }
>   
> +static inline int fscrypt_prepare_atomic_open(struct inode *dir,
> +					      struct dentry *dentry)
> +{
> +	return -EOPNOTSUPP;
> +}
> +
>   static inline int __fscrypt_prepare_setattr(struct dentry *dentry,
>   					    struct iattr *attr)
>   {
>
Reviewed-by: Xiubo Li <xiubli@redhat.com>