Unverified Commit fa6fe07d authored by NeilBrown's avatar NeilBrown Committed by Christian Brauner
Browse files

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: default avatarNeilBrown <neil@brown.name>
Link: https://lore.kernel.org/r/20250319031545.2999807-5-neil@brown.name


Signed-off-by: default avatarChristian Brauner <brauner@kernel.org>
parent 2011067c

Documentation/filesystems/porting.rst

+20 −0
Original line number Diff line number Diff line
@@ -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()

arch/s390/hypfs/inode.c

+1 −1
Original line number Diff line number Diff line
@@ -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;

drivers/android/binderfs.c

+2 −2
Original line number Diff line number Diff line
@@ -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;

drivers/infiniband/hw/qib/qib_fs.c

+2 −2
Original line number Diff line number Diff line
@@ -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);

fs/afs/dir.c

+1 −1
Original line number Diff line number Diff line
@@ -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);