[PATCH 0/3] vfs: call audit_inode_child() in lookup_open() on failure

[PATCH 0/3] vfs: call audit_inode_child() in lookup_open() on failure

[Jori Koolstra]

While reworking lookup_open() to implement O_CREAT|O_DIRECTORY I came
across inconsistency in how audit_inode_child() is called in various
file create paths.

audit_inode_child() is called in may_create_dentry() so that failed
filesystem operations still register an audit entry. On success, the
entry is overwritten when, for instance, fsnotify_create() is called.
This is the calling convention in vfs_create() and vfs_mkdir().
In lookup_open(), however, when atomic_open() should have created a
file but didn't, no call to audit_inode_child() is made. The same is
true for the regular ->create() path.

On the suggestion of Brauner, I am splitting this series from the
O_CREAT|O_DIRECTORY work, so that this can be discussed with the audit
people without delaying that work.

Jori Koolstra (3):
  vfs: move create error && negative dentry case in lookup_open() up
  vfs: call audit_inode_child() in lookup_open() on failure
  fs/namei.c: update kerneldoc of atomic_open()

 fs/namei.c | 128 ++++++++++++++++++++++++++++++++---------------------
 1 file changed, 78 insertions(+), 50 deletions(-)

-- 
2.55.0

[PATCH 1/3] vfs: move create error && negative dentry case in lookup_open() up

[Jori Koolstra]

O_CREAT is stripped when create_error is set in lookup_open(), so when
lookup does not return an inode, the case

	if (!dentry->d_inode && (open_flag & O_CREAT))

is always skipped. We can get rid of this cognitive step by handling the
error case first.

Reviewed-by: NeilBrown <neil@brown.name>
Signed-off-by: Jori Koolstra <jkoolstra@xs4all.nl>
---
 fs/namei.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/fs/namei.c b/fs/namei.c
index 19ce43c9a6e6..447eda7cbe34 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -4491,6 +4491,11 @@ static struct dentry *lookup_open(struct nameidata *nd, struct file *file,
 		}
 	}
 
+	if (unlikely(create_error) && !dentry->d_inode) {
+		error = create_error;
+		goto out_dput;
+	}
+
 	/* Negative dentry, just create the file */
 	if (!dentry->d_inode && (open_flag & O_CREAT)) {
 		/* but break the directory lease first! */
@@ -4510,10 +4515,7 @@ static struct dentry *lookup_open(struct nameidata *nd, struct file *file,
 		if (error)
 			goto out_dput;
 	}
-	if (unlikely(create_error) && !dentry->d_inode) {
-		error = create_error;
-		goto out_dput;
-	}
+
 	return dentry;
 
 out_dput:
-- 
2.55.0

[PATCH 2/3] vfs: call audit_inode_child() in lookup_open() on failure

[Jori Koolstra]

audit_inode_child() is called in may_create_dentry() so that failed
filesystem operations still register an audit entry. On success, the
entry is overwritten when, for instance, fsnotify_create() is called.
This is the calling convention in vfs_create() and vfs_mkdir().
In lookup_open(), however, when atomic_open() should have created a
file but didn't, no call to audit_inode_child() is made. The same is
true for the regular ->create() path.

Fix the calling of audit_inode_child() in lookup_open() to match the
vfs_create() path. For the ->atomic_open() filesystems this logic has
been pushed into atomic_open(). This function is also reordered a bit
to make the case distinction of the possible returns from
->atomic_open() more explicit (i.e. finish_open() or finish_no_open()).

When retrying delegation breaking, audit_inode_child() could be called
more than once, but this is OK because those entries are reused.

Signed-off-by: Jori Koolstra <jkoolstra@xs4all.nl>
---
 fs/namei.c | 92 ++++++++++++++++++++++++++++++++----------------------
 1 file changed, 55 insertions(+), 37 deletions(-)

diff --git a/fs/namei.c b/fs/namei.c
index 447eda7cbe34..fe5d59c6f925 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -4351,35 +4351,55 @@ static int may_o_create(struct mnt_idmap *idmap,
  */
 static struct dentry *atomic_open(const struct path *path, struct dentry *dentry,
 				  struct file *file,
-				  int open_flag, umode_t mode)
+				  int open_flag, umode_t mode, int create_error)
 {
 	struct dentry *const DENTRY_NOT_SET = (void *) -1UL;
-	struct inode *dir =  path->dentry->d_inode;
+	struct inode *dir_inode = path->dentry->d_inode;
 	int error;
 
 	file->__f_path.dentry = DENTRY_NOT_SET;
 	file->__f_path.mnt = path->mnt;
-	error = dir->i_op->atomic_open(dir, dentry, file,
+	error = dir_inode->i_op->atomic_open(dir_inode, dentry, file,
 				       open_to_namei_flags(open_flag), mode);
 	d_lookup_done(dentry);
+
 	if (!error) {
 		if (file->f_mode & FMODE_OPENED) {
-			if (unlikely(dentry != file->f_path.dentry)) {
+			/* finish_open() called */
+			struct dentry *opened = file->f_path.dentry;
+			if (unlikely(opened != dentry)) {
 				dput(dentry);
-				dentry = dget(file->f_path.dentry);
+				dentry = dget(opened);
 			}
-		} else if (WARN_ON(file->f_path.dentry == DENTRY_NOT_SET)) {
-			error = -EIO;
-		} else {
-			if (file->f_path.dentry) {
+		} else if (likely(file->f_path.dentry != DENTRY_NOT_SET)) {
+			/* finish_no_open() called */
+			struct dentry *replaced = file->f_path.dentry;
+			if (replaced) {
 				dput(dentry);
-				dentry = file->f_path.dentry;
+				dentry = replaced;
 			}
 			if (unlikely(d_is_negative(dentry)))
 				error = -ENOENT;
+		} else {
+			const char *fsname = dentry->d_sb->s_type->name;
+			WARN(1, "%s: ->atomic_open() left file->f_path.dentry unset!\n",
+			        fsname);
+			error = -EIO;
 		}
 	}
+
 	if (error) {
+		if (unlikely(create_error) && error == -ENOENT) {
+			/*
+			 * Should have done a create, but errored before.
+			 * Some filesystems return -ENOENT directly instead of
+			 * calling finish_no_open() with a negative dentry;
+			 * either way it should only mean the child doesn't exist,
+			 * so a refused create is safe to record here.
+			 */
+			audit_inode_child(dir_inode, dentry, AUDIT_TYPE_CHILD_CREATE);
+			error = create_error;
+		}
 		dput(dentry);
 		dentry = ERR_PTR(error);
 	}
@@ -4437,7 +4457,7 @@ static struct dentry *lookup_open(struct nameidata *nd, struct file *file,
 		dentry = NULL;
 	}
 	if (dentry->d_inode) {
-		/* Cached positive dentry: will open in f_op->open */
+		/* Cached positive dentry: will open in do_open(). */
 		return dentry;
 	}
 
@@ -4471,10 +4491,7 @@ static struct dentry *lookup_open(struct nameidata *nd, struct file *file,
 	if (dir_inode->i_op->atomic_open) {
 		if (nd->flags & LOOKUP_DIRECTORY)
 			open_flag |= O_DIRECTORY;
-		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;
+		return atomic_open(&nd->path, dentry, file, open_flag, mode, create_error);
 	}
 
 	if (d_in_lookup(dentry)) {
@@ -4490,32 +4507,36 @@ static struct dentry *lookup_open(struct nameidata *nd, struct file *file,
 			dentry = res;
 		}
 	}
+	if (dentry->d_inode || !(op->open_flag & O_CREAT)) {
+		/* No need to create a file. If lookup returned a positive
+		 * dentry, the file will be opened in do_open(). */
+		return dentry;
+	}
+
+	/* Negative dentry with O_CREAT flag set */
+	audit_inode_child(dir_inode, dentry, AUDIT_TYPE_CHILD_CREATE);
 
-	if (unlikely(create_error) && !dentry->d_inode) {
+	if (unlikely(create_error)) {
+		/* should have done a create, but we already errored */
 		error = create_error;
 		goto out_dput;
 	}
 
-	/* 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);
-		if (error)
-			goto out_dput;
-
-		file->f_mode |= FMODE_CREATED;
-		audit_inode_child(dir_inode, dentry, AUDIT_TYPE_CHILD_CREATE);
-		if (!dir_inode->i_op->create) {
-			error = -EACCES;
-			goto out_dput;
-		}
+	error = try_break_deleg(dir_inode, LEASE_BREAK_DIR_CREATE, delegated_inode);
+	if (error)
+		goto out_dput;
 
-		error = dir_inode->i_op->create(idmap, dir_inode, dentry,
-						mode, open_flag & O_EXCL);
-		if (error)
-			goto out_dput;
+	file->f_mode |= FMODE_CREATED;
+	if (!dir_inode->i_op->create) {
+		error = -EACCES;
+		goto out_dput;
 	}
 
+	error = dir_inode->i_op->create(idmap, dir_inode, dentry,
+					mode, open_flag & O_EXCL);
+	if (error)
+		goto out_dput;
+
 	return dentry;
 
 out_dput:
@@ -5053,7 +5074,7 @@ struct file *dentry_create(struct path *path, int flags, umode_t mode,
 
 		/* atomic_open will dput(dentry) on error */
 		dget(orig_dentry);
-		dentry = atomic_open(path, dentry, file, flags, mode);
+		dentry = atomic_open(path, dentry, file, flags, mode, create_error);
 		error = PTR_ERR_OR_ZERO(dentry);
 
 		if (IS_ERR(dentry))
@@ -5063,9 +5084,6 @@ struct file *dentry_create(struct path *path, int flags, umode_t mode,
 			/* Drop the extra reference */
 			dput(orig_dentry);
 
-		if (unlikely(create_error) && error == -ENOENT)
-			error = create_error;
-
 		if (!error) {
 			if (file->f_mode & FMODE_CREATED)
 				fsnotify_create(dir->d_inode, dentry);
-- 
2.55.0

[PATCH 3/3] fs/namei.c: update kerneldoc of atomic_open()

[Jori Koolstra]

The comments above atomic_open() contain several errors:

  - atomic_open() does not return 0 if successful
  - @path is not updated

Fix those and be more explicit about when FMODE_OPENED and FMODE_CREATED
are set. Change to a full kerneldoc.

Signed-off-by: Jori Koolstra <jkoolstra@xs4all.nl>
---
 fs/namei.c | 32 ++++++++++++++++++++------------
 1 file changed, 20 insertions(+), 12 deletions(-)

diff --git a/fs/namei.c b/fs/namei.c
index fe5d59c6f925..2eef2a49d3f4 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -4336,18 +4336,26 @@ static int may_o_create(struct mnt_idmap *idmap,
 	return security_inode_create(dir->dentry->d_inode, dentry, mode);
 }
 
-/*
- * Attempt to atomically look up, create and open a file from a negative
- * dentry.
- *
- * Returns 0 if successful.  The file will have been created and attached to
- * @file by the filesystem calling finish_open().
- *
- * If the file was looked up only or didn't need creating, FMODE_OPENED won't
- * be set.  The caller will need to perform the open themselves.  @path will
- * have been updated to point to the new dentry.  This may be negative.
- *
- * Returns an error code otherwise.
+/**
+ * atomic_open() - attempt to atomically look up, create and open a file
+ * from a negative dentry.
+ * @path:          parent directory path
+ * @dentry:        child to ->atomic_open()
+ * @file:          file to attach child to
+ * @open_flag:     open flags
+ * @mode:          create mode
+ * @create_error:  return value from may_o_create()
+ *
+ * If a non-error dentry is returned then: when FMODE_OPENED is set,
+ * the file will have been attached to @file by the filesystem calling
+ * finish_open(). If FMODE_OPENED isn't set, the filesystem instead called
+ * finish_no_open() and the caller will need to perform the open themselves.
+ *
+ * FMODE_CREATED is set when the call to ->atomic_open() actually created
+ * the file.
+ *
+ * Returns the opened/looked-up dentry on success or ERR_PTR(-E) on failure.
+ * On error, atomic_open() consumes @dentry.
  */
 static struct dentry *atomic_open(const struct path *path, struct dentry *dentry,
 				  struct file *file,
-- 
2.55.0

Re: [PATCH 0/3] vfs: call audit_inode_child() in lookup_open() on failure

[Christian Brauner]

> While reworking lookup_open() to implement O_CREAT|O_DIRECTORY I came
> across inconsistency in how audit_inode_child() is called in various
> file create paths.
> 
> audit_inode_child() is called in may_create_dentry() so that failed
> filesystem operations still register an audit entry. On success, the
> entry is overwritten when, for instance, fsnotify_create() is called.
> This is the calling convention in vfs_create() and vfs_mkdir().
> In lookup_open(), however, when atomic_open() should have created a
> file but didn't, no call to audit_inode_child() is made. The same is
> true for the regular ->create() path.
> 
> On the suggestion of Brauner, I am splitting this series from the
> O_CREAT|O_DIRECTORY work, so that this can be discussed with the audit
> people without delaying that work.

Looks good to me. I'll wait for the audit people to look at it too.

-- 

Re: [PATCH 1/3] vfs: move create error && negative dentry case in lookup_open() up

[Paul Moore]

On Jul 10, 2026 Jori Koolstra <jkoolstra@xs4all.nl> wrote:
> 
> O_CREAT is stripped when create_error is set in lookup_open(), so when
> lookup does not return an inode, the case
> 
> 	if (!dentry->d_inode && (open_flag & O_CREAT))
> 
> is always skipped. We can get rid of this cognitive step by handling the
> error case first.
> 
> Reviewed-by: NeilBrown <neil@brown.name>
> Signed-off-by: Jori Koolstra <jkoolstra@xs4all.nl>
> ---
>  fs/namei.c | 10 ++++++----
>  1 file changed, 6 insertions(+), 4 deletions(-)

Reviewed-by: Paul Moore <paul@paul-moore.com>

--
paul-moore.com

Re: [PATCH 2/3] vfs: call audit_inode_child() in lookup_open() on failure

[Paul Moore]

On Jul 10, 2026 Jori Koolstra <jkoolstra@xs4all.nl> wrote:
> 
> audit_inode_child() is called in may_create_dentry() so that failed
> filesystem operations still register an audit entry. On success, the
> entry is overwritten when, for instance, fsnotify_create() is called.
> This is the calling convention in vfs_create() and vfs_mkdir().
> In lookup_open(), however, when atomic_open() should have created a
> file but didn't, no call to audit_inode_child() is made. The same is
> true for the regular ->create() path.
> 
> Fix the calling of audit_inode_child() in lookup_open() to match the
> vfs_create() path. For the ->atomic_open() filesystems this logic has
> been pushed into atomic_open(). This function is also reordered a bit
> to make the case distinction of the possible returns from
> ->atomic_open() more explicit (i.e. finish_open() or finish_no_open()).
> 
> When retrying delegation breaking, audit_inode_child() could be called
> more than once, but this is OK because those entries are reused.
> 
> Signed-off-by: Jori Koolstra <jkoolstra@xs4all.nl>
> ---
>  fs/namei.c | 92 ++++++++++++++++++++++++++++++++----------------------
>  1 file changed, 55 insertions(+), 37 deletions(-)

I didn't dig into the directory lease/delegation changes because there
are better people on the To/CC line to comment on that, but the audit
changes look good to me.  Thanks for working on this, help is always
appreciated :)

Acked-by: Paul Moore (audit) <paul@paul-moore.com>

> diff --git a/fs/namei.c b/fs/namei.c
> index 447eda7cbe34..fe5d59c6f925 100644
> --- a/fs/namei.c
> +++ b/fs/namei.c
> @@ -4351,35 +4351,55 @@ static int may_o_create(struct mnt_idmap *idmap,
>   */
>  static struct dentry *atomic_open(const struct path *path, struct dentry *dentry,
>  				  struct file *file,
> -				  int open_flag, umode_t mode)
> +				  int open_flag, umode_t mode, int create_error)
>  {
>  	struct dentry *const DENTRY_NOT_SET = (void *) -1UL;
> -	struct inode *dir =  path->dentry->d_inode;
> +	struct inode *dir_inode = path->dentry->d_inode;
>  	int error;
>  
>  	file->__f_path.dentry = DENTRY_NOT_SET;
>  	file->__f_path.mnt = path->mnt;
> -	error = dir->i_op->atomic_open(dir, dentry, file,
> +	error = dir_inode->i_op->atomic_open(dir_inode, dentry, file,
>  				       open_to_namei_flags(open_flag), mode);
>  	d_lookup_done(dentry);
> +
>  	if (!error) {
>  		if (file->f_mode & FMODE_OPENED) {
> -			if (unlikely(dentry != file->f_path.dentry)) {
> +			/* finish_open() called */
> +			struct dentry *opened = file->f_path.dentry;
> +			if (unlikely(opened != dentry)) {
>  				dput(dentry);
> -				dentry = dget(file->f_path.dentry);
> +				dentry = dget(opened);
>  			}
> -		} else if (WARN_ON(file->f_path.dentry == DENTRY_NOT_SET)) {
> -			error = -EIO;
> -		} else {
> -			if (file->f_path.dentry) {
> +		} else if (likely(file->f_path.dentry != DENTRY_NOT_SET)) {
> +			/* finish_no_open() called */
> +			struct dentry *replaced = file->f_path.dentry;
> +			if (replaced) {
>  				dput(dentry);
> -				dentry = file->f_path.dentry;
> +				dentry = replaced;
>  			}
>  			if (unlikely(d_is_negative(dentry)))
>  				error = -ENOENT;
> +		} else {
> +			const char *fsname = dentry->d_sb->s_type->name;
> +			WARN(1, "%s: ->atomic_open() left file->f_path.dentry unset!\n",
> +			        fsname);
> +			error = -EIO;
>  		}
>  	}
> +
>  	if (error) {
> +		if (unlikely(create_error) && error == -ENOENT) {
> +			/*
> +			 * Should have done a create, but errored before.
> +			 * Some filesystems return -ENOENT directly instead of
> +			 * calling finish_no_open() with a negative dentry;
> +			 * either way it should only mean the child doesn't exist,
> +			 * so a refused create is safe to record here.
> +			 */
> +			audit_inode_child(dir_inode, dentry, AUDIT_TYPE_CHILD_CREATE);
> +			error = create_error;
> +		}
>  		dput(dentry);
>  		dentry = ERR_PTR(error);
>  	}
> @@ -4437,7 +4457,7 @@ static struct dentry *lookup_open(struct nameidata *nd, struct file *file,
>  		dentry = NULL;
>  	}
>  	if (dentry->d_inode) {
> -		/* Cached positive dentry: will open in f_op->open */
> +		/* Cached positive dentry: will open in do_open(). */
>  		return dentry;
>  	}
>  
> @@ -4471,10 +4491,7 @@ static struct dentry *lookup_open(struct nameidata *nd, struct file *file,
>  	if (dir_inode->i_op->atomic_open) {
>  		if (nd->flags & LOOKUP_DIRECTORY)
>  			open_flag |= O_DIRECTORY;
> -		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;
> +		return atomic_open(&nd->path, dentry, file, open_flag, mode, create_error);
>  	}
>  
>  	if (d_in_lookup(dentry)) {
> @@ -4490,32 +4507,36 @@ static struct dentry *lookup_open(struct nameidata *nd, struct file *file,
>  			dentry = res;
>  		}
>  	}
> +	if (dentry->d_inode || !(op->open_flag & O_CREAT)) {
> +		/* No need to create a file. If lookup returned a positive
> +		 * dentry, the file will be opened in do_open(). */
> +		return dentry;
> +	}
> +
> +	/* Negative dentry with O_CREAT flag set */
> +	audit_inode_child(dir_inode, dentry, AUDIT_TYPE_CHILD_CREATE);
>  
> -	if (unlikely(create_error) && !dentry->d_inode) {
> +	if (unlikely(create_error)) {
> +		/* should have done a create, but we already errored */
>  		error = create_error;
>  		goto out_dput;
>  	}
>  
> -	/* 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);
> -		if (error)
> -			goto out_dput;
> -
> -		file->f_mode |= FMODE_CREATED;
> -		audit_inode_child(dir_inode, dentry, AUDIT_TYPE_CHILD_CREATE);
> -		if (!dir_inode->i_op->create) {
> -			error = -EACCES;
> -			goto out_dput;
> -		}
> +	error = try_break_deleg(dir_inode, LEASE_BREAK_DIR_CREATE, delegated_inode);
> +	if (error)
> +		goto out_dput;
>  
> -		error = dir_inode->i_op->create(idmap, dir_inode, dentry,
> -						mode, open_flag & O_EXCL);
> -		if (error)
> -			goto out_dput;
> +	file->f_mode |= FMODE_CREATED;
> +	if (!dir_inode->i_op->create) {
> +		error = -EACCES;
> +		goto out_dput;
>  	}
>  
> +	error = dir_inode->i_op->create(idmap, dir_inode, dentry,
> +					mode, open_flag & O_EXCL);
> +	if (error)
> +		goto out_dput;
> +
>  	return dentry;
>  
>  out_dput:
> @@ -5053,7 +5074,7 @@ struct file *dentry_create(struct path *path, int flags, umode_t mode,
>  
>  		/* atomic_open will dput(dentry) on error */
>  		dget(orig_dentry);
> -		dentry = atomic_open(path, dentry, file, flags, mode);
> +		dentry = atomic_open(path, dentry, file, flags, mode, create_error);
>  		error = PTR_ERR_OR_ZERO(dentry);
>  
>  		if (IS_ERR(dentry))
> @@ -5063,9 +5084,6 @@ struct file *dentry_create(struct path *path, int flags, umode_t mode,
>  			/* Drop the extra reference */
>  			dput(orig_dentry);
>  
> -		if (unlikely(create_error) && error == -ENOENT)
> -			error = create_error;
> -
>  		if (!error) {
>  			if (file->f_mode & FMODE_CREATED)
>  				fsnotify_create(dir->d_inode, dentry);
> -- 
> 2.55.0

--
paul-moore.com

Re: [PATCH 3/3] fs/namei.c: update kerneldoc of atomic_open()

[Paul Moore]

On Jul 10, 2026 Jori Koolstra <jkoolstra@xs4all.nl> wrote:
> 
> The comments above atomic_open() contain several errors:
> 
>   - atomic_open() does not return 0 if successful
>   - @path is not updated
> 
> Fix those and be more explicit about when FMODE_OPENED and FMODE_CREATED
> are set. Change to a full kerneldoc.
> 
> Signed-off-by: Jori Koolstra <jkoolstra@xs4all.nl>
> ---
>  fs/namei.c | 32 ++++++++++++++++++++------------
>  1 file changed, 20 insertions(+), 12 deletions(-)

Reviewed-by: Paul Moore <paul@paul-moore.com>

--
paul-moore.com