Unverified Commit 3789a0ab authored by Christian Brauner's avatar Christian Brauner
Browse files

Merge patch series "VFS: change kern_path_locked() and user_path_locked_at()... 

Merge patch series "VFS: change kern_path_locked() and user_path_locked_at() to never return negative dentry"

NeilBrown <neilb@suse.de> says:

I found these opportunities for simplification as part of my work to
enhance filesystem directory operations to not require an exclusive
lock on the directory.
There are quite a collection of users of these interfaces incluing NFS,
smb/server, bcachefs, devtmpfs, and audit.  Hence the long Cc line.

* patches from https://lore.kernel.org/r/20250217003020.3170652-2-neilb@suse.de:
  VFS: add common error checks to lookup_one_qstr_excl()
  VFS: change kern_path_locked() and user_path_locked_at() to never return negative dentry

Link: https://lore.kernel.org/r/20250217003020.3170652-2-neilb@suse.de


Signed-off-by: default avatarChristian Brauner <brauner@kernel.org>
parents 2c3230fb 204a575e

Documentation/filesystems/porting.rst

+21 −0
Original line number Diff line number Diff line
@@ -1157,3 +1157,24 @@ in normal case it points into the pathname being looked up. 
NOTE: if you need something like full path from the root of filesystem,

you are still on your own - this assists with simple cases, but it's not

magic.



---



** recommended**



kern_path_locked() and user_path_locked() no longer return a negative

dentry so this doesn't need to be checked.  If the name cannot be found,

ERR_PTR(-ENOENT) is returned.



** recommend**



lookup_one_qstr_excl() is changed to return errors in more cases, so

these conditions don't require explicit checks:



 - if LOOKUP_CREATE is NOT given, then the dentry won't be negative,

   ERR_PTR(-ENOENT) is returned instead

 - if LOOKUP_EXCL IS given, then the dentry won't be positive,

   ERR_PTR(-EEXIST) is rreturned instread



LOOKUP_EXCL now means "target must not exist".  It can be combined with

LOOK_CREATE or LOOKUP_RENAME_TARGET.

drivers/base/devtmpfs.c

+30 −35
Original line number Diff line number Diff line
@@ -245,15 +245,12 @@ static int dev_rmdir(const char *name) 
	dentry = kern_path_locked(name, &parent);

	if (IS_ERR(dentry))

		return PTR_ERR(dentry);

	if (d_really_is_positive(dentry)) {

	if (d_inode(dentry)->i_private == &thread)

		err = vfs_rmdir(&nop_mnt_idmap, d_inode(parent.dentry),

				dentry);

	else

		err = -EPERM;

	} else {

		err = -ENOENT;

	}



	dput(dentry);

	inode_unlock(d_inode(parent.dentry));

	path_put(&parent);
@@ -310,6 +307,8 @@ static int handle_remove(const char *nodename, struct device *dev) 
{

	struct path parent;

	struct dentry *dentry;

	struct kstat stat;

	struct path p;

	int deleted = 0;

	int err;
@@ -317,9 +316,8 @@ static int handle_remove(const char *nodename, struct device *dev) 
	if (IS_ERR(dentry))

		return PTR_ERR(dentry);



	if (d_really_is_positive(dentry)) {

		struct kstat stat;

		struct path p = {.mnt = parent.mnt, .dentry = dentry};

	p.mnt = parent.mnt;

	p.dentry = dentry;

	err = vfs_getattr(&p, &stat, STATX_TYPE | STATX_MODE,

			  AT_STATX_SYNC_AS_STAT);

	if (!err && dev_mynode(dev, d_inode(dentry), &stat)) {
@@ -341,9 +339,6 @@ static int handle_remove(const char *nodename, struct device *dev) 
		if (!err || err == -ENOENT)

			deleted = 1;

	}

	} else {

		err = -ENOENT;

	}

	dput(dentry);

	inode_unlock(d_inode(parent.dentry));

fs/bcachefs/fs-ioctl.c

+0 −4
Original line number Diff line number Diff line
@@ -511,10 +511,6 @@ static long bch2_ioctl_subvolume_destroy(struct bch_fs *c, struct file *filp, 
		ret = -EXDEV;

		goto err;

	}

	if (!d_is_positive(victim)) {

		ret = -ENOENT;

		goto err;

	}

	ret = __bch2_unlink(dir, victim, true);

	if (!ret) {

		fsnotify_rmdir(dir, victim);

fs/namei.c

+21 −36
Original line number Diff line number Diff line
@@ -1670,6 +1670,8 @@ static struct dentry *lookup_dcache(const struct qstr *name, 
 * dentries - as the matter of fact, this only gets called

 * when directory is guaranteed to have no in-lookup children

 * at all.

 * Will return -ENOENT if name isn't found and LOOKUP_CREATE wasn't passed.

 * Will return -EEXIST if name is found and LOOKUP_EXCL was passed.

 */

struct dentry *lookup_one_qstr_excl(const struct qstr *name,

				    struct dentry *base,
@@ -1680,7 +1682,7 @@ struct dentry *lookup_one_qstr_excl(const struct qstr *name, 
	struct inode *dir = base->d_inode;



	if (dentry)

		return dentry;

		goto found;



	/* Don't create child dentry for a dead directory. */

	if (unlikely(IS_DEADDIR(dir)))
@@ -1695,6 +1697,17 @@ struct dentry *lookup_one_qstr_excl(const struct qstr *name, 
		dput(dentry);

		dentry = old;

	}

found:

	if (IS_ERR(dentry))

		return dentry;

	if (d_is_negative(dentry) && !(flags & LOOKUP_CREATE)) {

		dput(dentry);

		return ERR_PTR(-ENOENT);

	}

	if (d_is_positive(dentry) && (flags & LOOKUP_EXCL)) {

		dput(dentry);

		return ERR_PTR(-EEXIST);

	}

	return dentry;

}

EXPORT_SYMBOL(lookup_one_qstr_excl);
@@ -4078,27 +4091,13 @@ static struct dentry *filename_create(int dfd, struct filename *name, 
	 * '/', and a directory wasn't requested.

	 */

	if (last.name[last.len] && !want_dir)

		create_flags = 0;

		create_flags &= ~LOOKUP_CREATE;

	inode_lock_nested(path->dentry->d_inode, I_MUTEX_PARENT);

	dentry = lookup_one_qstr_excl(&last, path->dentry,

				      reval_flag | create_flags);

	if (IS_ERR(dentry))

		goto unlock;



	error = -EEXIST;

	if (d_is_positive(dentry))

		goto fail;



	/*

	 * Special case - lookup gave negative, but... we had foo/bar/

	 * From the vfs_mknod() POV we just have a negative dentry -

	 * all is fine. Let's be bastards - you had / on the end, you've

	 * been asking for (non-existent) directory. -ENOENT for you.

	 */

	if (unlikely(!create_flags)) {

		error = -ENOENT;

		goto fail;

	}

	if (unlikely(err2)) {

		error = err2;

		goto fail;
@@ -4445,10 +4444,6 @@ int do_rmdir(int dfd, struct filename *name) 
	error = PTR_ERR(dentry);

	if (IS_ERR(dentry))

		goto exit3;

	if (!dentry->d_inode) {

		error = -ENOENT;

		goto exit4;

	}

	error = security_path_rmdir(&path, dentry);

	if (error)

		goto exit4;
@@ -4579,7 +4574,7 @@ int do_unlinkat(int dfd, struct filename *name) 
	if (!IS_ERR(dentry)) {



		/* Why not before? Because we want correct error value */

		if (last.name[last.len] || d_is_negative(dentry))

		if (last.name[last.len])

			goto slashes;

		inode = dentry->d_inode;

		ihold(inode);
@@ -4613,9 +4608,7 @@ int do_unlinkat(int dfd, struct filename *name) 
	return error;



slashes:

	if (d_is_negative(dentry))

		error = -ENOENT;

	else if (d_is_dir(dentry))

	if (d_is_dir(dentry))

		error = -EISDIR;

	else

		error = -ENOTDIR;
@@ -5115,7 +5108,8 @@ int do_renameat2(int olddfd, struct filename *from, int newdfd, 
	struct qstr old_last, new_last;

	int old_type, new_type;

	struct inode *delegated_inode = NULL;

	unsigned int lookup_flags = 0, target_flags = LOOKUP_RENAME_TARGET;

	unsigned int lookup_flags = 0, target_flags =

		LOOKUP_RENAME_TARGET | LOOKUP_CREATE;

	bool should_retry = false;

	int error = -EINVAL;
@@ -5128,6 +5122,8 @@ int do_renameat2(int olddfd, struct filename *from, int newdfd, 


	if (flags & RENAME_EXCHANGE)

		target_flags = 0;

	if (flags & RENAME_NOREPLACE)

		target_flags |= LOOKUP_EXCL;



retry:

	error = filename_parentat(olddfd, from, lookup_flags, &old_path,
@@ -5169,23 +5165,12 @@ int do_renameat2(int olddfd, struct filename *from, int newdfd, 
	error = PTR_ERR(old_dentry);

	if (IS_ERR(old_dentry))

		goto exit3;

	/* source must exist */

	error = -ENOENT;

	if (d_is_negative(old_dentry))

		goto exit4;

	new_dentry = lookup_one_qstr_excl(&new_last, new_path.dentry,

					  lookup_flags | target_flags);

	error = PTR_ERR(new_dentry);

	if (IS_ERR(new_dentry))

		goto exit4;

	error = -EEXIST;

	if ((flags & RENAME_NOREPLACE) && d_is_positive(new_dentry))

		goto exit5;

	if (flags & RENAME_EXCHANGE) {

		error = -ENOENT;

		if (d_is_negative(new_dentry))

			goto exit5;



		if (!d_is_dir(new_dentry)) {

			error = -ENOTDIR;

			if (new_last.name[new_last.len])

fs/nfs/dir.c

+2 −1
Original line number Diff line number Diff line
@@ -1532,7 +1532,8 @@ static int nfs_is_exclusive_create(struct inode *dir, unsigned int flags) 
{

	if (NFS_PROTO(dir)->version == 2)

		return 0;

	return flags & LOOKUP_EXCL;

	return (flags & (LOOKUP_CREATE | LOOKUP_EXCL)) ==

		(LOOKUP_CREATE | LOOKUP_EXCL);

}



/*