On Fri, Apr 21, 2023 at 08:03:06PM -0400, Gabriel Krisman Bertazi wrote:
> diff --git a/fs/libfs.c b/fs/libfs.c
> index 4eda519c3002..f8881e29c5d5 100644
> --- a/fs/libfs.c
> +++ b/fs/libfs.c
> @@ -1467,9 +1467,43 @@ static int generic_ci_d_hash(const struct dentry *dentry, struct qstr *str)
> return 0;
> }
>
> +static inline int generic_ci_d_revalidate(struct dentry *dentry,
> + const struct qstr *name,
> + unsigned int flags)
> +{
> + int is_creation = flags & (LOOKUP_CREATE | LOOKUP_RENAME_TARGET);
> +
> + if (d_is_negative(dentry)) {
> + const struct dentry *parent = READ_ONCE(dentry->d_parent);
> + const struct inode *dir = READ_ONCE(parent->d_inode);
> +
> + if (dir && needs_casefold(dir)) {
> + if (!d_is_casefold_lookup(dentry))
> + return 0;
A comment that explains why the !d_is_casefold_lookup() check is needed would be
helpful. I know it's in the commit message, but that's not enough.
> +
> + if (is_creation) {
> + /*
> + * dentry->d_name won't change from under us in
> + * the is_creation path only, since d_revalidate
> + * during creation and renames is always called
> + * with the parent inode locked. This isn't the
> + * case for all lookup callpaths, so it should
> + * not be accessed outside
> + * (LOOKUP_CREATE|LOOKUP_RENAME_TARGET) context.
> + */
> + if (dentry->d_name.len != name->len ||
> + memcmp(dentry->d_name.name, name->name, name->len))
> + return 0;
> + }
> + }
> + }
> + return 1;
> +}
I notice that the existing vfat_revalidate_ci() in fs/fat/namei_vfat.c behaves
differently in the 'flags == 0' case:
/*
* This may be nfsd (or something), anyway, we can't see the
* intent of this. So, since this can be for creation, drop it.
*/
if (!flags)
return 0;
I don't know whether that's really needed, but have you thought about this?
- Eric
