[PATCH v3 0/3] VFS: refactor lookup_open and add vfs_lookup()

[PATCH v3 0/3] VFS: refactor lookup_open and add vfs_lookup()

[NeilBrown]

This v3 changes vfs_lookup_open() to return errors if a non-regular
files was found, rather than trying to open it:
  -EISDIR, -ELOOP, -ENODEV, -EFTYPE.

If this can land in a stable branch, I can ask nfsd maintainers to merge
it with the corresponding nfsd changes.

Original series description:

My recent proposal for inverting the order between inode_lock() on a
parent dir and d_alloc_parallel() missed that fact that atomic_open()
has two callers, and would have resulted in easy deadlocks from nfsd
when re-exporting and NFS filesystem.

I think the best way to fix this is to provide a richer interface for
nfsd to use, which includes all the locking as well as inode_operations
calls.  This allows the nfsd behaviour to share more code with the
system-call behaviour.

This series refactors code between lookup_open() and
open_last_lookups(), and uses the new lookup_open() to provide
vfs_lookup_open().

 [PATCH v3 1/3] VFS: move mnt_want_write() and locking into
 [PATCH v3 2/3] VFS: move delegated_inode retry loop into
 [PATCH v3 3/3] VFS: add vfs_lookup_open() for nfsd

Thanks,
NeilBrown

[PATCH v3 1/3] VFS: move mnt_want_write() and locking into lookup_open()

[NeilBrown]

From: NeilBrown <neil@brown.name>

The mnt_want_write() call and the parent inode locking in
open_last_lookups() are only needed for lookup_open().  So we can move
them and all the got_write handling into lookup_open().

Note that we need to also check create_error when determining whether to
unlock shared or not, as O_CREAT can be cleared, but create_error is
only set of O_CREAT was set.

The fsnotify calls come too as they must be in the locked region.

Also use the existing dir_inode uniformly for dir->d_inode.

This is a step towards exporting an better "open/create" interface to nfsd.

Reviewed-by: Jan Kara <jack@suse.cz>
Reviewed-by: Jori Koolstra <jkoolstra@xs4all.nl>
Signed-off-by: NeilBrown <neil@brown.name>
---
 fs/namei.c | 77 +++++++++++++++++++++++++++++-------------------------
 1 file changed, 41 insertions(+), 36 deletions(-)

diff --git a/fs/namei.c b/fs/namei.c
index 7ead3db55be2..55404fe0e269 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -4398,7 +4398,7 @@ static struct dentry *atomic_open(const struct path *path, struct dentry *dentry
  */
 static struct dentry *lookup_open(struct nameidata *nd, struct file *file,
 				  const struct open_flags *op,
-				  bool got_write, struct delegated_inode *delegated_inode)
+				  struct delegated_inode *delegated_inode)
 {
 	struct mnt_idmap *idmap;
 	struct dentry *dir = nd->path.dentry;
@@ -4407,9 +4407,25 @@ static struct dentry *lookup_open(struct nameidata *nd, struct file *file,
 	struct dentry *dentry;
 	int error, create_error = 0;
 	umode_t mode = op->mode;
+	bool got_write = false;
 
-	if (unlikely(IS_DEADDIR(dir_inode)))
-		return ERR_PTR(-ENOENT);
+	if (open_flag & (O_CREAT | O_TRUNC | O_WRONLY | O_RDWR)) {
+		got_write = !mnt_want_write(nd->path.mnt);
+		/*
+		 * do _not_ fail yet - we might not need that or fail with
+		 * a different error; let lookup_open() decide; we'll be
+		 * dropping this one anyway.
+		 */
+	}
+	if (open_flag & O_CREAT)
+		inode_lock(dir_inode);
+	else
+		inode_lock_shared(dir_inode);
+
+	if (unlikely(IS_DEADDIR(dir_inode))) {
+		dentry = ERR_PTR(-ENOENT);
+		goto out;
+	}
 
 	file->f_mode &= ~FMODE_CREATED;
 	dentry = d_lookup(dir, &nd->last);
@@ -4417,7 +4433,7 @@ static struct dentry *lookup_open(struct nameidata *nd, struct file *file,
 		if (!dentry) {
 			dentry = d_alloc_parallel(dir, &nd->last);
 			if (IS_ERR(dentry))
-				return dentry;
+				goto out;
 		}
 		if (d_in_lookup(dentry))
 			break;
@@ -4433,7 +4449,7 @@ static struct dentry *lookup_open(struct nameidata *nd, struct file *file,
 	}
 	if (dentry->d_inode) {
 		/* Cached positive dentry: will open in f_op->open */
-		return dentry;
+		goto out;
 	}
 
 	if (open_flag & O_CREAT)
@@ -4454,7 +4470,7 @@ static struct dentry *lookup_open(struct nameidata *nd, struct file *file,
 	if (open_flag & O_CREAT) {
 		if (open_flag & O_EXCL)
 			open_flag &= ~O_TRUNC;
-		mode = vfs_prepare_mode(idmap, dir->d_inode, mode, mode, mode);
+		mode = vfs_prepare_mode(idmap, dir_inode, mode, mode, mode);
 		if (likely(got_write))
 			create_error = may_o_create(idmap, &nd->path,
 						    dentry, mode);
@@ -4469,7 +4485,7 @@ static struct dentry *lookup_open(struct nameidata *nd, struct file *file,
 		dentry = atomic_open(&nd->path, dentry, file, open_flag, mode);
 		if (unlikely(create_error) && dentry == ERR_PTR(-ENOENT))
 			dentry = ERR_PTR(create_error);
-		return dentry;
+		goto out;
 	}
 
 	if (d_in_lookup(dentry)) {
@@ -4508,11 +4524,27 @@ static struct dentry *lookup_open(struct nameidata *nd, struct file *file,
 		error = create_error;
 		goto out_dput;
 	}
+out:
+	if (!IS_ERR(dentry)) {
+		if (file->f_mode & FMODE_CREATED)
+			fsnotify_create(dir_inode, dentry);
+		if (file->f_mode & FMODE_OPENED)
+			fsnotify_open(file);
+	}
+	if ((open_flag & O_CREAT) || create_error)
+		inode_unlock(dir_inode);
+	else
+		inode_unlock_shared(dir_inode);
+
+	if (got_write)
+		mnt_drop_write(nd->path.mnt);
+
 	return dentry;
 
 out_dput:
 	dput(dentry);
-	return ERR_PTR(error);
+	dentry = ERR_PTR(error);
+	goto out;
 }
 
 static inline bool trailing_slashes(struct nameidata *nd)
@@ -4555,9 +4587,7 @@ static const char *open_last_lookups(struct nameidata *nd,
 		   struct file *file, const struct open_flags *op)
 {
 	struct delegated_inode delegated_inode = { };
-	struct dentry *dir = nd->path.dentry;
 	int open_flag = op->open_flag;
-	bool got_write = false;
 	struct dentry *dentry;
 	const char *res;
 
@@ -4587,32 +4617,7 @@ static const char *open_last_lookups(struct nameidata *nd,
 		}
 	}
 retry:
-	if (open_flag & (O_CREAT | O_TRUNC | O_WRONLY | O_RDWR)) {
-		got_write = !mnt_want_write(nd->path.mnt);
-		/*
-		 * do _not_ fail yet - we might not need that or fail with
-		 * a different error; let lookup_open() decide; we'll be
-		 * dropping this one anyway.
-		 */
-	}
-	if (open_flag & O_CREAT)
-		inode_lock(dir->d_inode);
-	else
-		inode_lock_shared(dir->d_inode);
-	dentry = lookup_open(nd, file, op, got_write, &delegated_inode);
-	if (!IS_ERR(dentry)) {
-		if (file->f_mode & FMODE_CREATED)
-			fsnotify_create(dir->d_inode, dentry);
-		if (file->f_mode & FMODE_OPENED)
-			fsnotify_open(file);
-	}
-	if (open_flag & O_CREAT)
-		inode_unlock(dir->d_inode);
-	else
-		inode_unlock_shared(dir->d_inode);
-
-	if (got_write)
-		mnt_drop_write(nd->path.mnt);
+	dentry = lookup_open(nd, file, op, &delegated_inode);
 
 	if (IS_ERR(dentry)) {
 		if (is_delegated(&delegated_inode)) {
-- 
2.50.0.107.gf914562f5916.dirty

[PATCH v3 2/3] VFS: move delegated_inode retry loop into lookup_open()

[NeilBrown]

From: NeilBrown <neil@brown.name>

By moving this retry into lookup_open() we no longer need to pass around
the delegated_inode pointer.

Various variable assignments need to be moved out of the declaration
block so that they can be repeated after the "goto retry".

Reviewed-by: Jan Kara <jack@suse.cz>
Reviewed-by: Jori Koolstra <jkoolstra@xs4all.nl>
Signed-off-by: NeilBrown <neil@brown.name>
---
 fs/namei.c | 43 ++++++++++++++++++++++++-------------------
 1 file changed, 24 insertions(+), 19 deletions(-)

diff --git a/fs/namei.c b/fs/namei.c
index 55404fe0e269..cb33a01d16eb 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -4397,17 +4397,23 @@ static struct dentry *atomic_open(const struct path *path, struct dentry *dentry
  * An error code is returned on failure.
  */
 static struct dentry *lookup_open(struct nameidata *nd, struct file *file,
-				  const struct open_flags *op,
-				  struct delegated_inode *delegated_inode)
+				  const struct open_flags *op)
 {
+	struct delegated_inode delegated_inode = { };
 	struct mnt_idmap *idmap;
 	struct dentry *dir = nd->path.dentry;
 	struct inode *dir_inode = dir->d_inode;
-	int open_flag = op->open_flag;
+	int open_flag;
 	struct dentry *dentry;
-	int error, create_error = 0;
-	umode_t mode = op->mode;
-	bool got_write = false;
+	int error, create_error;
+	umode_t mode;
+	bool got_write;
+
+retry:
+	open_flag = op->open_flag;
+	got_write = false;
+	mode = op->mode;
+	create_error = 0;
 
 	if (open_flag & (O_CREAT | O_TRUNC | O_WRONLY | O_RDWR)) {
 		got_write = !mnt_want_write(nd->path.mnt);
@@ -4505,7 +4511,7 @@ static struct dentry *lookup_open(struct nameidata *nd, struct file *file,
 	/* Negative dentry, just create the file */
 	if (!dentry->d_inode && (open_flag & O_CREAT)) {
 		/* but break the directory lease first! */
-		error = try_break_deleg(dir_inode, LEASE_BREAK_DIR_CREATE, delegated_inode);
+		error = try_break_deleg(dir_inode, LEASE_BREAK_DIR_CREATE, &delegated_inode);
 		if (error)
 			goto out_dput;
 
@@ -4539,6 +4545,15 @@ static struct dentry *lookup_open(struct nameidata *nd, struct file *file,
 	if (got_write)
 		mnt_drop_write(nd->path.mnt);
 
+	if (is_delegated(&delegated_inode)) {
+		/* Must have come through out_dput: dentry is an ERR_PTR() */
+		error = break_deleg_wait(&delegated_inode);
+
+		if (!error)
+			goto retry;
+		dentry = ERR_PTR(error);
+	}
+
 	return dentry;
 
 out_dput:
@@ -4586,7 +4601,6 @@ static struct dentry *lookup_fast_for_open(struct nameidata *nd, int open_flag)
 static const char *open_last_lookups(struct nameidata *nd,
 		   struct file *file, const struct open_flags *op)
 {
-	struct delegated_inode delegated_inode = { };
 	int open_flag = op->open_flag;
 	struct dentry *dentry;
 	const char *res;
@@ -4616,19 +4630,10 @@ static const char *open_last_lookups(struct nameidata *nd,
 				return ERR_PTR(-ECHILD);
 		}
 	}
-retry:
-	dentry = lookup_open(nd, file, op, &delegated_inode);
-
-	if (IS_ERR(dentry)) {
-		if (is_delegated(&delegated_inode)) {
-			int error = break_deleg_wait(&delegated_inode);
 
-			if (!error)
-				goto retry;
-			return ERR_PTR(error);
-		}
+	dentry = lookup_open(nd, file, op);
+	if (IS_ERR(dentry))
 		return ERR_CAST(dentry);
-	}
 
 	if (file->f_mode & (FMODE_OPENED | FMODE_CREATED)) {
 		dput(nd->path.dentry);
-- 
2.50.0.107.gf914562f5916.dirty

[PATCH v3 3/3] VFS: add vfs_lookup_open() for nfsd

[NeilBrown]

From: NeilBrown <neil@brown.name>

vfs_lookup_open() is a limited version of lookup_open() which is
exported for nfsd to use - to replace dentry_create().

It is limited in that no filename is given (thus no auditing) and no
LOOKUP_ flags are passed.  A few "intent" LOOKUP flags are deduced from
the open flags.

If a non-regular file is found, an O_PATH file is returned.  This allows
the caller to see what was found and to return an appropriate error
code.  However if ->atomic_open was used, it may return an error rather
then provide a dentry.

Signed-off-by: NeilBrown <neil@brown.name>
---
 fs/namei.c            | 89 +++++++++++++++++++++++++++++++++++++++++++
 include/linux/namei.h |  3 ++
 2 files changed, 92 insertions(+)

diff --git a/fs/namei.c b/fs/namei.c
index cb33a01d16eb..2143f759443a 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -4562,6 +4562,95 @@ static struct dentry *lookup_open(struct nameidata *nd, struct file *file,
 	goto out;
 }
 
+/**
+ * vfs_lookup_open - open and possibly create a regular file
+ * @parent: directory to contain file
+ * @last: final component of file name
+ * @open_flag: O_flags
+ * @mode: initial permissions for file
+ *
+ * Open a file after lookup and/or create.  This provides similar
+ * functionality open_last_lookups() for non-VFS users, particularly
+ * nfsd.
+ * It uses ->atomic_open or ->lookup / ->create / ->open as appropriate.
+ *
+ * If the fs object found is not a regular file then an error is returned.
+ * In some cases, related errors are repurposed to so that the caller can
+ * determine the file of file found from the error.
+ * -EISDIR : a directory was found
+ * -ELOOP  : a symlink was found
+ * -ENODEV : a block or character device special file was found
+ * -EFTYPE : any other non-regular file was found, such as FIFO or SOCK.
+ *
+ * Returns: the opened struct file, or an error.
+ */
+struct file *vfs_lookup_open(struct path *parent, struct qstr *last,
+			     int open_flag, umode_t mode)
+{
+	struct file *file __free(fput) = NULL;
+	struct nameidata nd = {};
+	struct open_flags op = {};
+	struct dentry *dentry;
+	int error = 0;
+
+	WARN_ONCE(mode & ~S_IALLUGO, "mode must only have permission bits");
+	error = lookup_noperm_common(last, parent->dentry);
+	if (error)
+		return ERR_PTR(error);
+
+	file = alloc_empty_file(open_flag, current_cred());
+	if (IS_ERR(file))
+		return file;
+
+	nd.path = *parent;
+	nd.last = *last;
+	nd.flags = LOOKUP_OPEN;
+	if (open_flag & O_CREAT) {
+		nd.flags |= LOOKUP_CREATE;
+		if (open_flag & O_EXCL)
+			nd.flags |= LOOKUP_EXCL;
+	}
+	op.open_flag = open_flag;
+	op.mode = S_IFREG | mode;
+	dentry = lookup_open(&nd, file, &op);
+
+	if (IS_ERR(dentry))
+		return ERR_CAST(dentry);
+
+	if (d_really_is_negative(dentry)) {
+		error = -ENOENT;
+	} else if (!(file->f_mode & FMODE_CREATED) && (open_flag & O_EXCL)) {
+		error = -EEXIST;
+	} else if ((dentry->d_inode->i_mode & S_IFMT) != S_IFREG) {
+		switch (dentry->d_inode->i_mode & S_IFMT) {
+		case S_IFDIR:
+			error = -EISDIR;
+			break;
+		case S_IFLNK:
+			error = -ELOOP;
+			break;
+		case S_IFBLK:
+		case S_IFCHR:
+			error = -ENODEV;
+			break;
+		case S_IFIFO:
+		case S_IFSOCK:
+		default:
+			error = -EFTYPE;
+			break;
+		}
+	} else if (!(file->f_mode & FMODE_OPENED)) {
+		nd.path.dentry = dentry;
+		error = vfs_open(&nd.path, file);
+	}
+	dput(dentry);
+
+	if (error)
+		return ERR_PTR(error);
+	return no_free_ptr(file);
+}
+EXPORT_SYMBOL_FOR_MODULES(vfs_lookup_open, "nfsd");
+
 static inline bool trailing_slashes(struct nameidata *nd)
 {
 	return (bool)nd->last.name[nd->last.len];
diff --git a/include/linux/namei.h b/include/linux/namei.h
index ebe6e29f7e93..86d657b24fc6 100644
--- a/include/linux/namei.h
+++ b/include/linux/namei.h
@@ -97,6 +97,9 @@ struct dentry *start_creating_dentry(struct dentry *parent,
 struct dentry *start_removing_dentry(struct dentry *parent,
 				     struct dentry *child);
 
+struct file *vfs_lookup_open(struct path *parent, struct qstr *last,
+			     int open_flag, umode_t mode);
+
 /* end_creating - finish action started with start_creating
  * @child: dentry returned by start_creating() or vfs_mkdir()
  *
-- 
2.50.0.107.gf914562f5916.dirty

Re: [PATCH v3 3/3] VFS: add vfs_lookup_open() for nfsd

[Jori Koolstra]

> Op 13-07-2026 08:05 CEST schreef NeilBrown <neilb@ownmail.net>:
> 
>  
> From: NeilBrown <neil@brown.name>
> 
> vfs_lookup_open() is a limited version of lookup_open() which is
> exported for nfsd to use - to replace dentry_create().
> 
> It is limited in that no filename is given (thus no auditing) and no
> LOOKUP_ flags are passed.  A few "intent" LOOKUP flags are deduced from
> the open flags.
> 
> If a non-regular file is found, an O_PATH file is returned.  This allows
> the caller to see what was found and to return an appropriate error
> code.  However if ->atomic_open was used, it may return an error rather
> then provide a dentry.

Stale commit paragraph.

> 
> Signed-off-by: NeilBrown <neil@brown.name>
> ---
>  fs/namei.c            | 89 +++++++++++++++++++++++++++++++++++++++++++
>  include/linux/namei.h |  3 ++
>  2 files changed, 92 insertions(+)
> 
> diff --git a/fs/namei.c b/fs/namei.c
> index cb33a01d16eb..2143f759443a 100644
> --- a/fs/namei.c
> +++ b/fs/namei.c
> @@ -4562,6 +4562,95 @@ static struct dentry *lookup_open(struct nameidata *nd, struct file *file,
>  	goto out;
>  }
>  
> +/**
> + * vfs_lookup_open - open and possibly create a regular file
> + * @parent: directory to contain file
> + * @last: final component of file name
> + * @open_flag: O_flags
> + * @mode: initial permissions for file
> + *
> + * Open a file after lookup and/or create.  This provides similar
> + * functionality open_last_lookups() for non-VFS users, particularly
> + * nfsd.
> + * It uses ->atomic_open or ->lookup / ->create / ->open as appropriate.
> + *
> + * If the fs object found is not a regular file then an error is returned.
> + * In some cases, related errors are repurposed to so that the caller can
> + * determine the file of file found from the error.
> + * -EISDIR : a directory was found
> + * -ELOOP  : a symlink was found
> + * -ENODEV : a block or character device special file was found
> + * -EFTYPE : any other non-regular file was found, such as FIFO or SOCK.
> + *
> + * Returns: the opened struct file, or an error.
> + */
> +struct file *vfs_lookup_open(struct path *parent, struct qstr *last,
> +			     int open_flag, umode_t mode)
> +{
> +	struct file *file __free(fput) = NULL;
> +	struct nameidata nd = {};
> +	struct open_flags op = {};
> +	struct dentry *dentry;
> +	int error = 0;
> +
> +	WARN_ONCE(mode & ~S_IALLUGO, "mode must only have permission bits");
> +	error = lookup_noperm_common(last, parent->dentry);
> +	if (error)
> +		return ERR_PTR(error);
> +
> +	file = alloc_empty_file(open_flag, current_cred());
> +	if (IS_ERR(file))
> +		return file;
> +
> +	nd.path = *parent;
> +	nd.last = *last;
> +	nd.flags = LOOKUP_OPEN;
> +	if (open_flag & O_CREAT) {
> +		nd.flags |= LOOKUP_CREATE;
> +		if (open_flag & O_EXCL)
> +			nd.flags |= LOOKUP_EXCL;
> +	}
> +	op.open_flag = open_flag;
> +	op.mode = S_IFREG | mode;
> +	dentry = lookup_open(&nd, file, &op);
> +
> +	if (IS_ERR(dentry))
> +		return ERR_CAST(dentry);
> +
> +	if (d_really_is_negative(dentry)) {
> +		error = -ENOENT;
> +	} else if (!(file->f_mode & FMODE_CREATED) && (open_flag & O_EXCL)) {
> +		error = -EEXIST;
> +	} else if ((dentry->d_inode->i_mode & S_IFMT) != S_IFREG) {
> +		switch (dentry->d_inode->i_mode & S_IFMT) {
> +		case S_IFDIR:
> +			error = -EISDIR;
> +			break;
> +		case S_IFLNK:
> +			error = -ELOOP;
> +			break;
> +		case S_IFBLK:
> +		case S_IFCHR:
> +			error = -ENODEV;
> +			break;
> +		case S_IFIFO:
> +		case S_IFSOCK:
> +		default:
> +			error = -EFTYPE;
> +			break;
> +		}
> +	} else if (!(file->f_mode & FMODE_OPENED)) {
> +		nd.path.dentry = dentry;
> +		error = vfs_open(&nd.path, file);
> +	}
> +	dput(dentry);
> +
> +	if (error)
> +		return ERR_PTR(error);
> +	return no_free_ptr(file);
> +}
> +EXPORT_SYMBOL_FOR_MODULES(vfs_lookup_open, "nfsd");
> +
>  static inline bool trailing_slashes(struct nameidata *nd)
>  {
>  	return (bool)nd->last.name[nd->last.len];
> diff --git a/include/linux/namei.h b/include/linux/namei.h
> index ebe6e29f7e93..86d657b24fc6 100644
> --- a/include/linux/namei.h
> +++ b/include/linux/namei.h
> @@ -97,6 +97,9 @@ struct dentry *start_creating_dentry(struct dentry *parent,
>  struct dentry *start_removing_dentry(struct dentry *parent,
>  				     struct dentry *child);
>  
> +struct file *vfs_lookup_open(struct path *parent, struct qstr *last,
> +			     int open_flag, umode_t mode);
> +
>  /* end_creating - finish action started with start_creating
>   * @child: dentry returned by start_creating() or vfs_mkdir()
>   *
> -- 
> 2.50.0.107.gf914562f5916.dirty

I think it would still be sensible to check the O_ flags being allowed to
be passed in. I get that nsfd is not going to do weird things, but if this
is a helper that may be used by other code too at some point (unaware of the
intricacies of lookup_open()) the context could help as we didn't go through
all the normal checks in build_open_flags().

Best,
Jori.

Re: [PATCH v3 3/3] VFS: add vfs_lookup_open() for nfsd

[NeilBrown]

On Mon, 13 Jul 2026, Jori Koolstra wrote:
> > Op 13-07-2026 08:05 CEST schreef NeilBrown <neilb@ownmail.net>:
> > 
> >  
> > From: NeilBrown <neil@brown.name>
> > 
> > vfs_lookup_open() is a limited version of lookup_open() which is
> > exported for nfsd to use - to replace dentry_create().
> > 
> > It is limited in that no filename is given (thus no auditing) and no
> > LOOKUP_ flags are passed.  A few "intent" LOOKUP flags are deduced from
> > the open flags.
> > 
> > If a non-regular file is found, an O_PATH file is returned.  This allows
> > the caller to see what was found and to return an appropriate error
> > code.  However if ->atomic_open was used, it may return an error rather
> > then provide a dentry.
> 
> Stale commit paragraph.

Thanks :-(

> 
> > 
> > Signed-off-by: NeilBrown <neil@brown.name>
> > ---
> >  fs/namei.c            | 89 +++++++++++++++++++++++++++++++++++++++++++
> >  include/linux/namei.h |  3 ++
> >  2 files changed, 92 insertions(+)
> > 
> > diff --git a/fs/namei.c b/fs/namei.c
> > index cb33a01d16eb..2143f759443a 100644
> > --- a/fs/namei.c
> > +++ b/fs/namei.c
> > @@ -4562,6 +4562,95 @@ static struct dentry *lookup_open(struct nameidata *nd, struct file *file,
> >  	goto out;
> >  }
> >  
> > +/**
> > + * vfs_lookup_open - open and possibly create a regular file
> > + * @parent: directory to contain file
> > + * @last: final component of file name
> > + * @open_flag: O_flags
> > + * @mode: initial permissions for file
> > + *
> > + * Open a file after lookup and/or create.  This provides similar
> > + * functionality open_last_lookups() for non-VFS users, particularly
> > + * nfsd.
> > + * It uses ->atomic_open or ->lookup / ->create / ->open as appropriate.
> > + *
> > + * If the fs object found is not a regular file then an error is returned.
> > + * In some cases, related errors are repurposed to so that the caller can
> > + * determine the file of file found from the error.
> > + * -EISDIR : a directory was found
> > + * -ELOOP  : a symlink was found
> > + * -ENODEV : a block or character device special file was found
> > + * -EFTYPE : any other non-regular file was found, such as FIFO or SOCK.
> > + *
> > + * Returns: the opened struct file, or an error.
> > + */
> > +struct file *vfs_lookup_open(struct path *parent, struct qstr *last,
> > +			     int open_flag, umode_t mode)
> > +{
> > +	struct file *file __free(fput) = NULL;
> > +	struct nameidata nd = {};
> > +	struct open_flags op = {};
> > +	struct dentry *dentry;
> > +	int error = 0;
> > +
> > +	WARN_ONCE(mode & ~S_IALLUGO, "mode must only have permission bits");
> > +	error = lookup_noperm_common(last, parent->dentry);
> > +	if (error)
> > +		return ERR_PTR(error);
> > +
> > +	file = alloc_empty_file(open_flag, current_cred());
> > +	if (IS_ERR(file))
> > +		return file;
> > +
> > +	nd.path = *parent;
> > +	nd.last = *last;
> > +	nd.flags = LOOKUP_OPEN;
> > +	if (open_flag & O_CREAT) {
> > +		nd.flags |= LOOKUP_CREATE;
> > +		if (open_flag & O_EXCL)
> > +			nd.flags |= LOOKUP_EXCL;
> > +	}
> > +	op.open_flag = open_flag;
> > +	op.mode = S_IFREG | mode;
> > +	dentry = lookup_open(&nd, file, &op);
> > +
> > +	if (IS_ERR(dentry))
> > +		return ERR_CAST(dentry);
> > +
> > +	if (d_really_is_negative(dentry)) {
> > +		error = -ENOENT;
> > +	} else if (!(file->f_mode & FMODE_CREATED) && (open_flag & O_EXCL)) {
> > +		error = -EEXIST;
> > +	} else if ((dentry->d_inode->i_mode & S_IFMT) != S_IFREG) {
> > +		switch (dentry->d_inode->i_mode & S_IFMT) {
> > +		case S_IFDIR:
> > +			error = -EISDIR;
> > +			break;
> > +		case S_IFLNK:
> > +			error = -ELOOP;
> > +			break;
> > +		case S_IFBLK:
> > +		case S_IFCHR:
> > +			error = -ENODEV;
> > +			break;
> > +		case S_IFIFO:
> > +		case S_IFSOCK:
> > +		default:
> > +			error = -EFTYPE;
> > +			break;
> > +		}
> > +	} else if (!(file->f_mode & FMODE_OPENED)) {
> > +		nd.path.dentry = dentry;
> > +		error = vfs_open(&nd.path, file);
> > +	}
> > +	dput(dentry);
> > +
> > +	if (error)
> > +		return ERR_PTR(error);
> > +	return no_free_ptr(file);
> > +}
> > +EXPORT_SYMBOL_FOR_MODULES(vfs_lookup_open, "nfsd");
> > +
> >  static inline bool trailing_slashes(struct nameidata *nd)
> >  {
> >  	return (bool)nd->last.name[nd->last.len];
> > diff --git a/include/linux/namei.h b/include/linux/namei.h
> > index ebe6e29f7e93..86d657b24fc6 100644
> > --- a/include/linux/namei.h
> > +++ b/include/linux/namei.h
> > @@ -97,6 +97,9 @@ struct dentry *start_creating_dentry(struct dentry *parent,
> >  struct dentry *start_removing_dentry(struct dentry *parent,
> >  				     struct dentry *child);
> >  
> > +struct file *vfs_lookup_open(struct path *parent, struct qstr *last,
> > +			     int open_flag, umode_t mode);
> > +
> >  /* end_creating - finish action started with start_creating
> >   * @child: dentry returned by start_creating() or vfs_mkdir()
> >   *
> > -- 
> > 2.50.0.107.gf914562f5916.dirty
> 
> I think it would still be sensible to check the O_ flags being allowed to
> be passed in. I get that nsfd is not going to do weird things, but if this
> is a helper that may be used by other code too at some point (unaware of the
> intricacies of lookup_open()) the context could help as we didn't go through
> all the normal checks in build_open_flags().

So we allow

 O_ACCMODE|O_CREAT|O_EXCL|O_TRUNC

and should assert __O_REGULAR.

Thanks,
NeilBrown


> 
> Best,
> Jori.
> 

Re: [PATCH v3 3/3] VFS: add vfs_lookup_open() for nfsd

[Jori Koolstra]

> Op 13-07-2026 23:55 CEST schreef NeilBrown <neilb@ownmail.net>:
> 
>  
> On Mon, 13 Jul 2026, Jori Koolstra wrote:
> > > Op 13-07-2026 08:05 CEST schreef NeilBrown <neilb@ownmail.net>:
> > > 
> > >  
> > > From: NeilBrown <neil@brown.name>
> > > 
> > > vfs_lookup_open() is a limited version of lookup_open() which is
> > > exported for nfsd to use - to replace dentry_create().
> > > 
> > > It is limited in that no filename is given (thus no auditing) and no
> > > LOOKUP_ flags are passed.  A few "intent" LOOKUP flags are deduced from
> > > the open flags.
> > > 
> > > If a non-regular file is found, an O_PATH file is returned.  This allows
> > > the caller to see what was found and to return an appropriate error
> > > code.  However if ->atomic_open was used, it may return an error rather
> > > then provide a dentry.
> > 
> > Stale commit paragraph.
> 
> Thanks :-(
> 
> > 
> > > 
> > > Signed-off-by: NeilBrown <neil@brown.name>
> > > ---
> > >  fs/namei.c            | 89 +++++++++++++++++++++++++++++++++++++++++++
> > >  include/linux/namei.h |  3 ++
> > >  2 files changed, 92 insertions(+)
> > > 
> > > diff --git a/fs/namei.c b/fs/namei.c
> > > index cb33a01d16eb..2143f759443a 100644
> > > --- a/fs/namei.c
> > > +++ b/fs/namei.c
> > > @@ -4562,6 +4562,95 @@ static struct dentry *lookup_open(struct nameidata *nd, struct file *file,
> > >  	goto out;
> > >  }
> > >  
> > > +/**
> > > + * vfs_lookup_open - open and possibly create a regular file
> > > + * @parent: directory to contain file
> > > + * @last: final component of file name
> > > + * @open_flag: O_flags
> > > + * @mode: initial permissions for file
> > > + *
> > > + * Open a file after lookup and/or create.  This provides similar
> > > + * functionality open_last_lookups() for non-VFS users, particularly
> > > + * nfsd.
> > > + * It uses ->atomic_open or ->lookup / ->create / ->open as appropriate.
> > > + *
> > > + * If the fs object found is not a regular file then an error is returned.
> > > + * In some cases, related errors are repurposed to so that the caller can
> > > + * determine the file of file found from the error.
> > > + * -EISDIR : a directory was found
> > > + * -ELOOP  : a symlink was found
> > > + * -ENODEV : a block or character device special file was found
> > > + * -EFTYPE : any other non-regular file was found, such as FIFO or SOCK.
> > > + *
> > > + * Returns: the opened struct file, or an error.
> > > + */
> > > +struct file *vfs_lookup_open(struct path *parent, struct qstr *last,
> > > +			     int open_flag, umode_t mode)
> > > +{
> > > +	struct file *file __free(fput) = NULL;
> > > +	struct nameidata nd = {};
> > > +	struct open_flags op = {};
> > > +	struct dentry *dentry;
> > > +	int error = 0;
> > > +
> > > +	WARN_ONCE(mode & ~S_IALLUGO, "mode must only have permission bits");
> > > +	error = lookup_noperm_common(last, parent->dentry);
> > > +	if (error)
> > > +		return ERR_PTR(error);
> > > +
> > > +	file = alloc_empty_file(open_flag, current_cred());
> > > +	if (IS_ERR(file))
> > > +		return file;
> > > +
> > > +	nd.path = *parent;
> > > +	nd.last = *last;
> > > +	nd.flags = LOOKUP_OPEN;
> > > +	if (open_flag & O_CREAT) {
> > > +		nd.flags |= LOOKUP_CREATE;
> > > +		if (open_flag & O_EXCL)
> > > +			nd.flags |= LOOKUP_EXCL;
> > > +	}
> > > +	op.open_flag = open_flag;
> > > +	op.mode = S_IFREG | mode;
> > > +	dentry = lookup_open(&nd, file, &op);
> > > +
> > > +	if (IS_ERR(dentry))
> > > +		return ERR_CAST(dentry);
> > > +
> > > +	if (d_really_is_negative(dentry)) {
> > > +		error = -ENOENT;
> > > +	} else if (!(file->f_mode & FMODE_CREATED) && (open_flag & O_EXCL)) {
> > > +		error = -EEXIST;
> > > +	} else if ((dentry->d_inode->i_mode & S_IFMT) != S_IFREG) {
> > > +		switch (dentry->d_inode->i_mode & S_IFMT) {
> > > +		case S_IFDIR:
> > > +			error = -EISDIR;
> > > +			break;
> > > +		case S_IFLNK:
> > > +			error = -ELOOP;
> > > +			break;
> > > +		case S_IFBLK:
> > > +		case S_IFCHR:
> > > +			error = -ENODEV;
> > > +			break;
> > > +		case S_IFIFO:
> > > +		case S_IFSOCK:
> > > +		default:
> > > +			error = -EFTYPE;
> > > +			break;
> > > +		}
> > > +	} else if (!(file->f_mode & FMODE_OPENED)) {
> > > +		nd.path.dentry = dentry;
> > > +		error = vfs_open(&nd.path, file);
> > > +	}
> > > +	dput(dentry);
> > > +
> > > +	if (error)
> > > +		return ERR_PTR(error);
> > > +	return no_free_ptr(file);
> > > +}
> > > +EXPORT_SYMBOL_FOR_MODULES(vfs_lookup_open, "nfsd");
> > > +
> > >  static inline bool trailing_slashes(struct nameidata *nd)
> > >  {
> > >  	return (bool)nd->last.name[nd->last.len];
> > > diff --git a/include/linux/namei.h b/include/linux/namei.h
> > > index ebe6e29f7e93..86d657b24fc6 100644
> > > --- a/include/linux/namei.h
> > > +++ b/include/linux/namei.h
> > > @@ -97,6 +97,9 @@ struct dentry *start_creating_dentry(struct dentry *parent,
> > >  struct dentry *start_removing_dentry(struct dentry *parent,
> > >  				     struct dentry *child);
> > >  
> > > +struct file *vfs_lookup_open(struct path *parent, struct qstr *last,
> > > +			     int open_flag, umode_t mode);
> > > +
> > >  /* end_creating - finish action started with start_creating
> > >   * @child: dentry returned by start_creating() or vfs_mkdir()
> > >   *
> > > -- 
> > > 2.50.0.107.gf914562f5916.dirty
> > 
> > I think it would still be sensible to check the O_ flags being allowed to
> > be passed in. I get that nsfd is not going to do weird things, but if this
> > is a helper that may be used by other code too at some point (unaware of the
> > intricacies of lookup_open()) the context could help as we didn't go through
> > all the normal checks in build_open_flags().
> 
> So we allow
> 
>  O_ACCMODE|O_CREAT|O_EXCL|O_TRUNC
> 
> and should assert __O_REGULAR.
> 
> Thanks,
> NeilBrown
> 

Yes, sounds good. Keep in mind that vfs_open() does not handle truncate, it only
happens in do_open() in the open(2) path, so you must handle the actual truncation
back in the nfsd code. You are probably very aware of this, but just in case :)

__O_REGULAR is also only handled in do_open(), so it does nothing in namei.c right
now, still seems good to include it. I don't understand why the __O_REGULAR
implementation is so convoluted, but I haven't looked into it yet. I plan to do
that sometime soon.

Thanks,
Jori.

Re: [PATCH v3 3/3] VFS: add vfs_lookup_open() for nfsd

[NeilBrown]

On Tue, 14 Jul 2026, Jori Koolstra wrote:
> > Op 13-07-2026 23:55 CEST schreef NeilBrown <neilb@ownmail.net>:
> > 
> >  
> > On Mon, 13 Jul 2026, Jori Koolstra wrote:
> > > > Op 13-07-2026 08:05 CEST schreef NeilBrown <neilb@ownmail.net>:
> > > > 
> > > >  
> > > > From: NeilBrown <neil@brown.name>
> > > > 
> > > > vfs_lookup_open() is a limited version of lookup_open() which is
> > > > exported for nfsd to use - to replace dentry_create().
> > > > 
> > > > It is limited in that no filename is given (thus no auditing) and no
> > > > LOOKUP_ flags are passed.  A few "intent" LOOKUP flags are deduced from
> > > > the open flags.
> > > > 
> > > > If a non-regular file is found, an O_PATH file is returned.  This allows
> > > > the caller to see what was found and to return an appropriate error
> > > > code.  However if ->atomic_open was used, it may return an error rather
> > > > then provide a dentry.
> > > 
> > > Stale commit paragraph.
> > 
> > Thanks :-(
> > 
> > > 
> > > > 
> > > > Signed-off-by: NeilBrown <neil@brown.name>
> > > > ---
> > > >  fs/namei.c            | 89 +++++++++++++++++++++++++++++++++++++++++++
> > > >  include/linux/namei.h |  3 ++
> > > >  2 files changed, 92 insertions(+)
> > > > 
> > > > diff --git a/fs/namei.c b/fs/namei.c
> > > > index cb33a01d16eb..2143f759443a 100644
> > > > --- a/fs/namei.c
> > > > +++ b/fs/namei.c
> > > > @@ -4562,6 +4562,95 @@ static struct dentry *lookup_open(struct nameidata *nd, struct file *file,
> > > >  	goto out;
> > > >  }
> > > >  
> > > > +/**
> > > > + * vfs_lookup_open - open and possibly create a regular file
> > > > + * @parent: directory to contain file
> > > > + * @last: final component of file name
> > > > + * @open_flag: O_flags
> > > > + * @mode: initial permissions for file
> > > > + *
> > > > + * Open a file after lookup and/or create.  This provides similar
> > > > + * functionality open_last_lookups() for non-VFS users, particularly
> > > > + * nfsd.
> > > > + * It uses ->atomic_open or ->lookup / ->create / ->open as appropriate.
> > > > + *
> > > > + * If the fs object found is not a regular file then an error is returned.
> > > > + * In some cases, related errors are repurposed to so that the caller can
> > > > + * determine the file of file found from the error.
> > > > + * -EISDIR : a directory was found
> > > > + * -ELOOP  : a symlink was found
> > > > + * -ENODEV : a block or character device special file was found
> > > > + * -EFTYPE : any other non-regular file was found, such as FIFO or SOCK.
> > > > + *
> > > > + * Returns: the opened struct file, or an error.
> > > > + */
> > > > +struct file *vfs_lookup_open(struct path *parent, struct qstr *last,
> > > > +			     int open_flag, umode_t mode)
> > > > +{
> > > > +	struct file *file __free(fput) = NULL;
> > > > +	struct nameidata nd = {};
> > > > +	struct open_flags op = {};
> > > > +	struct dentry *dentry;
> > > > +	int error = 0;
> > > > +
> > > > +	WARN_ONCE(mode & ~S_IALLUGO, "mode must only have permission bits");
> > > > +	error = lookup_noperm_common(last, parent->dentry);
> > > > +	if (error)
> > > > +		return ERR_PTR(error);
> > > > +
> > > > +	file = alloc_empty_file(open_flag, current_cred());
> > > > +	if (IS_ERR(file))
> > > > +		return file;
> > > > +
> > > > +	nd.path = *parent;
> > > > +	nd.last = *last;
> > > > +	nd.flags = LOOKUP_OPEN;
> > > > +	if (open_flag & O_CREAT) {
> > > > +		nd.flags |= LOOKUP_CREATE;
> > > > +		if (open_flag & O_EXCL)
> > > > +			nd.flags |= LOOKUP_EXCL;
> > > > +	}
> > > > +	op.open_flag = open_flag;
> > > > +	op.mode = S_IFREG | mode;
> > > > +	dentry = lookup_open(&nd, file, &op);
> > > > +
> > > > +	if (IS_ERR(dentry))
> > > > +		return ERR_CAST(dentry);
> > > > +
> > > > +	if (d_really_is_negative(dentry)) {
> > > > +		error = -ENOENT;
> > > > +	} else if (!(file->f_mode & FMODE_CREATED) && (open_flag & O_EXCL)) {
> > > > +		error = -EEXIST;
> > > > +	} else if ((dentry->d_inode->i_mode & S_IFMT) != S_IFREG) {
> > > > +		switch (dentry->d_inode->i_mode & S_IFMT) {
> > > > +		case S_IFDIR:
> > > > +			error = -EISDIR;
> > > > +			break;
> > > > +		case S_IFLNK:
> > > > +			error = -ELOOP;
> > > > +			break;
> > > > +		case S_IFBLK:
> > > > +		case S_IFCHR:
> > > > +			error = -ENODEV;
> > > > +			break;
> > > > +		case S_IFIFO:
> > > > +		case S_IFSOCK:
> > > > +		default:
> > > > +			error = -EFTYPE;
> > > > +			break;
> > > > +		}
> > > > +	} else if (!(file->f_mode & FMODE_OPENED)) {
> > > > +		nd.path.dentry = dentry;
> > > > +		error = vfs_open(&nd.path, file);
> > > > +	}
> > > > +	dput(dentry);
> > > > +
> > > > +	if (error)
> > > > +		return ERR_PTR(error);
> > > > +	return no_free_ptr(file);
> > > > +}
> > > > +EXPORT_SYMBOL_FOR_MODULES(vfs_lookup_open, "nfsd");
> > > > +
> > > >  static inline bool trailing_slashes(struct nameidata *nd)
> > > >  {
> > > >  	return (bool)nd->last.name[nd->last.len];
> > > > diff --git a/include/linux/namei.h b/include/linux/namei.h
> > > > index ebe6e29f7e93..86d657b24fc6 100644
> > > > --- a/include/linux/namei.h
> > > > +++ b/include/linux/namei.h
> > > > @@ -97,6 +97,9 @@ struct dentry *start_creating_dentry(struct dentry *parent,
> > > >  struct dentry *start_removing_dentry(struct dentry *parent,
> > > >  				     struct dentry *child);
> > > >  
> > > > +struct file *vfs_lookup_open(struct path *parent, struct qstr *last,
> > > > +			     int open_flag, umode_t mode);
> > > > +
> > > >  /* end_creating - finish action started with start_creating
> > > >   * @child: dentry returned by start_creating() or vfs_mkdir()
> > > >   *
> > > > -- 
> > > > 2.50.0.107.gf914562f5916.dirty
> > > 
> > > I think it would still be sensible to check the O_ flags being allowed to
> > > be passed in. I get that nsfd is not going to do weird things, but if this
> > > is a helper that may be used by other code too at some point (unaware of the
> > > intricacies of lookup_open()) the context could help as we didn't go through
> > > all the normal checks in build_open_flags().
> > 
> > So we allow
> > 
> >  O_ACCMODE|O_CREAT|O_EXCL|O_TRUNC
> > 
> > and should assert __O_REGULAR.
> > 
> > Thanks,
> > NeilBrown
> > 
> 
> Yes, sounds good. Keep in mind that vfs_open() does not handle truncate, it only
> happens in do_open() in the open(2) path, so you must handle the actual truncation
> back in the nfsd code. You are probably very aware of this, but just in case :)

O_TRUNC handling is a bit untidy (or convoluted to use your word).  Some
atomic_open functions implement it, some deliberately don't.  If
atomic_open wasn't used, it happens much later.  Maybe I could try and
tidy that up.

> 
> __O_REGULAR is also only handled in do_open(), so it does nothing in namei.c right
> now, still seems good to include it. I don't understand why the __O_REGULAR
> implementation is so convoluted, but I haven't looked into it yet. I plan to do
> that sometime soon.

Some atomic_open functions handle __O_REGULAR, and I think it best to
abort an unwanted open as early as possible.
"convoluted" happens when code grows over time and people work with what
is there rather than tidying it up first, then working with tidy code.
We cannot tidy everything all at once, but if everyone making changes
does a little bit of tidying, we might avoid too much convolution!

Thanks,
NeilBrown


> 
> Thanks,
> Jori.
> 

Re: [PATCH v3 3/3] VFS: add vfs_lookup_open() for nfsd

[Jori Koolstra]

> Op 15-07-2026 00:58 CEST schreef NeilBrown <neilb@ownmail.net>:
> 
> > 
> > Yes, sounds good. Keep in mind that vfs_open() does not handle truncate, it only
> > happens in do_open() in the open(2) path, so you must handle the actual truncation
> > back in the nfsd code. You are probably very aware of this, but just in case :)
> 
> O_TRUNC handling is a bit untidy (or convoluted to use your word).  Some
> atomic_open functions implement it, some deliberately don't.  If
> atomic_open wasn't used, it happens much later.  Maybe I could try and
> tidy that up.
> 

Wait, so for those ->atomic_open() filesystems that do handle truncate, how do
they deal with this? In do_open() we have

	if (file->f_mode & FMODE_CREATED) {
		/* Don't check for write permission, don't truncate */
		open_flag &= ~O_TRUNC;
		acc_mode = 0;
	} else if (d_is_reg(nd->path.dentry) && open_flag & O_TRUNC) {
                ...
		do_truncate = true;
	}

So on truncate do they just set FMODE_CREATED regardless of whether the file existed
before or not? Or does truncate count as create in general?

> > 
> > __O_REGULAR is also only handled in do_open(), so it does nothing in namei.c right
> > now, still seems good to include it. I don't understand why the __O_REGULAR
> > implementation is so convoluted, but I haven't looked into it yet. I plan to do
> > that sometime soon.
> 
> Some atomic_open functions handle __O_REGULAR, and I think it best to
> abort an unwanted open as early as possible.

Yes, you are right, my bad.

> "convoluted" happens when code grows over time and people work with what
> is there rather than tidying it up first, then working with tidy code.
> We cannot tidy everything all at once, but if everyone making changes
> does a little bit of tidying, we might avoid too much convolution!
> 

Haha, maybe this is math-speak, just like you have legalese. My professor at
McGill used to call certain proofs "convoluted" if they involved a lot of
seemingly senseless rigmarole. I guess the term stuck :)

Best,
Jori.

Re: [PATCH v3 3/3] VFS: add vfs_lookup_open() for nfsd

[NeilBrown]

On Thu, 16 Jul 2026, Jori Koolstra wrote:
> > Op 15-07-2026 00:58 CEST schreef NeilBrown <neilb@ownmail.net>:
> > 
> > > 
> > > Yes, sounds good. Keep in mind that vfs_open() does not handle truncate, it only
> > > happens in do_open() in the open(2) path, so you must handle the actual truncation
> > > back in the nfsd code. You are probably very aware of this, but just in case :)
> > 
> > O_TRUNC handling is a bit untidy (or convoluted to use your word).  Some
> > atomic_open functions implement it, some deliberately don't.  If
> > atomic_open wasn't used, it happens much later.  Maybe I could try and
> > tidy that up.
> > 
> 
> Wait, so for those ->atomic_open() filesystems that do handle truncate, how do
> they deal with this? In do_open() we have
> 
> 	if (file->f_mode & FMODE_CREATED) {
> 		/* Don't check for write permission, don't truncate */
> 		open_flag &= ~O_TRUNC;
> 		acc_mode = 0;
> 	} else if (d_is_reg(nd->path.dentry) && open_flag & O_TRUNC) {
>                 ...
> 		do_truncate = true;
> 	}
> 
> So on truncate do they just set FMODE_CREATED regardless of whether the file existed
> before or not? Or does truncate count as create in general?

No, they only set FMODE_CREATED when the file was actually created.
But the setattr call fro do_open() which implements the truncate carries
ATTR_OPEN, and if the filesystem knows that it always handles O_TRUNC on
open, then it can ignore ATTR_SIZE when ATTR_OPEN is set.
I'm not certain that all filesystems do this correctly, but some seem to.

NeilBrown