Re: [PATCH 2/2] VFS: don't call ->atomic_open on cached negative without O_CREAT

[Al Viro <viro@zeniv.linux.org.uk>]

On Mon, Sep 15, 2025 at 01:01:08PM +1000, NeilBrown wrote:

> All filesystems with ->atomic_open() currently handle the case of a
> negative dentry without O_CREAT either by returning -ENOENT or by
> calling finish_no_open(), either with NULL or with the negative dentry.

Wait a sec...  Just who is passing finish_no_open() a negative dentry?
Any such is very likely to be a bug...

> All of these have the same effect.
> 
> For filesystems without ->atomic_open(), lookup_open() will, in this
> case, also call finish_no_open().
> 
> So this patch removes the burden from filesystems by calling
> finish_no_open() early on a negative cached dentry when O_CREAT isn't
> requested.

Re "removing the burden" - it still can be called with negative cached without
O_CREAT.

O_CREAT in open(2) arguments might not survive to the call of atomic_open() -
in case when you don't have write permissions on parent.  In that case
we strip O_CREAT and call into atomic_open() (if the method is there).
In that case -ENOENT from atomic_open() is translated into -EACCES or -EROFS:
                dentry = atomic_open(nd, dentry, file, open_flag, mode);
		if (unlikely(create_error) && dentry == ERR_PTR(-ENOENT))
			dentry = ERR_PTR(create_error);
		return dentry;
In case when no ->atomic_open() is there the same logics is
        if (unlikely(create_error) && !dentry->d_inode) {
		error = create_error;
		goto out_dput;
	}
in the very end of lookup_open().  The point is, you might have had a call
of ->d_revalidate() with LOOKUP_CREATE|LOOKUP_OPEN and then have the damn
thing passed to ->atomic_open() with no O_CREAT in flags.