From 5741909697a31cfb08e45d56b4211959fb791487 Mon Sep 17 00:00:00 2001 From: NeilBrown Date: Wed, 19 Mar 2025 14:01:32 +1100 Subject: VFS: improve interface for lookup_one functions The family of functions: lookup_one() lookup_one_unlocked() lookup_one_positive_unlocked() appear designed to be used by external clients of the filesystem rather than by filesystems acting on themselves as the lookup_one_len family are used. They are used by: btrfs/ioctl - which is a user-space interface rather than an internal activity exportfs - i.e. from nfsd or the open_by_handle_at interface overlayfs - at access the underlying filesystems smb/server - for file service They should be used by nfsd (more than just the exportfs path) and cachefs but aren't. It would help if the documentation didn't claim they should "not be called by generic code". Also the path component name is passed as "name" and "len" which are (confusingly?) separate by the "base". In some cases the len in simply "strlen" and so passing a qstr using QSTR() would make the calling clearer. Other callers do pass separate name and len which are stored in a struct. Sometimes these are already stored in a qstr, other times it easily could be. So this patch changes these three functions to receive a 'struct qstr *', and improves the documentation. QSTR_LEN() is added to make it easy to pass a QSTR containing a known len. [brauner@kernel.org: take a struct qstr pointer] Signed-off-by: NeilBrown Link: https://lore.kernel.org/r/20250319031545.2999807-2-neil@brown.name Signed-off-by: Christian Brauner --- include/linux/dcache.h | 3 ++- include/linux/namei.h | 9 ++++----- 2 files changed, 6 insertions(+), 6 deletions(-) (limited to 'include/linux') diff --git a/include/linux/dcache.h b/include/linux/dcache.h index 8d1395f945bf..e974e63bcdbc 100644 --- a/include/linux/dcache.h +++ b/include/linux/dcache.h @@ -57,7 +57,8 @@ struct qstr { }; #define QSTR_INIT(n,l) { { { .len = l } }, .name = n } -#define QSTR(n) (struct qstr)QSTR_INIT(n, strlen(n)) +#define QSTR_LEN(n,l) (struct qstr)QSTR_INIT(n,l) +#define QSTR(n) QSTR_LEN(n, strlen(n)) extern const struct qstr empty_name; extern const struct qstr slash_name; diff --git a/include/linux/namei.h b/include/linux/namei.h index e3042176cdf4..ba02304a2a8a 100644 --- a/include/linux/namei.h +++ b/include/linux/namei.h @@ -73,13 +73,12 @@ extern struct dentry *try_lookup_one_len(const char *, struct dentry *, int); extern struct dentry *lookup_one_len(const char *, struct dentry *, int); extern struct dentry *lookup_one_len_unlocked(const char *, struct dentry *, int); extern struct dentry *lookup_positive_unlocked(const char *, struct dentry *, int); -struct dentry *lookup_one(struct mnt_idmap *, const char *, struct dentry *, int); +struct dentry *lookup_one(struct mnt_idmap *, struct qstr *, struct dentry *); struct dentry *lookup_one_unlocked(struct mnt_idmap *idmap, - const char *name, struct dentry *base, - int len); + struct qstr *name, struct dentry *base); struct dentry *lookup_one_positive_unlocked(struct mnt_idmap *idmap, - const char *name, - struct dentry *base, int len); + struct qstr *name, + struct dentry *base); extern int follow_down_one(struct path *); extern int follow_down(struct path *path, unsigned int flags); -- cgit v1.2.3 From fa6fe07d1536361a227d655e69ca270faf28fdbe Mon Sep 17 00:00:00 2001 From: NeilBrown Date: Wed, 19 Mar 2025 14:01:35 +1100 Subject: VFS: rename lookup_one_len family to lookup_noperm and remove permission check The lookup_one_len family of functions is (now) only used internally by a filesystem on itself either - in a context where permission checking is irrelevant such as by a virtual filesystem populating itself, or xfs accessing its ORPHANAGE or dquota accessing the quota file; or - in a context where a permission check (MAY_EXEC on the parent) has just been performed such as a network filesystem finding in "silly-rename" file in the same directory. This is also the context after the _parentat() functions where currently lookup_one_qstr_excl() is used. So the permission check is pointless. The name "one_len" is unhelpful in understanding the purpose of these functions and should be changed. Most of the callers pass the len as "strlen()" so using a qstr and QSTR() can simplify the code. This patch renames these functions (include lookup_positive_unlocked() which is part of the family despite the name) to have a name based on "lookup_noperm". They are changed to receive a 'struct qstr' instead of separate name and len. In a few cases the use of QSTR() results in a new call to strlen(). try_lookup_noperm() takes a pointer to a qstr instead of the whole qstr. This is consistent with d_hash_and_lookup() (which is nearly identical) and useful for lookup_noperm_unlocked(). The new lookup_noperm_common() doesn't take a qstr yet. That will be tidied up in a subsequent patch. Signed-off-by: NeilBrown Link: https://lore.kernel.org/r/20250319031545.2999807-5-neil@brown.name Signed-off-by: Christian Brauner --- Documentation/filesystems/porting.rst | 20 ++++++++ arch/s390/hypfs/inode.c | 2 +- drivers/android/binderfs.c | 4 +- drivers/infiniband/hw/qib/qib_fs.c | 4 +- fs/afs/dir.c | 2 +- fs/afs/dir_silly.c | 6 +-- fs/autofs/dev-ioctl.c | 3 +- fs/binfmt_misc.c | 2 +- fs/debugfs/inode.c | 6 +-- fs/ecryptfs/inode.c | 16 ++++--- fs/kernfs/mount.c | 2 +- fs/namei.c | 87 +++++++++++++++++++++-------------- fs/nfs/unlink.c | 11 ++--- fs/overlayfs/export.c | 6 +-- fs/overlayfs/namei.c | 2 +- fs/quota/dquot.c | 2 +- fs/smb/client/cached_dir.c | 5 +- fs/smb/client/cifsfs.c | 3 +- fs/tracefs/inode.c | 2 +- fs/xfs/scrub/orphanage.c | 3 +- include/linux/namei.h | 8 ++-- ipc/mqueue.c | 5 +- kernel/bpf/inode.c | 2 +- security/apparmor/apparmorfs.c | 4 +- security/inode.c | 2 +- 25 files changed, 122 insertions(+), 87 deletions(-) (limited to 'include/linux') diff --git a/Documentation/filesystems/porting.rst b/Documentation/filesystems/porting.rst index 57dcba6de743..9150de7f64f1 100644 --- a/Documentation/filesystems/porting.rst +++ b/Documentation/filesystems/porting.rst @@ -1212,3 +1212,23 @@ lookup_one(), lookup_one_unlocked(), lookup_one_positive_unlocked() now take a qstr instead of a name and len. These, not the "one_len" versions, should be used whenever accessing a filesystem from outside that filesysmtem, through a mount point - which will have a mnt_idmap. + +--- + +** mandatory** + +Functions try_lookup_one_len(), lookup_one_len(), +lookup_one_len_unlocked() and lookup_positive_unlocked() have been +renamed to try_lookup_noperm(), lookup_noperm(), +lookup_noperm_unlocked(), lookup_noperm_positive_unlocked(). They now +take a qstr instead of separate name and length. QSTR() can be used +when strlen() is needed for the length. + +For try_lookup_noperm() a reference to the qstr is passed in case the +hash might subsequently be needed. + +These function no longer do any permission checking - they previously +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() diff --git a/arch/s390/hypfs/inode.c b/arch/s390/hypfs/inode.c index 04ea1c03a5ff..96409573c75d 100644 --- a/arch/s390/hypfs/inode.c +++ b/arch/s390/hypfs/inode.c @@ -342,7 +342,7 @@ static struct dentry *hypfs_create_file(struct dentry *parent, const char *name, struct inode *inode; inode_lock(d_inode(parent)); - dentry = lookup_one_len(name, parent, strlen(name)); + dentry = lookup_noperm(&QSTR(name), parent); if (IS_ERR(dentry)) { dentry = ERR_PTR(-ENOMEM); goto fail; diff --git a/drivers/android/binderfs.c b/drivers/android/binderfs.c index 94c6446604fc..98da8c4eea59 100644 --- a/drivers/android/binderfs.c +++ b/drivers/android/binderfs.c @@ -187,7 +187,7 @@ static int binderfs_binder_device_create(struct inode *ref_inode, inode_lock(d_inode(root)); /* look it up */ - dentry = lookup_one_len(name, root, name_len); + dentry = lookup_noperm(&QSTR(name), root); if (IS_ERR(dentry)) { inode_unlock(d_inode(root)); ret = PTR_ERR(dentry); @@ -487,7 +487,7 @@ static struct dentry *binderfs_create_dentry(struct dentry *parent, { struct dentry *dentry; - dentry = lookup_one_len(name, parent, strlen(name)); + dentry = lookup_noperm(&QSTR(name), parent); if (IS_ERR(dentry)) return dentry; diff --git a/drivers/infiniband/hw/qib/qib_fs.c b/drivers/infiniband/hw/qib/qib_fs.c index b9f4a2937c3a..2098de762bf5 100644 --- a/drivers/infiniband/hw/qib/qib_fs.c +++ b/drivers/infiniband/hw/qib/qib_fs.c @@ -90,7 +90,7 @@ static int create_file(const char *name, umode_t mode, int error; inode_lock(d_inode(parent)); - *dentry = lookup_one_len(name, parent, strlen(name)); + *dentry = lookup_noperm(&QSTR(name), parent); if (!IS_ERR(*dentry)) error = qibfs_mknod(d_inode(parent), *dentry, mode, fops, data); @@ -433,7 +433,7 @@ static int remove_device_files(struct super_block *sb, char unit[10]; snprintf(unit, sizeof(unit), "%u", dd->unit); - dir = lookup_one_len_unlocked(unit, sb->s_root, strlen(unit)); + dir = lookup_noperm_unlocked(&QSTR(unit), sb->s_root); if (IS_ERR(dir)) { pr_err("Lookup of %s failed\n", unit); diff --git a/fs/afs/dir.c b/fs/afs/dir.c index 9e7b1fe82c27..bfb69e066672 100644 --- a/fs/afs/dir.c +++ b/fs/afs/dir.c @@ -943,7 +943,7 @@ static struct dentry *afs_lookup_atsys(struct inode *dir, struct dentry *dentry) } strcpy(p, name); - ret = lookup_one_len(buf, dentry->d_parent, len); + ret = lookup_noperm(&QSTR(buf), dentry->d_parent); if (IS_ERR(ret) || d_is_positive(ret)) goto out_s; dput(ret); diff --git a/fs/afs/dir_silly.c b/fs/afs/dir_silly.c index a1e581946b93..0b80eb93fa40 100644 --- a/fs/afs/dir_silly.c +++ b/fs/afs/dir_silly.c @@ -113,16 +113,14 @@ int afs_sillyrename(struct afs_vnode *dvnode, struct afs_vnode *vnode, sdentry = NULL; do { - int slen; - dput(sdentry); sillycounter++; /* Create a silly name. Note that the ".__afs" prefix is * understood by the salvager and must not be changed. */ - slen = scnprintf(silly, sizeof(silly), ".__afs%04X", sillycounter); - sdentry = lookup_one_len(silly, dentry->d_parent, slen); + scnprintf(silly, sizeof(silly), ".__afs%04X", sillycounter); + sdentry = lookup_noperm(&QSTR(silly), dentry->d_parent); /* N.B. Better to return EBUSY here ... it could be dangerous * to delete the file while it's in use. diff --git a/fs/autofs/dev-ioctl.c b/fs/autofs/dev-ioctl.c index c5a6aae12d2c..d8dd150cbd74 100644 --- a/fs/autofs/dev-ioctl.c +++ b/fs/autofs/dev-ioctl.c @@ -459,7 +459,8 @@ static int autofs_dev_ioctl_timeout(struct file *fp, "the parent autofs mount timeout which could " "prevent shutdown\n"); - dentry = try_lookup_one_len(param->path, base, path_len); + dentry = try_lookup_noperm(&QSTR_LEN(param->path, path_len), + base); if (IS_ERR_OR_NULL(dentry)) return dentry ? PTR_ERR(dentry) : -ENOENT; ino = autofs_dentry_ino(dentry); diff --git a/fs/binfmt_misc.c b/fs/binfmt_misc.c index 5a7ebd160724..432fbf4fc334 100644 --- a/fs/binfmt_misc.c +++ b/fs/binfmt_misc.c @@ -842,7 +842,7 @@ static ssize_t bm_register_write(struct file *file, const char __user *buffer, } inode_lock(d_inode(root)); - dentry = lookup_one_len(e->name, root, strlen(e->name)); + dentry = lookup_noperm(&QSTR(e->name), root); err = PTR_ERR(dentry); if (IS_ERR(dentry)) goto out; diff --git a/fs/debugfs/inode.c b/fs/debugfs/inode.c index 75715d8877ee..30c4944e1862 100644 --- a/fs/debugfs/inode.c +++ b/fs/debugfs/inode.c @@ -346,7 +346,7 @@ struct dentry *debugfs_lookup(const char *name, struct dentry *parent) if (!parent) parent = debugfs_mount->mnt_root; - dentry = lookup_positive_unlocked(name, parent, strlen(name)); + dentry = lookup_noperm_positive_unlocked(&QSTR(name), parent); if (IS_ERR(dentry)) return NULL; return dentry; @@ -388,7 +388,7 @@ static struct dentry *start_creating(const char *name, struct dentry *parent) if (unlikely(IS_DEADDIR(d_inode(parent)))) dentry = ERR_PTR(-ENOENT); else - dentry = lookup_one_len(name, parent, strlen(name)); + dentry = lookup_noperm(&QSTR(name), parent); if (!IS_ERR(dentry) && d_really_is_positive(dentry)) { if (d_is_dir(dentry)) pr_err("Directory '%s' with parent '%s' already present!\n", @@ -872,7 +872,7 @@ int __printf(2, 3) debugfs_change_name(struct dentry *dentry, const char *fmt, . } if (strcmp(old_name.name.name, new_name) == 0) goto out; - target = lookup_one_len(new_name, parent, strlen(new_name)); + target = lookup_noperm(&QSTR(new_name), parent); if (IS_ERR(target)) { error = PTR_ERR(target); goto out; diff --git a/fs/ecryptfs/inode.c b/fs/ecryptfs/inode.c index 51a5c54eb740..493d7f194956 100644 --- a/fs/ecryptfs/inode.c +++ b/fs/ecryptfs/inode.c @@ -394,8 +394,8 @@ static struct dentry *ecryptfs_lookup(struct inode *ecryptfs_dir_inode, char *encrypted_and_encoded_name = NULL; struct ecryptfs_mount_crypt_stat *mount_crypt_stat; struct dentry *lower_dir_dentry, *lower_dentry; - const char *name = ecryptfs_dentry->d_name.name; - size_t len = ecryptfs_dentry->d_name.len; + struct qstr qname = QSTR_INIT(ecryptfs_dentry->d_name.name, + ecryptfs_dentry->d_name.len); struct dentry *res; int rc = 0; @@ -404,23 +404,25 @@ static struct dentry *ecryptfs_lookup(struct inode *ecryptfs_dir_inode, mount_crypt_stat = &ecryptfs_superblock_to_private( ecryptfs_dentry->d_sb)->mount_crypt_stat; if (mount_crypt_stat->flags & ECRYPTFS_GLOBAL_ENCRYPT_FILENAMES) { + size_t len = qname.len; rc = ecryptfs_encrypt_and_encode_filename( &encrypted_and_encoded_name, &len, - mount_crypt_stat, name, len); + mount_crypt_stat, qname.name, len); if (rc) { printk(KERN_ERR "%s: Error attempting to encrypt and encode " "filename; rc = [%d]\n", __func__, rc); return ERR_PTR(rc); } - name = encrypted_and_encoded_name; + qname.name = encrypted_and_encoded_name; + qname.len = len; } - lower_dentry = lookup_one_len_unlocked(name, lower_dir_dentry, len); + lower_dentry = lookup_noperm_unlocked(&qname, lower_dir_dentry); if (IS_ERR(lower_dentry)) { - ecryptfs_printk(KERN_DEBUG, "%s: lookup_one_len() returned " + ecryptfs_printk(KERN_DEBUG, "%s: lookup_noperm() returned " "[%ld] on lower_dentry = [%s]\n", __func__, PTR_ERR(lower_dentry), - name); + qname.name); res = ERR_CAST(lower_dentry); } else { res = ecryptfs_lookup_interpose(ecryptfs_dentry, lower_dentry); diff --git a/fs/kernfs/mount.c b/fs/kernfs/mount.c index 5124e196c2bf..a82fbce25c28 100644 --- a/fs/kernfs/mount.c +++ b/fs/kernfs/mount.c @@ -255,7 +255,7 @@ struct dentry *kernfs_node_dentry(struct kernfs_node *kn, dput(dentry); return ERR_PTR(-ENOMEM); } - dtmp = lookup_positive_unlocked(name, dentry, strlen(name)); + dtmp = lookup_noperm_positive_unlocked(&QSTR(name), dentry); dput(dentry); kfree(name); if (IS_ERR(dtmp)) diff --git a/fs/namei.c b/fs/namei.c index e499fb609271..f397ab5704e7 100644 --- a/fs/namei.c +++ b/fs/namei.c @@ -2834,9 +2834,9 @@ int vfs_path_lookup(struct dentry *dentry, struct vfsmount *mnt, } EXPORT_SYMBOL(vfs_path_lookup); -static int lookup_one_common(struct mnt_idmap *idmap, - const char *name, struct dentry *base, int len, - struct qstr *this) +static int lookup_noperm_common(const char *name, struct dentry *base, + int len, + struct qstr *this) { this->name = name; this->len = len; @@ -2861,50 +2861,58 @@ static int lookup_one_common(struct mnt_idmap *idmap, if (err < 0) return err; } + return 0; +} +static int lookup_one_common(struct mnt_idmap *idmap, + const char *name, struct dentry *base, int len, + struct qstr *this) { + int err; + err = lookup_noperm_common(name, base, len, this); + if (err < 0) + return err; return inode_permission(idmap, base->d_inode, MAY_EXEC); } /** - * try_lookup_one_len - filesystem helper to lookup single pathname component - * @name: pathname component to lookup + * try_lookup_noperm - filesystem helper to lookup single pathname component + * @name: qstr storing pathname component to lookup * @base: base directory to lookup from - * @len: maximum length @len should be interpreted to * * Look up a dentry by name in the dcache, returning NULL if it does not * currently exist. The function does not try to create a dentry. * * Note that this routine is purely a helper for filesystem usage and should - * not be called by generic code. + * not be called by generic code. It does no permission checking. * * No locks need be held - only a counted reference to @base is needed. * */ -struct dentry *try_lookup_one_len(const char *name, struct dentry *base, int len) +struct dentry *try_lookup_noperm(struct qstr *name, struct dentry *base) { struct qstr this; int err; - err = lookup_one_common(&nop_mnt_idmap, name, base, len, &this); + err = lookup_noperm_common(name->name, base, name->len, &this); if (err) return ERR_PTR(err); - return lookup_dcache(&this, base, 0); + name->hash = this.hash; + return lookup_dcache(name, base, 0); } -EXPORT_SYMBOL(try_lookup_one_len); +EXPORT_SYMBOL(try_lookup_noperm); /** - * lookup_one_len - filesystem helper to lookup single pathname component - * @name: pathname component to lookup + * lookup_noperm - filesystem helper to lookup single pathname component + * @name: qstr storing pathname component to lookup * @base: base directory to lookup from - * @len: maximum length @len should be interpreted to * * Note that this routine is purely a helper for filesystem usage and should - * not be called by generic code. + * not be called by generic code. It does no permission checking. * * The caller must hold base->i_mutex. */ -struct dentry *lookup_one_len(const char *name, struct dentry *base, int len) +struct dentry *lookup_noperm(struct qstr *name, struct dentry *base) { struct dentry *dentry; struct qstr this; @@ -2912,14 +2920,14 @@ struct dentry *lookup_one_len(const char *name, struct dentry *base, int len) WARN_ON_ONCE(!inode_is_locked(base->d_inode)); - err = lookup_one_common(&nop_mnt_idmap, name, base, len, &this); + err = lookup_noperm_common(name->name, base, name->len, &this); if (err) return ERR_PTR(err); dentry = lookup_dcache(&this, base, 0); return dentry ? dentry : __lookup_slow(&this, base, 0); } -EXPORT_SYMBOL(lookup_one_len); +EXPORT_SYMBOL(lookup_noperm); /** * lookup_one - lookup single pathname component @@ -2957,7 +2965,7 @@ EXPORT_SYMBOL(lookup_one); * * This can be used for in-kernel filesystem clients such as file servers. * - * Unlike lookup_one_len, it should be called without the parent + * Unlike lookup_one, it should be called without the parent * i_rwsem held, and will take the i_rwsem itself if necessary. */ struct dentry *lookup_one_unlocked(struct mnt_idmap *idmap, @@ -3010,39 +3018,48 @@ struct dentry *lookup_one_positive_unlocked(struct mnt_idmap *idmap, EXPORT_SYMBOL(lookup_one_positive_unlocked); /** - * lookup_one_len_unlocked - filesystem helper to lookup single pathname component + * lookup_noperm_unlocked - filesystem helper to lookup single pathname component * @name: pathname component to lookup * @base: base directory to lookup from - * @len: maximum length @len should be interpreted to * * Note that this routine is purely a helper for filesystem usage and should - * not be called by generic code. + * not be called by generic code. It does no permission checking. * - * Unlike lookup_one_len, it should be called without the parent - * i_mutex held, and will take the i_mutex itself if necessary. + * Unlike lookup_noperm, it should be called without the parent + * i_rwsem held, and will take the i_rwsem itself if necessary. */ -struct dentry *lookup_one_len_unlocked(const char *name, - struct dentry *base, int len) +struct dentry *lookup_noperm_unlocked(struct qstr *name, struct dentry *base) { - return lookup_one_unlocked(&nop_mnt_idmap, &QSTR_LEN(name, len), base); + struct dentry *ret; + + ret = try_lookup_noperm(name, base); + if (!ret) + ret = lookup_slow(name, base, 0); + return ret; } -EXPORT_SYMBOL(lookup_one_len_unlocked); +EXPORT_SYMBOL(lookup_noperm_unlocked); /* - * Like lookup_one_len_unlocked(), except that it yields ERR_PTR(-ENOENT) + * Like lookup_noperm_unlocked(), except that it yields ERR_PTR(-ENOENT) * on negatives. Returns known positive or ERR_PTR(); that's what * most of the users want. Note that pinned negative with unlocked parent - * _can_ become positive at any time, so callers of lookup_one_len_unlocked() + * _can_ become positive at any time, so callers of lookup_noperm_unlocked() * need to be very careful; pinned positives have ->d_inode stable, so * this one avoids such problems. */ -struct dentry *lookup_positive_unlocked(const char *name, - struct dentry *base, int len) +struct dentry *lookup_noperm_positive_unlocked(struct qstr *name, + struct dentry *base) { - return lookup_one_positive_unlocked(&nop_mnt_idmap, - &QSTR_LEN(name, len), base); + struct dentry *ret; + + ret = lookup_noperm_unlocked(name, base); + if (!IS_ERR(ret) && d_flags_negative(smp_load_acquire(&ret->d_flags))) { + dput(ret); + ret = ERR_PTR(-ENOENT); + } + return ret; } -EXPORT_SYMBOL(lookup_positive_unlocked); +EXPORT_SYMBOL(lookup_noperm_positive_unlocked); #ifdef CONFIG_UNIX98_PTYS int path_pts(struct path *path) diff --git a/fs/nfs/unlink.c b/fs/nfs/unlink.c index bf77399696a7..b55467911648 100644 --- a/fs/nfs/unlink.c +++ b/fs/nfs/unlink.c @@ -464,18 +464,17 @@ nfs_sillyrename(struct inode *dir, struct dentry *dentry) sdentry = NULL; do { - int slen; dput(sdentry); sillycounter++; - slen = scnprintf(silly, sizeof(silly), - SILLYNAME_PREFIX "%0*llx%0*x", - SILLYNAME_FILEID_LEN, fileid, - SILLYNAME_COUNTER_LEN, sillycounter); + scnprintf(silly, sizeof(silly), + SILLYNAME_PREFIX "%0*llx%0*x", + SILLYNAME_FILEID_LEN, fileid, + SILLYNAME_COUNTER_LEN, sillycounter); dfprintk(VFS, "NFS: trying to rename %pd to %s\n", dentry, silly); - sdentry = lookup_one_len(silly, dentry->d_parent, slen); + sdentry = lookup_noperm(&QSTR(silly), dentry->d_parent); /* * N.B. Better to return EBUSY here ... it could be * dangerous to delete the file while it's in use. diff --git a/fs/overlayfs/export.c b/fs/overlayfs/export.c index 444aeeccb6da..83f80fdb1567 100644 --- a/fs/overlayfs/export.c +++ b/fs/overlayfs/export.c @@ -385,11 +385,9 @@ static struct dentry *ovl_lookup_real_one(struct dentry *connected, */ take_dentry_name_snapshot(&name, real); /* - * No idmap handling here: it's an internal lookup. Could skip - * permission checking altogether, but for now just use non-idmap - * transformed ids. + * No idmap handling here: it's an internal lookup. */ - this = lookup_one_len(name.name.name, connected, name.name.len); + this = lookup_noperm(&name.name, connected); release_dentry_name_snapshot(&name); err = PTR_ERR(this); if (IS_ERR(this)) { diff --git a/fs/overlayfs/namei.c b/fs/overlayfs/namei.c index ac6e893e846a..bf722daf19a9 100644 --- a/fs/overlayfs/namei.c +++ b/fs/overlayfs/namei.c @@ -757,7 +757,7 @@ struct dentry *ovl_get_index_fh(struct ovl_fs *ofs, struct ovl_fh *fh) if (err) return ERR_PTR(err); - index = lookup_positive_unlocked(name.name, ofs->workdir, name.len); + index = lookup_noperm_positive_unlocked(&name, ofs->workdir); kfree(name.name); if (IS_ERR(index)) { if (PTR_ERR(index) == -ENOENT) diff --git a/fs/quota/dquot.c b/fs/quota/dquot.c index 825c5c2e0962..df4a9b348769 100644 --- a/fs/quota/dquot.c +++ b/fs/quota/dquot.c @@ -2560,7 +2560,7 @@ int dquot_quota_on_mount(struct super_block *sb, char *qf_name, struct dentry *dentry; int error; - dentry = lookup_positive_unlocked(qf_name, sb->s_root, strlen(qf_name)); + dentry = lookup_noperm_positive_unlocked(&QSTR(qf_name), sb->s_root); if (IS_ERR(dentry)) return PTR_ERR(dentry); diff --git a/fs/smb/client/cached_dir.c b/fs/smb/client/cached_dir.c index fe738623cf1b..e6fc92667d41 100644 --- a/fs/smb/client/cached_dir.c +++ b/fs/smb/client/cached_dir.c @@ -109,7 +109,8 @@ path_to_dentry(struct cifs_sb_info *cifs_sb, const char *path) while (*s && *s != sep) s++; - child = lookup_positive_unlocked(p, dentry, s - p); + child = lookup_noperm_positive_unlocked(&QSTR_LEN(p, s - p), + dentry); dput(dentry); dentry = child; } while (!IS_ERR(dentry)); @@ -207,7 +208,7 @@ replay_again: spin_unlock(&cfids->cfid_list_lock); /* - * Skip any prefix paths in @path as lookup_positive_unlocked() ends up + * Skip any prefix paths in @path as lookup_noperm_positive_unlocked() ends up * calling ->lookup() which already adds those through * build_path_from_dentry(). Also, do it earlier as we might reconnect * below when trying to send compounded request and then potentially diff --git a/fs/smb/client/cifsfs.c b/fs/smb/client/cifsfs.c index a08c42363ffc..fb04e263611c 100644 --- a/fs/smb/client/cifsfs.c +++ b/fs/smb/client/cifsfs.c @@ -929,7 +929,8 @@ cifs_get_root(struct smb3_fs_context *ctx, struct super_block *sb) while (*s && *s != sep) s++; - child = lookup_positive_unlocked(p, dentry, s - p); + child = lookup_noperm_positive_unlocked(&QSTR_LEN(p, s - p), + dentry); dput(dentry); dentry = child; } while (!IS_ERR(dentry)); diff --git a/fs/tracefs/inode.c b/fs/tracefs/inode.c index cb1af30b49f5..a3fd3cc591bd 100644 --- a/fs/tracefs/inode.c +++ b/fs/tracefs/inode.c @@ -555,7 +555,7 @@ struct dentry *tracefs_start_creating(const char *name, struct dentry *parent) if (unlikely(IS_DEADDIR(d_inode(parent)))) dentry = ERR_PTR(-ENOENT); else - dentry = lookup_one_len(name, parent, strlen(name)); + dentry = lookup_noperm(&QSTR(name), parent); if (!IS_ERR(dentry) && d_inode(dentry)) { dput(dentry); dentry = ERR_PTR(-EEXIST); diff --git a/fs/xfs/scrub/orphanage.c b/fs/xfs/scrub/orphanage.c index 3537f3cca6d5..475bb899b2c6 100644 --- a/fs/xfs/scrub/orphanage.c +++ b/fs/xfs/scrub/orphanage.c @@ -153,8 +153,7 @@ xrep_orphanage_create( /* Try to find the orphanage directory. */ inode_lock_nested(root_inode, I_MUTEX_PARENT); - orphanage_dentry = lookup_one_len(ORPHANAGE, root_dentry, - strlen(ORPHANAGE)); + orphanage_dentry = lookup_noperm(&QSTR(ORPHANAGE), root_dentry); if (IS_ERR(orphanage_dentry)) { error = PTR_ERR(orphanage_dentry); goto out_unlock_root; diff --git a/include/linux/namei.h b/include/linux/namei.h index ba02304a2a8a..cb6ce97782e0 100644 --- a/include/linux/namei.h +++ b/include/linux/namei.h @@ -69,10 +69,10 @@ int vfs_path_parent_lookup(struct filename *filename, unsigned int flags, int vfs_path_lookup(struct dentry *, struct vfsmount *, const char *, unsigned int, struct path *); -extern struct dentry *try_lookup_one_len(const char *, struct dentry *, int); -extern struct dentry *lookup_one_len(const char *, struct dentry *, int); -extern struct dentry *lookup_one_len_unlocked(const char *, struct dentry *, int); -extern struct dentry *lookup_positive_unlocked(const char *, struct dentry *, int); +extern struct dentry *try_lookup_noperm(struct qstr *, struct dentry *); +extern struct dentry *lookup_noperm(struct qstr *, struct dentry *); +extern struct dentry *lookup_noperm_unlocked(struct qstr *, struct dentry *); +extern struct dentry *lookup_noperm_positive_unlocked(struct qstr *, struct dentry *); struct dentry *lookup_one(struct mnt_idmap *, struct qstr *, struct dentry *); struct dentry *lookup_one_unlocked(struct mnt_idmap *idmap, struct qstr *name, struct dentry *base); diff --git a/ipc/mqueue.c b/ipc/mqueue.c index 35b4f8659904..82ed2d3c9846 100644 --- a/ipc/mqueue.c +++ b/ipc/mqueue.c @@ -913,7 +913,7 @@ static int do_mq_open(const char __user *u_name, int oflag, umode_t mode, ro = mnt_want_write(mnt); /* we'll drop it in any case */ inode_lock(d_inode(root)); - path.dentry = lookup_one_len(name->name, root, strlen(name->name)); + path.dentry = lookup_noperm(&QSTR(name->name), root); if (IS_ERR(path.dentry)) { error = PTR_ERR(path.dentry); goto out_putfd; @@ -969,8 +969,7 @@ SYSCALL_DEFINE1(mq_unlink, const char __user *, u_name) if (err) goto out_name; inode_lock_nested(d_inode(mnt->mnt_root), I_MUTEX_PARENT); - dentry = lookup_one_len(name->name, mnt->mnt_root, - strlen(name->name)); + dentry = lookup_noperm(&QSTR(name->name), mnt->mnt_root); if (IS_ERR(dentry)) { err = PTR_ERR(dentry); goto out_unlock; diff --git a/kernel/bpf/inode.c b/kernel/bpf/inode.c index dc3aa91a6ba0..5c2e96b19392 100644 --- a/kernel/bpf/inode.c +++ b/kernel/bpf/inode.c @@ -421,7 +421,7 @@ static int bpf_iter_link_pin_kernel(struct dentry *parent, int ret; inode_lock(parent->d_inode); - dentry = lookup_one_len(name, parent, strlen(name)); + dentry = lookup_noperm(&QSTR(name), parent); if (IS_ERR(dentry)) { inode_unlock(parent->d_inode); return PTR_ERR(dentry); diff --git a/security/apparmor/apparmorfs.c b/security/apparmor/apparmorfs.c index 6039afae4bfc..0aef34b9609b 100644 --- a/security/apparmor/apparmorfs.c +++ b/security/apparmor/apparmorfs.c @@ -283,7 +283,7 @@ static struct dentry *aafs_create(const char *name, umode_t mode, dir = d_inode(parent); inode_lock(dir); - dentry = lookup_one_len(name, parent, strlen(name)); + dentry = lookup_noperm(&QSTR(name), parent); if (IS_ERR(dentry)) { error = PTR_ERR(dentry); goto fail_lock; @@ -2551,7 +2551,7 @@ static int aa_mk_null_file(struct dentry *parent) return error; inode_lock(d_inode(parent)); - dentry = lookup_one_len(NULL_FILE_NAME, parent, strlen(NULL_FILE_NAME)); + dentry = lookup_noperm(&QSTR(NULL_FILE_NAME), parent); if (IS_ERR(dentry)) { error = PTR_ERR(dentry); goto out; diff --git a/security/inode.c b/security/inode.c index da3ab44c8e57..3913501621fa 100644 --- a/security/inode.c +++ b/security/inode.c @@ -128,7 +128,7 @@ static struct dentry *securityfs_create_dentry(const char *name, umode_t mode, dir = d_inode(parent); inode_lock(dir); - dentry = lookup_one_len(name, parent, strlen(name)); + dentry = lookup_noperm(&QSTR(name), parent); if (IS_ERR(dentry)) goto out; -- cgit v1.2.3 From 06c567403ae5a0b56005c2d4a184c903f572c844 Mon Sep 17 00:00:00 2001 From: NeilBrown Date: Wed, 19 Mar 2025 14:01:36 +1100 Subject: Use try_lookup_noperm() instead of d_hash_and_lookup() outside of VFS 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 Link: https://lore.kernel.org/r/20250319031545.2999807-6-neil@brown.name Signed-off-by: Christian Brauner --- Documentation/filesystems/porting.rst | 11 +++++++++++ fs/dcache.c | 1 - fs/efivarfs/super.c | 15 +++++---------- 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, 30 insertions(+), 24 deletions(-) (limited to 'include/linux') diff --git a/Documentation/filesystems/porting.rst b/Documentation/filesystems/porting.rst index 9150de7f64f1..3111ef5592f3 100644 --- a/Documentation/filesystems/porting.rst +++ b/Documentation/filesystems/porting.rst @@ -1232,3 +1232,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 bd5aa136153a..89f4acab08c0 100644 --- a/fs/dcache.c +++ b/fs/dcache.c @@ -2412,7 +2412,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 0486e9b68bc6..b2de4079864c 100644 --- a/fs/efivarfs/super.c +++ b/fs/efivarfs/super.c @@ -18,6 +18,7 @@ #include #include #include +#include #include "internal.h" @@ -204,7 +205,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 +217,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); @@ -404,8 +402,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; @@ -441,7 +439,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)) @@ -451,9 +448,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 b9b3e29a73fd..213bf3226213 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 b0d4e1908b22..fe33a5843fbd 100644 --- a/fs/proc/base.c +++ b/fs/proc/base.c @@ -2121,7 +2121,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 +#include #include #include #include @@ -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 475bb899b2c6..9c12cb844231 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 e974e63bcdbc..a324f82df562 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); -- cgit v1.2.3