Re: [PATCH v6 07/12] vfs: add O_CREAT|O_DIRECTORY to open*(2)

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 <[email protected]>
> ---
>  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().

Here's the callchain:

  Common path: create the cgroup, come back with a negative dentry

  __x64_sys_openat
   do_sys_openat2                          fs/open.c
    build_open_flags                       O_IS_MKDIR(flags) -> op->mode = mode | S_IFDIR
                                           O_DIRECTORY -> lookup_flags |= LOOKUP_DIRECTORY
    do_file_open                           fs/namei.c
       lookup_fast_for_open                dcache miss -> NULL
       lookup_open
        mnt_want_write
        inode_lock_nested(dir, I_MUTEX_PARENT)
        d_lookup -> NULL
        d_alloc_parallel                   in-lookup dentry
        o_create_mode                      S_IFDIR | 0755 & ~umask
        may_o_create
         security_path_mkdir
         inode_permission(MAY_WRITE|MAY_EXEC)
          kernfs_iop_permission
           generic_permission              owner of a 0755 dir: allowed
         security_inode_mkdir
        dir_inode->i_op->lookup
         kernfs_iop_lookup
          kernfs_find_ns -> NULL           node does not exist yet
          d_splice_alias(NULL, dentry)     -> __d_add(): hashed, NEGATIVE
         try_break_deleg
         dir->i_op->mkdir
          kernfs_iop_mkdir
           scops->mkdir
            cgroup_mkdir                   no capable() check
             cgroup_create                 owner = current_fsuid()
             css_populate_dir
             kernfs_activate               cgroup now exists
           return ERR_PTR(0)               == NULL, dentry untouched
         de == NULL -> keep original dentry
         fsnotify_mkdir(dir, dentry)       fine with a negative dentry
        dentry = res                       still negative
        file->f_mode |= FMODE_CREATED
        inode_unlock(dir); mnt_drop_write
       FMODE_CREATED set ->
        dput(nd->path.dentry)
        nd->path.dentry = dentry           negative dentry becomes the "opened" path
        return NULL
      do_open
       open_flag & O_CREAT ->
        may_create_in_sticky(idmap, nd, d_backing_inode(nd->path.dentry))
                                           inode == NULL

  Scenario 1, parent not sticky: cgroup created, ENOTDIR returned

       may_create_in_sticky
        if (!(dir_mode & S_ISVTX)) return 0;     inode never touched
       (nd->flags & LOOKUP_DIRECTORY) && !d_can_lookup(nd->path.dentry)
                                           DCACHE_MISS_TYPE -> false
        return -ENOTDIR
      terminate_walk
      fput_close(file)
     return ERR_PTR(-ENOTDIR)              "child" cgroup stays behind

  A second openat of the same name works because kernfs_dop_revalidate() sees the parent's revision changed, d_invalidate()s the stale negative dentry, and the fresh kernfs_iop_lookup() now
  finds the node.

  Scenario 2, sticky parent: NULL dereference

  Setup, one-time, as root (what systemd's Delegate=yes does for user.slice/user-1000.slice/[email protected]):

  fchown(pfd, 1000, 1000)
   chown_common -> notify_change -> kernfs_iop_setattr -> __kernfs_setattr

  Then as uid 1000:

  fchmod(pfd, 01755)
   chmod_common
    newattrs.ia_mode = (mode & S_IALLUGO) | ...   S_ISVTX is inside S_IALLUGO
    notify_change
     setattr_prepare
      inode_owner_or_capable                    owner -> ok
     kernfs_iop_setattr
      __kernfs_setattr                          kn->mode = ia_mode, no masking
      setattr_copy                              inode->i_mode = 01755

  and the open, same chain as above until may_create_in_sticky(), now with nd->dir_mode = 01755:

       may_create_in_sticky
        if (!(dir_mode & S_ISVTX)) return 0;     not taken
        if (S_ISREG(inode->i_mode) && ...)       inode == NULL
                                                 -> KASAN null-ptr-deref