On Wed, 2025-03-19 at 14:01 +1100, NeilBrown wrote:
> From: NeilBrown <neilb@xxxxxxx>
>
> try_lookup_noperm() and d_hash_and_lookup() are nearly identical. The
> former does some validation of the name where the latter doesn't.
> Outside of the VFS that validation is likely valuable, and having only
> one exported function for this task is certainly a good idea.
>
> So make d_hash_and_lookup() local to VFS files and change all other
> callers to try_lookup_noperm(). Note that the arguments are swapped.
>
> Signed-off-by: NeilBrown <neilb@xxxxxxx>
> ---
> Documentation/filesystems/porting.rst | 11 +++++++++++
> fs/dcache.c | 1 -
> fs/efivarfs/super.c | 14 ++++----------
> fs/internal.h | 1 +
> fs/proc/base.c | 2 +-
> fs/smb/client/readdir.c | 3 ++-
> fs/xfs/scrub/orphanage.c | 4 ++--
> include/linux/dcache.h | 1 -
> net/sunrpc/rpc_pipe.c | 12 ++++++------
> security/selinux/selinuxfs.c | 4 ++--
> 10 files changed, 29 insertions(+), 24 deletions(-)
>
> diff --git a/Documentation/filesystems/porting.rst b/Documentation/filesystems/porting.rst
> index df9516cd82e0..626f094787e8 100644
> --- a/Documentation/filesystems/porting.rst
> +++ b/Documentation/filesystems/porting.rst
> @@ -1225,3 +1225,14 @@ checked that the caller has 'X' permission on the parent. They must
> ONLY be used internally by a filesystem on itself when it knows that
> permissions are irrelevant or in a context where permission checks have
> already been performed such as after vfs_path_parent_lookup()
> +
> +---
> +
> +** mandatory**
> +
> +d_hash_and_lookup() is no longer exported or available outside the VFS.
> +Use try_lookup_noperm() instead. This adds name validation and takes
> +arguments in the opposite order but is otherwise identical.
> +
> +Using try_lookup_noperm() will require linux/namei.h to be included.
> +
> diff --git a/fs/dcache.c b/fs/dcache.c
> index 726a5be2747b..17f8e0b7f04f 100644
> --- a/fs/dcache.c
> +++ b/fs/dcache.c
> @@ -2395,7 +2395,6 @@ struct dentry *d_hash_and_lookup(struct dentry *dir, struct qstr *name)
> }
> return d_lookup(dir, name);
> }
> -EXPORT_SYMBOL(d_hash_and_lookup);
>
> /*
> * When a file is deleted, we have two options:
> diff --git a/fs/efivarfs/super.c b/fs/efivarfs/super.c
> index 09fcf731e65d..867cd6e0fbad 100644
> --- a/fs/efivarfs/super.c
> +++ b/fs/efivarfs/super.c
> @@ -204,7 +204,6 @@ bool efivarfs_variable_is_present(efi_char16_t *variable_name,
> char *name = efivar_get_utf8name(variable_name, vendor);
> struct super_block *sb = data;
> struct dentry *dentry;
> - struct qstr qstr;
>
> if (!name)
> /*
> @@ -217,9 +216,7 @@ bool efivarfs_variable_is_present(efi_char16_t *variable_name,
> */
> return true;
>
> - qstr.name = name;
> - qstr.len = strlen(name);
> - dentry = d_hash_and_lookup(sb->s_root, &qstr);
> + dentry = try_lookup_noperm(&QSTR(name), sb->s_root);
> kfree(name);
> if (!IS_ERR_OR_NULL(dentry))
> dput(dentry);
> @@ -402,8 +399,8 @@ static bool efivarfs_actor(struct dir_context *ctx, const char *name, int len,
> {
> unsigned long size;
> struct efivarfs_ctx *ectx = container_of(ctx, struct efivarfs_ctx, ctx);
> - struct qstr qstr = { .name = name, .len = len };
> - struct dentry *dentry = d_hash_and_lookup(ectx->sb->s_root, &qstr);
> + struct dentry *dentry = try_lookup_noperm(QSTR_LEN(name, len),
> + ectx->sb->s_root);
> struct inode *inode;
> struct efivar_entry *entry;
> int err;
> @@ -439,7 +436,6 @@ static int efivarfs_check_missing(efi_char16_t *name16, efi_guid_t vendor,
> char *name;
> struct super_block *sb = data;
> struct dentry *dentry;
> - struct qstr qstr;
> int err;
>
> if (guid_equal(&vendor, &LINUX_EFI_RANDOM_SEED_TABLE_GUID))
> @@ -449,9 +445,7 @@ static int efivarfs_check_missing(efi_char16_t *name16, efi_guid_t vendor,
> if (!name)
> return -ENOMEM;
>
> - qstr.name = name;
> - qstr.len = strlen(name);
> - dentry = d_hash_and_lookup(sb->s_root, &qstr);
> + dentry = try_lookup_noperm(&QSTR(name), sb->s_root);
> if (IS_ERR(dentry)) {
> err = PTR_ERR(dentry);
> goto out;
> diff --git a/fs/internal.h b/fs/internal.h
> index e7f02ae1e098..c21534a23196 100644
> --- a/fs/internal.h
> +++ b/fs/internal.h
> @@ -66,6 +66,7 @@ int do_linkat(int olddfd, struct filename *old, int newdfd,
> int vfs_tmpfile(struct mnt_idmap *idmap,
> const struct path *parentpath,
> struct file *file, umode_t mode);
> +struct dentry *d_hash_and_lookup(struct dentry *, struct qstr *);
>
> /*
> * namespace.c
> diff --git a/fs/proc/base.c b/fs/proc/base.c
> index cd89e956c322..7d36c7567c31 100644
> --- a/fs/proc/base.c
> +++ b/fs/proc/base.c
> @@ -2124,7 +2124,7 @@ bool proc_fill_cache(struct file *file, struct dir_context *ctx,
> unsigned type = DT_UNKNOWN;
> ino_t ino = 1;
>
> - child = d_hash_and_lookup(dir, &qname);
> + child = try_lookup_noperm(&qname, dir);
> if (!child) {
> DECLARE_WAIT_QUEUE_HEAD_ONSTACK(wq);
> child = d_alloc_parallel(dir, &qname, &wq);
> diff --git a/fs/smb/client/readdir.c b/fs/smb/client/readdir.c
> index 50f96259d9ad..7329ec532bcf 100644
> --- a/fs/smb/client/readdir.c
> +++ b/fs/smb/client/readdir.c
> @@ -9,6 +9,7 @@
> *
> */
> #include <linux/fs.h>
> +#include <linux/namei.h>
> #include <linux/pagemap.h>
> #include <linux/slab.h>
> #include <linux/stat.h>
> @@ -78,7 +79,7 @@ cifs_prime_dcache(struct dentry *parent, struct qstr *name,
>
> cifs_dbg(FYI, "%s: for %s\n", __func__, name->name);
>
> - dentry = d_hash_and_lookup(parent, name);
> + dentry = try_lookup_noperm(name, parent);
> if (!dentry) {
> /*
> * If we know that the inode will need to be revalidated
> diff --git a/fs/xfs/scrub/orphanage.c b/fs/xfs/scrub/orphanage.c
> index 987af5b2bb82..f42ffad5a7b9 100644
> --- a/fs/xfs/scrub/orphanage.c
> +++ b/fs/xfs/scrub/orphanage.c
> @@ -444,7 +444,7 @@ xrep_adoption_check_dcache(
> if (!d_orphanage)
> return 0;
>
> - d_child = d_hash_and_lookup(d_orphanage, &qname);
> + d_child = try_lookup_noperm(&qname, d_orphanage);
> if (d_child) {
> trace_xrep_adoption_check_child(sc->mp, d_child);
>
> @@ -481,7 +481,7 @@ xrep_adoption_zap_dcache(
> if (!d_orphanage)
> return;
>
> - d_child = d_hash_and_lookup(d_orphanage, &qname);
> + d_child = try_lookup_noperm(&qname, d_orphanage);
> while (d_child != NULL) {
> trace_xrep_adoption_invalidate_child(sc->mp, d_child);
>
> diff --git a/include/linux/dcache.h b/include/linux/dcache.h
> index 1f01f4e734c5..cf37ae54955d 100644
> --- a/include/linux/dcache.h
> +++ b/include/linux/dcache.h
> @@ -288,7 +288,6 @@ extern void d_exchange(struct dentry *, struct dentry *);
> extern struct dentry *d_ancestor(struct dentry *, struct dentry *);
>
> extern struct dentry *d_lookup(const struct dentry *, const struct qstr *);
> -extern struct dentry *d_hash_and_lookup(struct dentry *, struct qstr *);
>
> static inline unsigned d_count(const struct dentry *dentry)
> {
> diff --git a/net/sunrpc/rpc_pipe.c b/net/sunrpc/rpc_pipe.c
> index eadc00410ebc..98f78cd55905 100644
> --- a/net/sunrpc/rpc_pipe.c
> +++ b/net/sunrpc/rpc_pipe.c
> @@ -631,7 +631,7 @@ static struct dentry *__rpc_lookup_create_exclusive(struct dentry *parent,
> const char *name)
> {
> struct qstr q = QSTR(name);
> - struct dentry *dentry = d_hash_and_lookup(parent, &q);
> + struct dentry *dentry = try_lookup_noperm(&q, parent);
> if (!dentry) {
> dentry = d_alloc(parent, &q);
> if (!dentry)
> @@ -658,7 +658,7 @@ static void __rpc_depopulate(struct dentry *parent,
> for (i = start; i < eof; i++) {
> name.name = files[i].name;
> name.len = strlen(files[i].name);
> - dentry = d_hash_and_lookup(parent, &name);
> + dentry = try_lookup_noperm(&name, parent);
>
> if (dentry == NULL)
> continue;
> @@ -1190,7 +1190,7 @@ static const struct rpc_filelist files[] = {
> struct dentry *rpc_d_lookup_sb(const struct super_block *sb,
> const unsigned char *dir_name)
> {
> - return d_hash_and_lookup(sb->s_root, &QSTR(dir_name));
> + return try_lookup_noperm(&QSTR(dir_name), sb->s_root);
> }
> EXPORT_SYMBOL_GPL(rpc_d_lookup_sb);
>
> @@ -1301,7 +1301,7 @@ rpc_gssd_dummy_populate(struct dentry *root, struct rpc_pipe *pipe_data)
> struct dentry *pipe_dentry = NULL;
>
> /* We should never get this far if "gssd" doesn't exist */
> - gssd_dentry = d_hash_and_lookup(root, &QSTR(files[RPCAUTH_gssd].name));
> + gssd_dentry = try_lookup_noperm(&QSTR(files[RPCAUTH_gssd].name), root);
> if (!gssd_dentry)
> return ERR_PTR(-ENOENT);
>
> @@ -1311,8 +1311,8 @@ rpc_gssd_dummy_populate(struct dentry *root, struct rpc_pipe *pipe_data)
> goto out;
> }
>
> - clnt_dentry = d_hash_and_lookup(gssd_dentry,
> - &QSTR(gssd_dummy_clnt_dir[0].name));
> + clnt_dentry = try_lookup_noperm(&QSTR(gssd_dummy_clnt_dir[0].name),
> + gssd_dentry);
> if (!clnt_dentry) {
> __rpc_depopulate(gssd_dentry, gssd_dummy_clnt_dir, 0, 1);
> pipe_dentry = ERR_PTR(-ENOENT);
> diff --git a/security/selinux/selinuxfs.c b/security/selinux/selinuxfs.c
> index 47480eb2189b..e67a8ce4b64c 100644
> --- a/security/selinux/selinuxfs.c
> +++ b/security/selinux/selinuxfs.c
> @@ -2158,8 +2158,8 @@ static int __init init_sel_fs(void)
> return err;
> }
>
> - selinux_null.dentry = d_hash_and_lookup(selinux_null.mnt->mnt_root,
> - &null_name);
> + selinux_null.dentry = try_lookup_noperm(&null_name,
> + selinux_null.mnt->mnt_root);
> if (IS_ERR(selinux_null.dentry)) {
> pr_err("selinuxfs: could not lookup null!\n");
> err = PTR_ERR(selinux_null.dentry);
Reviewed-by: Jeff Layton <jlayton@xxxxxxxxxx>
