vfs: call audit_inode_child() in lookup_open() on failure

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>
Link: https://patch.msgid.link/20260710164233.827744-3-jkoolstra@xs4all.nl
Acked-by: Paul Moore (audit) <paul@paul-moore.com>
Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>

diff --git a/fs/namei.c b/fs/namei.c
index 62f1b8600ec18e9c1b1f95b4782f34314aed3eee..263eff01b1e401f1961e16250aebc86234729fbb 100644 (file)
--- 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);
        }
@@ -4459,7 +4479,7 @@ retry:
                dentry = NULL;
        }
        if (dentry->d_inode) {
-               /* Cached positive dentry: will open in f_op->open */
+               /* Cached positive dentry: will open in do_open(). */
                goto out;
        }
 
@@ -4493,9 +4513,8 @@ retry:
        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);
+               dentry = atomic_open(&nd->path, dentry, file, open_flag, mode,
+                                    create_error);
                goto out;
        }
 
@@ -4512,31 +4531,35 @@ retry:
                        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(). */
+               goto out;
+       }
+
+       /* 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;
 out:
        if (!IS_ERR(dentry)) {
                if (file->f_mode & FMODE_CREATED)
@@ -5154,7 +5177,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))
@@ -5164,9 +5187,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);