Re: [PATCH v3 09/14] vfs: add O_CREAT|O_DIRECTORY to open*(2)

[Christian Brauner <brauner@kernel.org>]

On 2026-07-10 20:55 +0200, Jori Koolstra wrote:
> 
> > Op 07-07-2026 12:51 CEST schreef Christian Brauner <brauner@kernel.org>:
> > 
> >  
> > > Currently there is no way to race-freely create and open a directory.
> > > For regular files we have open(O_CREAT) for creating a new file inode,
> > > and returning a pinning fd to it. The lack of such functionality for
> > > directories means that when populating a directory tree there's always
> > > a race involved: the inodes first need to be created, and then opened
> > > to adjust their permissions/ownership/labels/timestamps/acls/xattrs/...,
> > > but in the time window between the creation and the opening they might
> > > be replaced by something else.
> > > 
> > > Addressing this race without proper APIs is possible (by immediately
> > > fstat()ing what was opened, to verify that it has the right inode type),
> > > but difficult to get right. Hence, adding support for a new flag combo
> > 
> > Eh, but that still means someone could've raced you and actually created
> > that directory. When you bring in a shared filesysem state with joint
> > cleanup responsibilities things aren't easily solved by fstat()ing.
> > 
> 
> Right, but can't I check with fstat() if the object satisfies all properties
> I need, like owner, group, inode, type, etc.?
> 
> If it does, then why care how it was created?

Because you might not want to delete something that another task with
the same credentials created. There's always going to be some race with
splitting it into two operations that matters in subtle ways.

> > > O_CREAT|O_DIRECTORY to open*(2) that creates a directory (if it does not
> > > exist already) and returns an O_DIRECTORY fd is very useful.
> > > 
> > > Historically, the O_CREAT|O_DIRECTORY behaviour was to return ENOTDIR if
> > > a regular file exists at the open path; EISDIR if a directory exists at
> > > the path; and to create a regular file if no file exists at the path.
> > > This behaviour changed accidentally with 973d4b73fbaf ("do_last(): rejoin
> > > the common path even earlier in FMODE_{OPENED,CREATED} case") causing
> > > ENOTDIR to return in the last case while still creating the file. As
> > > this change was not detected for a long time, Brauner proposed to adopt
> > > the more consistent NetBSD behaviour, i.e. to return EINVAL on the the
> > > O_CREAT|O_DIRECTORY combination. This change was applied in 43b450632676
> > > ("open: return EINVAL for O_DIRECTORY | O_CREAT") in March, 2023. As
> > > the EINVAL behaviour has been in the kernel for about 3 year now, no
> > > rollback is expected as a result of userspace reliance on old
> > > behaviour, leaving us free to reassign the O_CREAT|O_DIRECTORY semantics.
> > > 
> > > O_CREAT|O_DIRECTORY is made to reduce to a lookup on ->atomic_open()
> > > filesystems. This is implemented by stripping the O_CREAT bit. The other
> > > option of simply returning -EINVAL leads to inconsistent behaviour:
> > 
> > I think this doesn't spell out why that's done: afaict, netfses can't
> > currently deal with O_CREAT | O_DIRECTORY without protocol extensions.
> > So that means you force it into a fallback mode where it manages to open
> > an existing directory but fails with ENOENT otherwise. This should carry
> > a brief comment why you force that type of fallback.
> > 
> > Also, how do you deal with O_EXCL? If you strip O_CREAT but leave O_EXCL
> > depending on where exactly you do it, might this risk opening a
> > directory giving userspace the impression they exclusively created it?
> > 
> 
> I think we're fine, in do_open() we still have:
> 
> 	if (open_flag & O_CREAT) {
> 		if ((open_flag & O_EXCL) && !(file->f_mode & FMODE_CREATED))
> 			return -EEXIST;
> 
> and I am stripping the local variable O_CREAT bit only. Also, do_open() is
> only reached if we didn't have an -ENOENT return (lookup succeeded).
> 
> So it's either -ENOENT if the dir does not exist or if it does -EEXIST
> (with O_EXCL set).
> 
> Agree?

Sounds good. Make sure that there are O_EXCL tests, please.