| Message ID | 20250319031545.2999807-6-neil@brown.name (mailing list archive) |
|---|---|
| State | Handled Elsewhere |
| Headers | show |
| Series | tidy up various VFS lookup functions | expand |
On Wed, 2025-03-19 at 14:01 +1100, NeilBrown wrote: > From: NeilBrown <neilb@suse.de> > > 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@suse.de> > --- > 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@kernel.org>
On Wed, Mar 19, 2025 at 02:01:36PM +1100, NeilBrown wrote: efivarfs stuff conflicts with jejb's fixes. And rpc_pipe part adds fuckload of conflicts for me; what's more, the only caller to survive is rpc_d_lookup_sb(); the rest will be gone anyway... Oh, well...
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);