Re: [PATCH v6 07/12] vfs: add O_CREAT|O_DIRECTORY to open*(2)
From: NeilBrown
Date: Fri Sep 18 2026 - 20:07:59 EST
On Fri, 18 Sep 2026, NeilBrown wrote:
> On Fri, 18 Sep 2026, Christian Brauner wrote:
> > On Sun, Sep 13, 2026 at 08:50:11PM +0200, Jori Koolstra wrote:
> > > 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 a proper API is only partially possible:
> > > the caller can immediately fstat() what was opened to verify that it
> > > has the expected inode type, owner and mode. But besides being easy to
> > > get wrong, this cannot establish who created the directory: a directory
> > > created by another process with identical credentials is
> > > indistinguishable from one the caller created itself, so the caller
> > > cannot tell whether the directory is its own to manage.
> > >
> > > 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
> > > commit 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 O_CREAT|O_DIRECTORY combination.
> > > This change was applied in commit 43b450632676 ("open: return EINVAL for
> > > O_DIRECTORY | O_CREAT") in March, 2023. As the EINVAL behaviour has been
> > > in the kernel for about 3 years 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. These filesystems currently cannot handle
> > > O_CREAT|O_DIRECTORY without protocol extensions and therefore are forced
> > > into a fallback mode by stripping the O_CREAT bit. This causes existing
> > > directories to be successfully opened, while for targets that should
> > > have been created, -ENOENT is returned. This -ENOENT is then converted
> > > to -EOPNOTSUPP in later atomic_open(). The simple option of just
> > > returning -EOPNOTSUPP directly leads to inconsistent behaviour: before
> > > ->atomic_open() is called in lookup_open(), the dcache is queried. So
> > > returning -EOPNOTSUPP immediately would make O_CREAT|O_DIRECTORY
> > > dependent on the cache state of the dentry.
> > >
> > > There is no separate sysctl for directory creation implemented currently.
> > > Therefore, for the S_ISDIR case, disabling sysctl_protected_regular is
> > > not enough to allow creating a directory in a sticky folder, because that
> > > may surprise users not expecting that O_CREAT|O_DIRECTORY is possible on
> > > newer kernels.
> > >
> > > This feature idea (and some of its description) is taken from the
> > > UAPI group:
> > > https://github.com/uapi-group/kernel-features?tab=readme-ov-file#race-free-creation-and-opening-of-non-file-inodes
> > >
> > > Signed-off-by: Jori Koolstra <jkoolstra@xxxxxxxxx>
> > > ---
> > > fs/namei.c | 116 +++++++++++++++++++++++++++++++++++-------
> > > fs/open.c | 25 +++++----
> > > include/linux/fcntl.h | 6 +++
> > > 3 files changed, 117 insertions(+), 30 deletions(-)
> > >
> > > diff --git a/fs/namei.c b/fs/namei.c
> > > index 0efd395a1a65..6ff0a3c04f02 100644
> > > --- a/fs/namei.c
> > > +++ b/fs/namei.c
> > > @@ -1382,13 +1382,13 @@ int may_linkat(struct mnt_idmap *idmap, const struct path *link)
> > >
> > > /**
> > > * may_create_in_sticky - Check whether an O_CREAT open in a sticky directory
> > > - * should be allowed, or not, on files that already
> > > - * exist.
> > > + * should be allowed, or not, on files/directories that
> > > + * already exist.
> > > * @idmap: idmap of the mount the inode was found from
> > > * @nd: nameidata pathwalk data
> > > * @inode: the inode of the file to open
> > > *
> > > - * Block an O_CREAT open of a FIFO (or a regular file) when:
> > > + * Block an O_CREAT open of a FIFO (or a regular file/directory) when:
> > > * - sysctl_protected_fifos (or sysctl_protected_regular) is enabled
> > > * - the file already exists
> > > * - we are in a sticky directory
> > > @@ -1416,6 +1416,14 @@ static int may_create_in_sticky(struct mnt_idmap *idmap, struct nameidata *nd,
> > > if (likely(!(dir_mode & S_ISVTX)))
> > > return 0;
> > >
> > > + /*
> > > + * There is no separate sysctl for directory creation in sticky
> > > + * folders. Therefore, for the S_ISDIR case, disabling
> > > + * sysctl_protected_regular is not enough to allow creating a
> > > + * directory in a sticky folder, because that may surprise users
> > > + * not expecting that O_CREAT|O_DIRECTORY is possible on newer
> > > + * kernels.
> > > + */
> > > if (S_ISREG(inode->i_mode) && !sysctl_protected_regular)
> > > return 0;
> > >
> > > @@ -1447,6 +1455,12 @@ static int may_create_in_sticky(struct mnt_idmap *idmap, struct nameidata *nd,
> > > "sticky_create_regular");
> > > return -EACCES;
> > > }
> > > +
> > > + if (S_ISDIR(inode->i_mode)) {
> > > + audit_log_path_denied(AUDIT_ANOM_CREAT,
> > > + "sticky_create_dir");
> > > + return -EACCES;
> > > + }
> > > }
> > >
> > > return 0;
> > > @@ -4334,21 +4348,43 @@ static inline int open_to_namei_flags(int flag)
> > >
> > > static int may_o_create(struct mnt_idmap *idmap,
> > > const struct path *dir, struct dentry *dentry,
> > > - umode_t mode)
> > > + int open_flag, umode_t mode)
> > > {
> > > - int error = security_path_mknod(dir, dentry, mode, 0);
> > > + struct inode *dir_inode = dir->dentry->d_inode;
> > > + bool create_dir = O_IS_MKDIR(open_flag);
> > > + int error;
> > > +
> > > + WARN_ON_ONCE(create_dir && !(mode & S_IFDIR));
> > > +
> > > + if (create_dir)
> > > + error = security_path_mkdir(dir, dentry, mode);
> > > + else
> > > + error = security_path_mknod(dir, dentry, mode, 0);
> > > if (error)
> > > return error;
> > >
> > > if (!fsuidgid_has_mapping(dir->dentry->d_sb, idmap))
> > > return -EOVERFLOW;
> > >
> > > - error = inode_permission(idmap, dir->dentry->d_inode,
> > > - MAY_WRITE | MAY_EXEC);
> > > + error = inode_permission(idmap, dir_inode, MAY_WRITE | MAY_EXEC);
> > > if (error)
> > > return error;
> > >
> > > - return security_inode_create(dir->dentry->d_inode, dentry, mode);
> > > + if (create_dir)
> > > + error = security_inode_mkdir(dir_inode, dentry, mode);
> > > + else
> > > + error = security_inode_create(dir_inode, dentry, mode);
> > > +
> > > + return error;
> > > +}
> > > +
> > > +static inline umode_t o_create_mode(struct mnt_idmap *idmap,
> > > + const struct inode *dir, int open_flag, umode_t mode)
> > > +{
> > > + if (O_IS_MKDIR(open_flag))
> > > + return vfs_prepare_mode(idmap, dir, mode, S_IRWXUGO | S_ISVTX, S_IFDIR);
> > > + else
> > > + return vfs_prepare_mode(idmap, dir, mode, S_IALLUGO, S_IFREG);
> > > }
> > >
> > > /**
> > > @@ -4384,8 +4420,9 @@ static struct dentry *atomic_open(const struct path *path, struct dentry *dentry
> > >
> > > file->__f_path.dentry = DENTRY_NOT_SET;
> > > file->__f_path.mnt = path->mnt;
> > > +
> > > error = dir_inode->i_op->atomic_open(dir_inode, dentry, file,
> > > - open_to_namei_flags(open_flag), mode);
> > > + open_to_namei_flags(open_flag), mode);
> > > d_lookup_done(dentry);
> > >
> > > if (!error) {
> > > @@ -4427,12 +4464,32 @@ static struct dentry *atomic_open(const struct path *path, struct dentry *dentry
> > > */
> > > audit_inode_child(dir_inode, dentry, AUDIT_TYPE_CHILD_CREATE);
> > > error = create_error;
> > > + } else if (O_IS_MKDIR(open_flag) && error == -ENOENT) {
> > > + /*
> > > + * If the underlying filesystem does not implement
> > > + * O_CREAT|O_DIRECTORY, it strips the O_CREAT bit and
> > > + * continues as a lookup. We can't simply return
> > > + * -EOPNOTSUPP from unsupported ->atomic_open()
> > > + * implementations because the dentry might be in the
> > > + * dcache. In that case, lookup_open() returns before
> > > + * reaching ->atomic_open(), and hence whether you get
> > > + * -EOPNOTSUPP on O_CREAT|O_DIRECTORY would not only
> > > + * depend on the underlying filesystem, but also on
> > > + * the state of the dcache. Still, we must make an
> > > + * effort to differentiate a regular -ENOENT from the
> > > + * unsupported O_CREAT|O_DIRECTORY case.
> > > + */
> > > + error = -EOPNOTSUPP;
> > > }
> > > dput(dentry);
> > > dentry = ERR_PTR(error);
> > > } else {
> > > - if (file->f_mode & FMODE_CREATED)
> > > - fsnotify_create(dir_inode, dentry);
> > > + if (file->f_mode & FMODE_CREATED) {
> > > + if (d_is_dir(dentry))
> > > + fsnotify_mkdir(dir_inode, dentry);
> > > + else
> > > + fsnotify_create(dir_inode, dentry);
> > > + }
> > > if (file->f_mode & FMODE_OPENED)
> > > fsnotify_open(file);
> > > }
> > > @@ -4441,6 +4498,9 @@ static struct dentry *atomic_open(const struct path *path, struct dentry *dentry
> > > return dentry;
> > > }
> > >
> > > +static inline
> > > +struct dentry *vfs_mkdir_no_perm(struct mnt_idmap *, struct inode *, struct dentry *,
> > > + umode_t, struct delegated_inode *);
> > > /*
> > > * Look up and maybe create and open the last component.
> > > *
> > > @@ -4462,6 +4522,7 @@ static struct dentry *lookup_open(struct nameidata *nd, struct file *file,
> > > struct mnt_idmap *idmap;
> > > struct dentry *dir = nd->path.dentry;
> > > struct inode *dir_inode = dir->d_inode;
> > > + bool create_dir = O_IS_MKDIR(op->open_flag);
> > > int open_flag;
> > > struct dentry *dentry;
> > > int error, create_error;
> > > @@ -4474,6 +4535,9 @@ static struct dentry *lookup_open(struct nameidata *nd, struct file *file,
> > > mode = op->mode;
> > > create_error = 0;
> > >
> > > + if (create_dir && dir_inode->i_op->atomic_open)
> > > + open_flag &= ~O_CREAT;
> > > +
> > > if (open_flag & (O_CREAT | O_TRUNC | O_WRONLY | O_RDWR)) {
> > > got_write = !mnt_want_write(nd->path.mnt);
> > > /*
> > > @@ -4534,10 +4598,10 @@ static struct dentry *lookup_open(struct nameidata *nd, struct file *file,
> > > if (open_flag & O_CREAT) {
> > > if (open_flag & O_EXCL)
> > > open_flag &= ~O_TRUNC;
> > > - mode = vfs_prepare_mode(idmap, dir_inode, mode, mode, mode);
> > > + mode = o_create_mode(idmap, dir_inode, open_flag, mode);
> > > if (likely(got_write))
> > > create_error = may_o_create(idmap, &nd->path,
> > > - dentry, mode);
> > > + dentry, open_flag, mode);
> > > else
> > > create_error = -EROFS;
> > > }
> > > @@ -4582,12 +4646,25 @@ static struct dentry *lookup_open(struct nameidata *nd, struct file *file,
> > > goto out_dput;
> > > }
> > >
> > > - if (!dir_inode->i_op->create) {
> > > + /* mimic operation missing errnos of vfs_mkdir/vfs_create */
> > > + if (create_dir && !dir_inode->i_op->mkdir) {
> > > + error = -EPERM;
> > > + goto out_dput;
> > > + }
> > > + if (!create_dir && !dir_inode->i_op->create) {
> > > error = -EACCES;
> > > goto out_dput;
> > > }
> > >
> > > - error = vfs_create_no_perm(idmap, dentry, mode, &delegated_inode);
> > > + if (create_dir) {
> > > + struct dentry *res = vfs_mkdir_no_perm(idmap, dir_inode, dentry, mode,
> > > + &delegated_inode);
> >
> > So, I think this is broken. Whatever vfs_mkdir_no_perm() returns is
> > passed to do_open(). Kernfs makes that buggy.
> >
> > cgroup, cgroup2, and resctrl are all implemented on top of kernfs. And
> > kernfs ->mkdir:: iop never instantiates the dentry.
> >
> > So that means e.g.,
> >
> > openat(cgroup_dir, "subdir", O_CREAT|O_DIRECTORY) creates a cgroup
> > and then fails with ENOTDIR.
> >
> > So the negative dentry gets handed out and now userspace holds an fd
> > with that negative dentry. So say userspace does fchown() to 1000 and
> > then fchmod() with the sticky bit and then you get a NULL deref. I have
> > reproduced this.
> >
> > Neil can correct me but the fix might be to check whether the dentry is
> > negative in lookup_open() and re-lookup nd->last with the parent still locked.
> > I think that's what nfsd_create_locked() and cachefiles_get_directory() do
> > after vfs_mkdir().
>
> nfsd_create_locked() used to do that before vfs_mkdir() could return a
> dentry, but it doesn't any more. The reason was because
> d_splice_alias() on might return a different dentry.
> In this case we want the same dentry, but we need to do a lookup on it.
>
> I'd rather fix this in kernfs, but maybe that is a longer-term goal.
>
> The comment in kernfs_dop_revalidate() suggests the we should d_drop()
> the negative dentry and d_alloc_parallel() a new one and ->lookup that.
> I'm not certain that is needed if we keep the parent locked, but we
> would need to be certain.
> We at least need to d_drop() the dentry before ->lookup as ->lookup
> cannot handle hashed dentries and a hashed-negative dentry is passed
> to ->mkdir.
>
> I wonder if we could just disable O_CREATE|O_DIRECTORY on kernfs ....
> probably not.
>
> Summary: I think that if vfs_mkdir() returns NULL (success) but the
> dentry is negative, we need to d_drop() and call ->lookup with a big
> comment about kernfs. But we need to double-check that this will do the
> right thing with ->d_time (I think it will).
> We also need to think carefully about races with
> kernfs_dop_revalidate(), which could happen concurrently with the
> ->lookup.
I've thought a bit more about this ... I think that doing a lookup after
the vfs_mkdir() results in a negative is a bit ugly. It assumes things
about the fs that I would rather not assume.
I would rather have the current proposed code check for a negative
dentry, and fail with -EIO or similar.
We could then "fix" kernfs by providing an atomic_open which does the
mkdir and then the lookup, and provides the dentry to
finish_no_lookup().
That way we don't need to change kernfs mkdir.
Note that tracefs_syscall_mkdir() has the same behaviour as
kernfs_iop_mkdir, and could have the same fix.
NeilBrown