Re: [PATCH v2 04/14] vfs: call audit_inode_child() in lookup_open() on failure too

[NeilBrown]

On Tue, 30 Jun 2026, Jori Koolstra 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. Note that if ->atomic_open() returned an error
> other than -ENOENT while trying to do a create, the audit is skipped.
> There is no way around this because we can't know at the vfs level
> if the error was due to a failed lookup or failed create.
> 
> Signed-off-by: Jori Koolstra <jkoolstra@xxxxxxxxx>
> ---
>  fs/namei.c | 60 ++++++++++++++++++++++++++++++++++--------------------
>  1 file changed, 38 insertions(+), 22 deletions(-)
> 
> diff --git a/fs/namei.c b/fs/namei.c
> index 9cdc9191dcda..4f747546ff3d 100644
> --- a/fs/namei.c
> +++ b/fs/namei.c
> @@ -4462,7 +4462,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;
>  	}
>  
> @@ -4494,11 +4494,23 @@ static struct dentry *lookup_open(struct nameidata *nd, struct file *file,
>  	if (create_error)
>  		open_flag &= ~O_CREAT;
>  	if (dir_inode->i_op->atomic_open) {
> +		struct dentry *orig_dentry = dentry;
>  		if (nd->flags & LOOKUP_DIRECTORY)
>  			open_flag |= O_DIRECTORY;
> +		dget(orig_dentry);
>  		dentry = atomic_open(&nd->path, dentry, file, open_flag, mode);
> -		if (unlikely(create_error) && dentry == ERR_PTR(-ENOENT))
> -			dentry = ERR_PTR(create_error);
> +		error = PTR_ERR_OR_ZERO(dentry);
> +		if (unlikely(error))
> +			dentry = orig_dentry;
> +		else
> +			dput(orig_dentry);
> +		if (unlikely(create_error) && error == -ENOENT) {
> +			/* Should have done a create, but we already errored. */
> +			audit_inode_child(dir->d_inode, dentry, AUDIT_TYPE_CHILD_CREATE);
> +			error = create_error;
> +		}
> +		if (unlikely(error))
> +			goto out_dput;
>  		return dentry;

I think the extra dget/dput is unnecessary and a little clumsy.
I would rather you passed 'create_error' to atomic_open() and let it
call audit_inode_child() is needed before it calls dput().

Otherwise I like this patch - I think it clarifies the code flow.

Thanks,
NeilBrown


>  	}
>  
> @@ -4515,33 +4527,37 @@ 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->d_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;
> +	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;
> -		}
> +	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;
> +	error = dir_inode->i_op->create(idmap, dir_inode, dentry,
> +					mode, open_flag & O_EXCL);
> +	if (error)
> +		goto out_dput;
>  
> -		fsnotify_create(dir_inode, dentry);
> -	}
> +	fsnotify_create(dir_inode, dentry);
>  
>  	return dentry;
>  
> -- 
> 2.54.0
> 
> 
>