Re: [PATCH v2 11/20] fscrypt_d_revalidate(): use stable parent inode passed by caller

Jeff Layton
2025-01-17T15:20:41Z

On Thu, 2025-01-16 at 05:23 +0000, Al Viro wrote:
> The only thing it's using is parent directory inode and we are already
> given a stable reference to that - no need to bother with boilerplate.
>
> Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
> ---
> fs/crypto/fname.c | 21 +++++----------------
> 1 file changed, 5 insertions(+), 16 deletions(-)
>
> diff --git a/fs/crypto/fname.c b/fs/crypto/fname.c
> index 389f5b2bf63b..010f9c0a4c2f 100644
> --- a/fs/crypto/fname.c
> +++ b/fs/crypto/fname.c
> @@ -574,12 +574,10 @@ EXPORT_SYMBOL_GPL(fscrypt_fname_siphash);
> * Validate dentries in encrypted directories to make sure we aren't potentially
> * caching stale dentries after a key has been added.
> */
> -int fscrypt_d_revalidate(struct inode *parent_dir, const struct qstr *name,
> +int fscrypt_d_revalidate(struct inode *dir, const struct qstr *name,
> struct dentry *dentry, unsigned int flags)
> {
> - struct dentry *dir;
> int err;
> - int valid;
>
> /*
> * Plaintext names are always valid, since fscrypt doesn't support
> @@ -592,30 +590,21 @@ int fscrypt_d_revalidate(struct inode *parent_dir, const struct qstr *name,
> /*
> * No-key name; valid if the directory's key is still unavailable.
> *
> - * Although fscrypt forbids rename() on no-key names, we still must use
> - * dget_parent() here rather than use ->d_parent directly. That's
> - * because a corrupted fs image may contain directory hard links, which
> - * the VFS handles by moving the directory's dentry tree in the dcache
> - * each time ->lookup() finds the directory and it already has a dentry
> - * elsewhere. Thus ->d_parent can be changing, and we must safely grab
> - * a reference to some ->d_parent to prevent it from being freed.
> + * Note in RCU mode we have to bail if we get here -
> + * fscrypt_get_encryption_info() may block.
> */
>
> if (flags & LOOKUP_RCU)
> return -ECHILD;
>
> - dir = dget_parent(dentry);
> /*
> * Pass allow_unsupported=true, so that files with an unsupported
> * encryption policy can be deleted.
> */
> - err = fscrypt_get_encryption_info(d_inode(dir), true);
> - valid = !fscrypt_has_encryption_key(d_inode(dir));
> - dput(dir);
> -
> + err = fscrypt_get_encryption_info(dir, true);
> if (err < 0)
> return err;
>
> - return valid;
> + return !fscrypt_has_encryption_key(dir);
> }
> EXPORT_SYMBOL_GPL(fscrypt_d_revalidate);

Reviewed-by: Jeff Layton <jlayton@kernel.org>