[RFC PATCH v5 0/2] vfs: add O_CREAT|O_DIRECTORY to open*(2)

[RFC PATCH v5 0/2] vfs: add O_CREAT|O_DIRECTORY to open*(2)

[Jori Koolstra]

This series implements new semantics for the O_CREAT|O_DIRECTORY flag
combination for open*(2): perform a mkdir and open the resulting
directory; return a pinning fd (which mkdir does not).

Three comments from me upfront:

- This patch EINVAL bans O_CREAT|O_DIRECTORY in each individual
  atomic_open implementation. An argument to do it in the generic
  atomic_open() in fs/namei.c instead is to give out of tree
  filesystems more time to implement (or block) O_CREAT|O_DIRECTORY.

- If we create a regular file with mknod, before creation
  security_path_mknod() is called, and after creation
  security_path_post_mknod(). If we create a regular file using O_CREAT
  (and this is also pre-patch) only security_path_mknod() is called. Is
  this the correct behaviour?

- open_last_lookups() locks the parent inode like like: 

		inode_lock(dir->d_inode);

  should this perhaps be

		inode_lock_nested(dir, I_MUTEX_PARENT);

  to stay consistent with the start_dirop() path that is used by
  filename_create() for instance in mknod(2)? I get that we are only
  locking one inode here at most, so it does not really matter, but
  now one regular file create path does set the lockdep and the other
  does not.

Changes:
v5: fixed Sashiko reported issues [1]. Moved to EINVAL banning
O_CREAT|O_DIRECTORY in each individual atomic_open implementation
instead of in the generic atomic_open() in fs/namei.c.
v3/4: fixed syzbot reported bugs
v2: don't introduce a new syscall (mkdirat2) but implement this
functionality as O_CREAT|O_DIRECTORY in open*(2).

[1]: 
https://sashiko.dev/#/patchset/20260518165237.2084042-1-jkoolstra%40xs4all.nl

Jori Koolstra (2):
  vfs: add O_CREAT|O_DIRECTORY to open*(2)
  selftest: add tests for open*(O_CREAT|O_DIRECTORY)

 fs/9p/vfs_inode.c                             |   3 +
 fs/9p/vfs_inode_dotl.c                        |   3 +
 fs/ceph/file.c                                |   3 +
 fs/fuse/dir.c                                 |   3 +
 fs/gfs2/inode.c                               |   3 +
 fs/namei.c                                    | 177 ++++++++++------
 fs/nfs/dir.c                                  |   3 +
 fs/nfs/file.c                                 |   3 +
 fs/open.c                                     |  25 ++-
 fs/smb/client/dir.c                           |   3 +
 fs/vboxsf/dir.c                               |   3 +
 include/linux/fcntl.h                         |   2 +
 .../testing/selftests/filesystems/.gitignore  |   1 +
 tools/testing/selftests/filesystems/Makefile  |   4 +-
 tools/testing/selftests/filesystems/fclog.c   |   1 +
 .../filesystems/open_o_creat_o_dir.c          | 197 ++++++++++++++++++
 16 files changed, 362 insertions(+), 72 deletions(-)
 create mode 100644 tools/testing/selftests/filesystems/open_o_creat_o_dir.c

-- 
2.54.0

[RFC PATCH v5 1/2] vfs: add O_CREAT|O_DIRECTORY to open*(2)

[Jori Koolstra]

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
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.

This commit also changes the error returned when a filesystem operation
is unsupported (i_op->mkdir/creat) to EOPNOTSUPP. Current error values
are inconsistent (both EPERM and EACCES are used) and confusing.

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@xs4all.nl>
---
 fs/9p/vfs_inode.c      |   3 +
 fs/9p/vfs_inode_dotl.c |   3 +
 fs/ceph/file.c         |   3 +
 fs/fuse/dir.c          |   3 +
 fs/gfs2/inode.c        |   3 +
 fs/namei.c             | 177 +++++++++++++++++++++++++++--------------
 fs/nfs/dir.c           |   3 +
 fs/nfs/file.c          |   3 +
 fs/open.c              |  25 +++---
 fs/smb/client/dir.c    |   3 +
 fs/vboxsf/dir.c        |   3 +
 include/linux/fcntl.h  |   2 +
 12 files changed, 161 insertions(+), 70 deletions(-)

diff --git a/fs/9p/vfs_inode.c b/fs/9p/vfs_inode.c
index f468acb8ee7d..d1925333d327 100644
--- a/fs/9p/vfs_inode.c
+++ b/fs/9p/vfs_inode.c
@@ -771,6 +771,9 @@ v9fs_vfs_atomic_open(struct inode *dir, struct dentry *dentry,
 	struct inode *inode;
 	int p9_omode;
 
+	if ((flags & O_MKDIR_MASK) == O_MKDIR_MASK)
+		return -EINVAL;
+
 	if (d_in_lookup(dentry)) {
 		struct dentry *res = v9fs_vfs_lookup(dir, dentry, 0);
 		if (res || d_really_is_positive(dentry))
diff --git a/fs/9p/vfs_inode_dotl.c b/fs/9p/vfs_inode_dotl.c
index 141fb54db65d..9f4b865d07d7 100644
--- a/fs/9p/vfs_inode_dotl.c
+++ b/fs/9p/vfs_inode_dotl.c
@@ -239,6 +239,9 @@ v9fs_vfs_atomic_open_dotl(struct inode *dir, struct dentry *dentry,
 	struct v9fs_session_info *v9ses;
 	struct posix_acl *pacl = NULL, *dacl = NULL;
 
+	if ((flags & O_MKDIR_MASK) == O_MKDIR_MASK)
+		return -EINVAL;
+
 	if (d_in_lookup(dentry)) {
 		struct dentry *res = v9fs_vfs_lookup(dir, dentry, 0);
 		if (res || d_really_is_positive(dentry))
diff --git a/fs/ceph/file.c b/fs/ceph/file.c
index d54d71669176..9707d9ed17b6 100644
--- a/fs/ceph/file.c
+++ b/fs/ceph/file.c
@@ -813,6 +813,9 @@ int ceph_atomic_open(struct inode *dir, struct dentry *dentry,
 	if (dentry->d_name.len > NAME_MAX)
 		return -ENAMETOOLONG;
 
+	if ((flags & O_MKDIR_MASK) == O_MKDIR_MASK)
+		return -EINVAL;
+
 	err = ceph_wait_on_conflict_unlink(dentry);
 	if (err)
 		return err;
diff --git a/fs/fuse/dir.c b/fs/fuse/dir.c
index b658b6baf72f..4c59992b9867 100644
--- a/fs/fuse/dir.c
+++ b/fs/fuse/dir.c
@@ -940,6 +940,9 @@ static int fuse_atomic_open(struct inode *dir, struct dentry *entry,
 	if (fuse_is_bad(dir))
 		return -EIO;
 
+	if ((flags & O_MKDIR_MASK) == O_MKDIR_MASK)
+		return -EINVAL;
+
 	if (d_in_lookup(entry)) {
 		struct dentry *res = fuse_lookup(dir, entry, 0);
 		if (res || d_really_is_positive(entry))
diff --git a/fs/gfs2/inode.c b/fs/gfs2/inode.c
index e9bf4879c07f..21c6544fbee5 100644
--- a/fs/gfs2/inode.c
+++ b/fs/gfs2/inode.c
@@ -1384,6 +1384,9 @@ static int gfs2_atomic_open(struct inode *dir, struct dentry *dentry,
 {
 	bool excl = !!(flags & O_EXCL);
 
+	if ((flags & O_MKDIR_MASK) == O_MKDIR_MASK)
+		return -EINVAL;
+
 	if (d_in_lookup(dentry)) {
 		struct dentry *d = __gfs2_lookup(dir, dentry, file);
 		if (file->f_mode & FMODE_OPENED) {
diff --git a/fs/namei.c b/fs/namei.c
index c7fac83c9a85..9d9529ef30c4 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -2777,9 +2777,14 @@ static const char *path_init(struct nameidata *nd, unsigned flags)
 	return s;
 }
 
+static inline bool trailing_slashes(struct nameidata *nd)
+{
+	return (bool)nd->last.name[nd->last.len];
+}
+
 static inline const char *lookup_last(struct nameidata *nd)
 {
-	if (nd->last_type == LAST_NORM && nd->last.name[nd->last.len])
+	if (nd->last_type == LAST_NORM && trailing_slashes(nd))
 		nd->flags |= LOOKUP_FOLLOW | LOOKUP_DIRECTORY;
 
 	return walk_component(nd, WALK_TRAILING);
@@ -4166,6 +4171,16 @@ static inline umode_t vfs_prepare_mode(struct mnt_idmap *idmap,
 	return mode;
 }
 
+static int __vfs_create(struct mnt_idmap *idmap, struct dentry *dentry, umode_t mode,
+			struct delegated_inode *di, bool excl)
+{
+	struct inode *dir = d_inode(dentry->d_parent);
+	int error = try_break_deleg(dir, di);
+	if (error)
+		return error;
+	return dir->i_op->create(idmap, dir, dentry, mode, excl);
+}
+
 /**
  * vfs_create - create new file
  * @idmap:	idmap of the mount the inode was found from
@@ -4192,16 +4207,14 @@ int vfs_create(struct mnt_idmap *idmap, struct dentry *dentry, umode_t mode,
 		return error;
 
 	if (!dir->i_op->create)
-		return -EACCES;	/* shouldn't it be ENOSYS? */
+		return -EOPNOTSUPP;
 
 	mode = vfs_prepare_mode(idmap, dir, mode, S_IALLUGO, S_IFREG);
 	error = security_inode_create(dir, dentry, mode);
 	if (error)
 		return error;
-	error = try_break_deleg(dir, di);
-	if (error)
-		return error;
-	error = dir->i_op->create(idmap, dir, dentry, mode, true);
+
+	error = __vfs_create(idmap, dentry, mode, di, true);
 	if (!error)
 		fsnotify_create(dir, dentry);
 	return error;
@@ -4321,21 +4334,32 @@ 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)
+			umode_t mode, bool create_dir)
 {
-	int error = security_path_mknod(dir, dentry, mode, 0);
+	struct inode *dir_inode = dir->dentry->d_inode;
+	int error;
+
+	error = create_dir ? security_path_mkdir(dir, dentry, mode)
+			   : 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);
+	return create_dir ? security_inode_mkdir(dir_inode, dentry, mode)
+			  : security_inode_create(dir_inode, dentry, mode);
+}
+
+static inline umode_t o_create_mode(struct mnt_idmap *idmap,
+		const struct inode *dir, umode_t mode, bool create_dir)
+{
+	return create_dir ? vfs_prepare_mode(idmap, dir, mode, S_IRWXUGO | S_ISVTX, 0)
+			  : vfs_prepare_mode(idmap, dir, mode, S_IALLUGO, S_IFREG);
 }
 
 /*
@@ -4388,6 +4412,9 @@ static struct dentry *atomic_open(const struct path *path, struct dentry *dentry
 	return dentry;
 }
 
+static struct dentry *__vfs_mkdir(struct mnt_idmap *, struct inode *,
+			 struct dentry *, umode_t,
+			 struct delegated_inode *);
 /*
  * Look up and maybe create and open the last component.
  *
@@ -4412,8 +4439,9 @@ static struct dentry *lookup_open(struct nameidata *nd, struct file *file,
 	struct inode *dir_inode = dir->d_inode;
 	int open_flag = op->open_flag;
 	struct dentry *dentry;
-	int error, create_error = 0;
+	int error = 0, create_error = 0;
 	umode_t mode = op->mode;
+	bool create_dir = (open_flag & O_MKDIR_MASK) == O_MKDIR_MASK;
 	DECLARE_WAIT_QUEUE_HEAD_ONSTACK(wq);
 
 	if (unlikely(IS_DEADDIR(dir_inode)))
@@ -4462,10 +4490,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->d_inode, mode, mode, mode);
+		mode = o_create_mode(idmap, dir_inode, mode, create_dir);
 		if (likely(got_write))
 			create_error = may_o_create(idmap, &nd->path,
-						    dentry, mode);
+						    dentry, mode, create_dir);
 		else
 			create_error = -EROFS;
 	}
@@ -4494,29 +4522,37 @@ static struct dentry *lookup_open(struct nameidata *nd, struct file *file,
 		}
 	}
 
+	if (unlikely(create_error) && !dentry->d_inode) {
+		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, 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;
+		if ((create_dir && !dir_inode->i_op->mkdir)
+		    || (!create_dir && !dir_inode->i_op->create)) {
+			error = -EOPNOTSUPP;
 			goto out_dput;
 		}
 
-		error = dir_inode->i_op->create(idmap, dir_inode, dentry,
-						mode, open_flag & O_EXCL);
+		if (create_dir) {
+			struct dentry *res = __vfs_mkdir(idmap, dir_inode, dentry, mode,
+							 delegated_inode);
+			if (IS_ERR(res))
+				error = PTR_ERR(res);
+			else
+				dentry = res;
+		} else {
+			error = __vfs_create(idmap, dentry, mode, delegated_inode,
+					     open_flag & O_EXCL);
+		}
 		if (error)
 			goto out_dput;
 	}
-	if (unlikely(create_error) && !dentry->d_inode) {
-		error = create_error;
-		goto out_dput;
-	}
+
 	return dentry;
 
 out_dput:
@@ -4524,17 +4560,12 @@ static struct dentry *lookup_open(struct nameidata *nd, struct file *file,
 	return ERR_PTR(error);
 }
 
-static inline bool trailing_slashes(struct nameidata *nd)
-{
-	return (bool)nd->last.name[nd->last.len];
-}
-
 static struct dentry *lookup_fast_for_open(struct nameidata *nd, int open_flag)
 {
 	struct dentry *dentry;
 
 	if (open_flag & O_CREAT) {
-		if (trailing_slashes(nd))
+		if (trailing_slashes(nd) && !(open_flag & O_DIRECTORY))
 			return ERR_PTR(-EISDIR);
 
 		/* Don't bother on an O_EXCL create */
@@ -4605,13 +4636,17 @@ static const char *open_last_lookups(struct nameidata *nd,
 		 */
 	}
 	if (open_flag & O_CREAT)
-		inode_lock(dir->d_inode);
+		inode_lock_nested(dir->d_inode, I_MUTEX_PARENT);
 	else
 		inode_lock_shared(dir->d_inode);
 	dentry = lookup_open(nd, file, op, got_write, &delegated_inode);
 	if (!IS_ERR(dentry)) {
-		if (file->f_mode & FMODE_CREATED)
-			fsnotify_create(dir->d_inode, dentry);
+		if (file->f_mode & FMODE_CREATED) {
+			if (open_flag & O_DIRECTORY)
+				fsnotify_mkdir(dir->d_inode, dentry);
+			else
+				fsnotify_create(dir->d_inode, dentry);
+		}
 		if (file->f_mode & FMODE_OPENED)
 			fsnotify_open(file);
 	}
@@ -4672,12 +4707,16 @@ static int do_open(struct nameidata *nd,
 	if (open_flag & O_CREAT) {
 		if ((open_flag & O_EXCL) && !(file->f_mode & FMODE_CREATED))
 			return -EEXIST;
-		if (d_is_dir(nd->path.dentry))
-			return -EISDIR;
-		error = may_create_in_sticky(idmap, nd,
-					     d_backing_inode(nd->path.dentry));
-		if (unlikely(error))
-			return error;
+		// there are no special rules for creating dirs in a sticky bit dir
+		if (!(open_flag & O_DIRECTORY)) {
+			if (d_is_dir(nd->path.dentry))
+				return -EISDIR;
+
+			error = may_create_in_sticky(idmap, nd,
+						     d_backing_inode(nd->path.dentry));
+			if (unlikely(error))
+				return error;
+		}
 	}
 	if ((nd->flags & LOOKUP_DIRECTORY) && !d_can_lookup(nd->path.dentry))
 		return -ENOTDIR;
@@ -5039,7 +5078,7 @@ struct file *dentry_create(struct path *path, int flags, umode_t mode,
 		path->dentry = dir;
 		mode = vfs_prepare_mode(idmap, dir_inode, mode, S_IALLUGO, S_IFREG);
 
-		create_error = may_o_create(idmap, path, dentry, mode);
+		create_error = may_o_create(idmap, path, dentry, mode, false);
 		if (create_error)
 			flags &= ~O_CREAT;
 
@@ -5207,6 +5246,37 @@ SYSCALL_DEFINE3(mknod, const char __user *, filename, umode_t, mode, unsigned, d
 	return filename_mknodat(AT_FDCWD, name, mode, dev);
 }
 
+static struct dentry *__vfs_mkdir(struct mnt_idmap *idmap, struct inode *dir,
+			 struct dentry *dentry, umode_t mode,
+			 struct delegated_inode *di)
+{
+	int error;
+	unsigned max_links = dir->i_sb->s_max_links;
+	struct dentry *de;
+
+	error = -EMLINK;
+	if (max_links && dir->i_nlink >= max_links)
+		goto err;
+
+	error = try_break_deleg(dir, di);
+	if (error)
+		goto err;
+
+	de = dir->i_op->mkdir(idmap, dir, dentry, mode);
+	if (IS_ERR(de)) {
+		error = PTR_ERR(de);
+		goto err;
+	}
+	if (de) {
+		dput(dentry);
+		dentry = de;
+	}
+	return dentry;
+
+err:
+	return ERR_PTR(error);
+}
+
 /**
  * vfs_mkdir - create directory returning correct dentry if possible
  * @idmap:		idmap of the mount the inode was found from
@@ -5231,17 +5301,16 @@ SYSCALL_DEFINE3(mknod, const char __user *, filename, umode_t, mode, unsigned, d
  */
 struct dentry *vfs_mkdir(struct mnt_idmap *idmap, struct inode *dir,
 			 struct dentry *dentry, umode_t mode,
-			 struct delegated_inode *delegated_inode)
+			 struct delegated_inode *di)
 {
 	int error;
-	unsigned max_links = dir->i_sb->s_max_links;
 	struct dentry *de;
 
 	error = may_create_dentry(idmap, dir, dentry);
 	if (error)
 		goto err;
 
-	error = -EPERM;
+	error = -EOPNOTSUPP;
 	if (!dir->i_op->mkdir)
 		goto err;
 
@@ -5250,22 +5319,12 @@ struct dentry *vfs_mkdir(struct mnt_idmap *idmap, struct inode *dir,
 	if (error)
 		goto err;
 
-	error = -EMLINK;
-	if (max_links && dir->i_nlink >= max_links)
-		goto err;
-
-	error = try_break_deleg(dir, delegated_inode);
-	if (error)
-		goto err;
-
-	de = dir->i_op->mkdir(idmap, dir, dentry, mode);
-	error = PTR_ERR(de);
-	if (IS_ERR(de))
+	de = __vfs_mkdir(idmap, dir, dentry, mode, di);
+	if (IS_ERR(de)) {
+		error = PTR_ERR(de);
 		goto err;
-	if (de) {
-		dput(dentry);
-		dentry = de;
 	}
+	dentry = de;
 	fsnotify_mkdir(dir, dentry);
 	return dentry;
 
diff --git a/fs/nfs/dir.c b/fs/nfs/dir.c
index e9ce1883288c..e44c7598b68e 100644
--- a/fs/nfs/dir.c
+++ b/fs/nfs/dir.c
@@ -2314,6 +2314,9 @@ int nfs_atomic_open_v23(struct inode *dir, struct dentry *dentry,
 	if (dentry->d_name.len > NFS_SERVER(dir)->namelen)
 		return -ENAMETOOLONG;
 
+	if ((open_flags & O_MKDIR_MASK) == O_MKDIR_MASK)
+		return -EINVAL;
+
 	if (open_flags & O_CREAT) {
 		error = nfs_do_create(dir, dentry, mode, open_flags);
 		if (!error) {
diff --git a/fs/nfs/file.c b/fs/nfs/file.c
index 25048a3c2364..467f6bc707da 100644
--- a/fs/nfs/file.c
+++ b/fs/nfs/file.c
@@ -52,6 +52,9 @@ int nfs_check_flags(int flags)
 	if ((flags & (O_APPEND | O_DIRECT)) == (O_APPEND | O_DIRECT))
 		return -EINVAL;
 
+	if ((flags & O_MKDIR_MASK) == O_MKDIR_MASK)
+		return -EINVAL;
+
 	return 0;
 }
 EXPORT_SYMBOL_GPL(nfs_check_flags);
diff --git a/fs/open.c b/fs/open.c
index 681d405bc61e..865ea6f70e8c 100644
--- a/fs/open.c
+++ b/fs/open.c
@@ -1209,29 +1209,30 @@ inline int build_open_flags(const struct open_how *how, struct open_flags *op)
 	if (WILL_CREATE(flags)) {
 		if (how->mode & ~S_IALLUGO)
 			return -EINVAL;
-		op->mode = how->mode | S_IFREG;
+		if ((flags & (O_MKDIR_MASK)) == O_MKDIR_MASK)
+			op->mode = how->mode | S_IFDIR;
+		else
+			op->mode = how->mode | S_IFREG;
 	} else {
 		if (how->mode != 0)
 			return -EINVAL;
 		op->mode = 0;
 	}
 
-	/*
-	 * Block bugs where O_DIRECTORY | O_CREAT created regular files.
-	 * Note, that blocking O_DIRECTORY | O_CREAT here also protects
-	 * O_TMPFILE below which requires O_DIRECTORY being raised.
-	 */
-	if ((flags & (O_DIRECTORY | O_CREAT)) == (O_DIRECTORY | O_CREAT))
-		return -EINVAL;
-
 	/* Now handle the creative implementation of O_TMPFILE. */
 	if (flags & __O_TMPFILE) {
 		/*
 		 * In order to ensure programs get explicit errors when trying
 		 * to use O_TMPFILE on old kernels we enforce that O_DIRECTORY
-		 * is raised alongside __O_TMPFILE.
+		 * is raised alongside __O_TMPFILE, but without O_CREAT. The
+		 * reason for disallowing O_CREAT|O_TMPFILE is that
+		 * O_DIRECTORY|O_CREAT used to work and created a regular file
+		 * if nothing existed at the open path. Hence, allowing the
+		 * combination would have caused O_CREAT|O_TMPFILE to create a
+		 * regular (non-temporary) file on old kernels, while the caller
+		 * would believe they created an actual O_TMPFILE.
 		 */
-		if (!(flags & O_DIRECTORY))
+		if (!(flags & O_DIRECTORY) || (flags & O_CREAT))
 			return -EINVAL;
 		if (!(acc_mode & MAY_WRITE))
 			return -EINVAL;
@@ -1268,6 +1269,8 @@ inline int build_open_flags(const struct open_how *how, struct open_flags *op)
 	op->intent = flags & O_PATH ? 0 : LOOKUP_OPEN;
 
 	if (flags & O_CREAT) {
+		if ((flags & O_DIRECTORY) && (acc_mode & MAY_WRITE))
+			return -EISDIR;
 		op->intent |= LOOKUP_CREATE;
 		if (flags & O_EXCL) {
 			op->intent |= LOOKUP_EXCL;
diff --git a/fs/smb/client/dir.c b/fs/smb/client/dir.c
index e4295a5b55b3..ec8c54c91261 100644
--- a/fs/smb/client/dir.c
+++ b/fs/smb/client/dir.c
@@ -526,6 +526,9 @@ int cifs_atomic_open(struct inode *dir, struct dentry *direntry,
 	if (unlikely(cifs_forced_shutdown(cifs_sb)))
 		return smb_EIO(smb_eio_trace_forced_shutdown);
 
+	if ((oflags & O_MKDIR_MASK) == O_MKDIR_MASK)
+		return -EINVAL;
+
 	/*
 	 * Posix open is only called (at lookup time) for file create now. For
 	 * opens (rather than creates), because we do not know if it is a file
diff --git a/fs/vboxsf/dir.c b/fs/vboxsf/dir.c
index 42bedc4ec7af..aef5ca6730be 100644
--- a/fs/vboxsf/dir.c
+++ b/fs/vboxsf/dir.c
@@ -318,6 +318,9 @@ static int vboxsf_dir_atomic_open(struct inode *parent, struct dentry *dentry,
 	u64 handle;
 	int err;
 
+	if ((flags & O_MKDIR_MASK) == O_MKDIR_MASK)
+		return -EINVAL;
+
 	if (d_in_lookup(dentry)) {
 		struct dentry *res = vboxsf_dir_lookup(parent, dentry, 0);
 		if (res || d_really_is_positive(dentry))
diff --git a/include/linux/fcntl.h b/include/linux/fcntl.h
index a332e79b3207..e31f3a57f07c 100644
--- a/include/linux/fcntl.h
+++ b/include/linux/fcntl.h
@@ -12,6 +12,8 @@
 	 FASYNC	| O_DIRECT | O_LARGEFILE | O_DIRECTORY | O_NOFOLLOW | \
 	 O_NOATIME | O_CLOEXEC | O_PATH | __O_TMPFILE)
 
+#define O_MKDIR_MASK	(O_CREAT | O_DIRECTORY)
+
 /* List of all valid flags for the how->resolve argument: */
 #define VALID_RESOLVE_FLAGS \
 	(RESOLVE_NO_XDEV | RESOLVE_NO_MAGICLINKS | RESOLVE_NO_SYMLINKS | \
-- 
2.54.0

Re: [RFC PATCH v5 1/2] vfs: add O_CREAT|O_DIRECTORY to open*(2)

[NeilBrown]

On Tue, 26 May 2026, 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 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
> 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.
> 
> This commit also changes the error returned when a filesystem operation
> is unsupported (i_op->mkdir/creat) to EOPNOTSUPP. Current error values
> are inconsistent (both EPERM and EACCES are used) and confusing.

This commit description is good at justifying the change.  But it is not
so good at explaining the details of how the change happens so that a
reviewer can match the code with the explanation.  There is a lot
happening here and I think I would rather it were split into a few
separate patches so it is easier to comprehend.

I *think* the intention is that this new functionality would not be
available on filesystems which support ->atomic_open, until those
filesystems are given tailored support.  Is that correct?  Spelling that
out at the start would be useful.  I would implement that by checking
for O_DIRECTORY in atomic_open() and leaving the filesystems untouched.
Then a patch later in the series would move that test into the various
filesystems.

I'm not convinced the change to EOPNOTSUPP is a good idea.  It should
be a separate patch so that it can be reviewed separately.

EOPNOTSUPP is documented as
       EOPNOTSUPP      Operation not supported on socket (POSIX.1-2001).

and we have no sockets here!
ENOTSUP might be a reasonable choice - though it has the same numeric
value and it often confused with EOPNOTSUPP in the kernel.

I don't think EACCES is particularly good, but I don't think anything
else is enough better to justify a change.

Other comments interleaved...

> 
> 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@xs4all.nl>
> ---
>  fs/9p/vfs_inode.c      |   3 +
>  fs/9p/vfs_inode_dotl.c |   3 +
>  fs/ceph/file.c         |   3 +
>  fs/fuse/dir.c          |   3 +
>  fs/gfs2/inode.c        |   3 +
>  fs/namei.c             | 177 +++++++++++++++++++++++++++--------------
>  fs/nfs/dir.c           |   3 +
>  fs/nfs/file.c          |   3 +
>  fs/open.c              |  25 +++---
>  fs/smb/client/dir.c    |   3 +
>  fs/vboxsf/dir.c        |   3 +
>  include/linux/fcntl.h  |   2 +
>  12 files changed, 161 insertions(+), 70 deletions(-)
> 
> diff --git a/fs/9p/vfs_inode.c b/fs/9p/vfs_inode.c
> index f468acb8ee7d..d1925333d327 100644
> --- a/fs/9p/vfs_inode.c
> +++ b/fs/9p/vfs_inode.c
> @@ -771,6 +771,9 @@ v9fs_vfs_atomic_open(struct inode *dir, struct dentry *dentry,
>  	struct inode *inode;
>  	int p9_omode;
>  
> +	if ((flags & O_MKDIR_MASK) == O_MKDIR_MASK)
> +		return -EINVAL;
> +

O_MKDIR_MASK is only ever used in this construct.  I would prefer
something like

static inline bool O_IS_MKDIR(unsigned flags)
{
    return (flags & (O_CREAT|O_DIRECTORY)) == O_CREAT|O_DIRECTORY);
}

so we would have

   if (O_IS_MKDIR(flags))
         return -EINVAL;


>  	if (d_in_lookup(dentry)) {
>  		struct dentry *res = v9fs_vfs_lookup(dir, dentry, 0);
>  		if (res || d_really_is_positive(dentry))
> diff --git a/fs/9p/vfs_inode_dotl.c b/fs/9p/vfs_inode_dotl.c
> index 141fb54db65d..9f4b865d07d7 100644
> --- a/fs/9p/vfs_inode_dotl.c
> +++ b/fs/9p/vfs_inode_dotl.c
> @@ -239,6 +239,9 @@ v9fs_vfs_atomic_open_dotl(struct inode *dir, struct dentry *dentry,
>  	struct v9fs_session_info *v9ses;
>  	struct posix_acl *pacl = NULL, *dacl = NULL;
>  
> +	if ((flags & O_MKDIR_MASK) == O_MKDIR_MASK)
> +		return -EINVAL;
> +
>  	if (d_in_lookup(dentry)) {
>  		struct dentry *res = v9fs_vfs_lookup(dir, dentry, 0);
>  		if (res || d_really_is_positive(dentry))
> diff --git a/fs/ceph/file.c b/fs/ceph/file.c
> index d54d71669176..9707d9ed17b6 100644
> --- a/fs/ceph/file.c
> +++ b/fs/ceph/file.c
> @@ -813,6 +813,9 @@ int ceph_atomic_open(struct inode *dir, struct dentry *dentry,
>  	if (dentry->d_name.len > NAME_MAX)
>  		return -ENAMETOOLONG;
>  
> +	if ((flags & O_MKDIR_MASK) == O_MKDIR_MASK)
> +		return -EINVAL;
> +
>  	err = ceph_wait_on_conflict_unlink(dentry);
>  	if (err)
>  		return err;
> diff --git a/fs/fuse/dir.c b/fs/fuse/dir.c
> index b658b6baf72f..4c59992b9867 100644
> --- a/fs/fuse/dir.c
> +++ b/fs/fuse/dir.c
> @@ -940,6 +940,9 @@ static int fuse_atomic_open(struct inode *dir, struct dentry *entry,
>  	if (fuse_is_bad(dir))
>  		return -EIO;
>  
> +	if ((flags & O_MKDIR_MASK) == O_MKDIR_MASK)
> +		return -EINVAL;
> +
>  	if (d_in_lookup(entry)) {
>  		struct dentry *res = fuse_lookup(dir, entry, 0);
>  		if (res || d_really_is_positive(entry))
> diff --git a/fs/gfs2/inode.c b/fs/gfs2/inode.c
> index e9bf4879c07f..21c6544fbee5 100644
> --- a/fs/gfs2/inode.c
> +++ b/fs/gfs2/inode.c
> @@ -1384,6 +1384,9 @@ static int gfs2_atomic_open(struct inode *dir, struct dentry *dentry,
>  {
>  	bool excl = !!(flags & O_EXCL);
>  
> +	if ((flags & O_MKDIR_MASK) == O_MKDIR_MASK)
> +		return -EINVAL;
> +
>  	if (d_in_lookup(dentry)) {
>  		struct dentry *d = __gfs2_lookup(dir, dentry, file);
>  		if (file->f_mode & FMODE_OPENED) {
> diff --git a/fs/namei.c b/fs/namei.c
> index c7fac83c9a85..9d9529ef30c4 100644
> --- a/fs/namei.c
> +++ b/fs/namei.c
> @@ -2777,9 +2777,14 @@ static const char *path_init(struct nameidata *nd, unsigned flags)
>  	return s;
>  }
>  
> +static inline bool trailing_slashes(struct nameidata *nd)
> +{
> +	return (bool)nd->last.name[nd->last.len];
> +}

Moving and reusing this function could be a patch on its own.  It is a
nice tidy-up which is unrelated to the rest of the change.

> +
>  static inline const char *lookup_last(struct nameidata *nd)
>  {
> -	if (nd->last_type == LAST_NORM && nd->last.name[nd->last.len])
> +	if (nd->last_type == LAST_NORM && trailing_slashes(nd))
>  		nd->flags |= LOOKUP_FOLLOW | LOOKUP_DIRECTORY;
>  
>  	return walk_component(nd, WALK_TRAILING);
> @@ -4166,6 +4171,16 @@ static inline umode_t vfs_prepare_mode(struct mnt_idmap *idmap,
>  	return mode;
>  }
>  
> +static int __vfs_create(struct mnt_idmap *idmap, struct dentry *dentry, umode_t mode,
> +			struct delegated_inode *di, bool excl)
> +{
> +	struct inode *dir = d_inode(dentry->d_parent);
> +	int error = try_break_deleg(dir, di);
> +	if (error)
> +		return error;
> +	return dir->i_op->create(idmap, dir, dentry, mode, excl);
> +}

I don't understand why you have factored this out.  Why not leave the
try_break_deleg() where it was?
This is the sort of thing that really benefits from a few words in the
commit message.

> +
>  /**
>   * vfs_create - create new file
>   * @idmap:	idmap of the mount the inode was found from
> @@ -4192,16 +4207,14 @@ int vfs_create(struct mnt_idmap *idmap, struct dentry *dentry, umode_t mode,
>  		return error;
>  
>  	if (!dir->i_op->create)
> -		return -EACCES;	/* shouldn't it be ENOSYS? */
> +		return -EOPNOTSUPP;
>  
>  	mode = vfs_prepare_mode(idmap, dir, mode, S_IALLUGO, S_IFREG);
>  	error = security_inode_create(dir, dentry, mode);
>  	if (error)
>  		return error;
> -	error = try_break_deleg(dir, di);
> -	if (error)
> -		return error;
> -	error = dir->i_op->create(idmap, dir, dentry, mode, true);
> +
> +	error = __vfs_create(idmap, dentry, mode, di, true);
>  	if (!error)
>  		fsnotify_create(dir, dentry);
>  	return error;
> @@ -4321,21 +4334,32 @@ 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)
> +			umode_t mode, bool create_dir)
                                      ^^^^^^

I don't like bool arguments much.  Particularly when code passes a
literal "true" or "false" as in

	create_error = may_o_create(idmap, path, dentry, mode, false);

as it isn't clear what the "false" is supposed to mean.
Could we arrange to pass S_IFDIR or S_IFREG as appropriate?
Or somehow make the intent of that 5th arg more obvious.

>  {
> -	int error = security_path_mknod(dir, dentry, mode, 0);
> +	struct inode *dir_inode = dir->dentry->d_inode;
> +	int error;
> +
> +	error = create_dir ? security_path_mkdir(dir, dentry, mode)
> +			   : 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);
> +	return create_dir ? security_inode_mkdir(dir_inode, dentry, mode)
> +			  : security_inode_create(dir_inode, dentry, mode);
> +}
> +
> +static inline umode_t o_create_mode(struct mnt_idmap *idmap,
> +		const struct inode *dir, umode_t mode, bool create_dir)
> +{
> +	return create_dir ? vfs_prepare_mode(idmap, dir, mode, S_IRWXUGO | S_ISVTX, 0)
> +			  : vfs_prepare_mode(idmap, dir, mode, S_IALLUGO, S_IFREG);

It isn't clear the purpose of this.
In the create_dir case, shouldn't we pass S_IFDIR as the last arg: type?

The difference between "S_IRWXUGO | S_ISVTX" and "S_IALLUGO" is not
immediately obvious, but the former excludes S_ISUID and S_ISGID.
The setuid bit is meaningless on directories so there is little point in
stripping it, but I don't object as long as the intent is documented
here.
The setgid bit is meaningful but vfs_prepare_mode() seems to already
handle it correctly for directories if you pass the type as S_IFDIR.
If you have a good reason to impose different handling here, that might
be sensible, but it should be documented.

>  }
>  
>  /*
> @@ -4388,6 +4412,9 @@ static struct dentry *atomic_open(const struct path *path, struct dentry *dentry
>  	return dentry;
>  }
>  
> +static struct dentry *__vfs_mkdir(struct mnt_idmap *, struct inode *,
> +			 struct dentry *, umode_t,
> +			 struct delegated_inode *);
>  /*
>   * Look up and maybe create and open the last component.
>   *
> @@ -4412,8 +4439,9 @@ static struct dentry *lookup_open(struct nameidata *nd, struct file *file,
>  	struct inode *dir_inode = dir->d_inode;
>  	int open_flag = op->open_flag;
>  	struct dentry *dentry;
> -	int error, create_error = 0;
> +	int error = 0, create_error = 0;
>  	umode_t mode = op->mode;
> +	bool create_dir = (open_flag & O_MKDIR_MASK) == O_MKDIR_MASK;
>  	DECLARE_WAIT_QUEUE_HEAD_ONSTACK(wq);
>  
>  	if (unlikely(IS_DEADDIR(dir_inode)))
> @@ -4462,10 +4490,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->d_inode, mode, mode, mode);
> +		mode = o_create_mode(idmap, dir_inode, mode, create_dir);
>  		if (likely(got_write))
>  			create_error = may_o_create(idmap, &nd->path,
> -						    dentry, mode);
> +						    dentry, mode, create_dir);
>  		else
>  			create_error = -EROFS;
>  	}
> @@ -4494,29 +4522,37 @@ static struct dentry *lookup_open(struct nameidata *nd, struct file *file,
>  		}
>  	}
>  
> +	if (unlikely(create_error) && !dentry->d_inode) {
> +		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, 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;
> +		if ((create_dir && !dir_inode->i_op->mkdir)
> +		    || (!create_dir && !dir_inode->i_op->create)) {
> +			error = -EOPNOTSUPP;
>  			goto out_dput;
>  		}
>  
> -		error = dir_inode->i_op->create(idmap, dir_inode, dentry,
> -						mode, open_flag & O_EXCL);
> +		if (create_dir) {
> +			struct dentry *res = __vfs_mkdir(idmap, dir_inode, dentry, mode,
> +							 delegated_inode);
> +			if (IS_ERR(res))
> +				error = PTR_ERR(res);
> +			else
> +				dentry = res;
> +		} else {
> +			error = __vfs_create(idmap, dentry, mode, delegated_inode,
> +					     open_flag & O_EXCL);
> +		}
>  		if (error)
>  			goto out_dput;
>  	}
> -	if (unlikely(create_error) && !dentry->d_inode) {
> -		error = create_error;
> -		goto out_dput;
> -	}
> +
>  	return dentry;
>  
>  out_dput:
> @@ -4524,17 +4560,12 @@ static struct dentry *lookup_open(struct nameidata *nd, struct file *file,
>  	return ERR_PTR(error);
>  }
>  
> -static inline bool trailing_slashes(struct nameidata *nd)
> -{
> -	return (bool)nd->last.name[nd->last.len];
> -}
> -
>  static struct dentry *lookup_fast_for_open(struct nameidata *nd, int open_flag)
>  {
>  	struct dentry *dentry;
>  
>  	if (open_flag & O_CREAT) {
> -		if (trailing_slashes(nd))
> +		if (trailing_slashes(nd) && !(open_flag & O_DIRECTORY))
>  			return ERR_PTR(-EISDIR);
>  
>  		/* Don't bother on an O_EXCL create */
> @@ -4605,13 +4636,17 @@ static const char *open_last_lookups(struct nameidata *nd,
>  		 */
>  	}
>  	if (open_flag & O_CREAT)
> -		inode_lock(dir->d_inode);
> +		inode_lock_nested(dir->d_inode, I_MUTEX_PARENT);
>  	else
>  		inode_lock_shared(dir->d_inode);
>  	dentry = lookup_open(nd, file, op, got_write, &delegated_inode);
>  	if (!IS_ERR(dentry)) {
> -		if (file->f_mode & FMODE_CREATED)
> -			fsnotify_create(dir->d_inode, dentry);
> +		if (file->f_mode & FMODE_CREATED) {
> +			if (open_flag & O_DIRECTORY)
> +				fsnotify_mkdir(dir->d_inode, dentry);
> +			else
> +				fsnotify_create(dir->d_inode, dentry);
> +		}
>  		if (file->f_mode & FMODE_OPENED)
>  			fsnotify_open(file);
>  	}
> @@ -4672,12 +4707,16 @@ static int do_open(struct nameidata *nd,
>  	if (open_flag & O_CREAT) {
>  		if ((open_flag & O_EXCL) && !(file->f_mode & FMODE_CREATED))
>  			return -EEXIST;
> -		if (d_is_dir(nd->path.dentry))
> -			return -EISDIR;
> -		error = may_create_in_sticky(idmap, nd,
> -					     d_backing_inode(nd->path.dentry));
> -		if (unlikely(error))
> -			return error;
> +		// there are no special rules for creating dirs in a sticky bit dir

Maybe there *should* be special rules for creating dirs.
I don't know the details of the exploits that these rules protect
against, but if we add O_CREAT|O_DIRECTORY and people start using
it, and they don't do appropriate validation in sticky
directories (and the whole point here is to avoid the need
for validation) then maybe similar sorts of bugs and exploits
could appear.

I would recommend that O_CREAT|O_DIRECTORY must never successfully open
a directory with different ownership in a sticky directory.  There is no
need for a sysctl, because there is no legacy behaviour to protect.

Q: is O_EXCL supported with O_CREAT|O_DIRECTORY? I didn't notice
and special handling, but I could easily have missed it.


> +		if (!(open_flag & O_DIRECTORY)) {
> +			if (d_is_dir(nd->path.dentry))
> +				return -EISDIR;
> +
> +			error = may_create_in_sticky(idmap, nd,
> +						     d_backing_inode(nd->path.dentry));
> +			if (unlikely(error))
> +				return error;
> +		}
>  	}
>  	if ((nd->flags & LOOKUP_DIRECTORY) && !d_can_lookup(nd->path.dentry))
>  		return -ENOTDIR;
> @@ -5039,7 +5078,7 @@ struct file *dentry_create(struct path *path, int flags, umode_t mode,
>  		path->dentry = dir;
>  		mode = vfs_prepare_mode(idmap, dir_inode, mode, S_IALLUGO, S_IFREG);
>  
> -		create_error = may_o_create(idmap, path, dentry, mode);
> +		create_error = may_o_create(idmap, path, dentry, mode, false);
>  		if (create_error)
>  			flags &= ~O_CREAT;
>  
> @@ -5207,6 +5246,37 @@ SYSCALL_DEFINE3(mknod, const char __user *, filename, umode_t, mode, unsigned, d
>  	return filename_mknodat(AT_FDCWD, name, mode, dev);
>  }
>  
> +static struct dentry *__vfs_mkdir(struct mnt_idmap *idmap, struct inode *dir,
> +			 struct dentry *dentry, umode_t mode,
> +			 struct delegated_inode *di)
> +{
> +	int error;
> +	unsigned max_links = dir->i_sb->s_max_links;
> +	struct dentry *de;
> +
> +	error = -EMLINK;
> +	if (max_links && dir->i_nlink >= max_links)
> +		goto err;
> +
> +	error = try_break_deleg(dir, di);
> +	if (error)
> +		goto err;
> +
> +	de = dir->i_op->mkdir(idmap, dir, dentry, mode);
> +	if (IS_ERR(de)) {
> +		error = PTR_ERR(de);
> +		goto err;
> +	}
> +	if (de) {
> +		dput(dentry);
> +		dentry = de;
> +	}
> +	return dentry;
> +
> +err:
> +	return ERR_PTR(error);
> +}
> +
>  /**
>   * vfs_mkdir - create directory returning correct dentry if possible
>   * @idmap:		idmap of the mount the inode was found from
> @@ -5231,17 +5301,16 @@ SYSCALL_DEFINE3(mknod, const char __user *, filename, umode_t, mode, unsigned, d
>   */
>  struct dentry *vfs_mkdir(struct mnt_idmap *idmap, struct inode *dir,
>  			 struct dentry *dentry, umode_t mode,
> -			 struct delegated_inode *delegated_inode)
> +			 struct delegated_inode *di)
>  {
>  	int error;
> -	unsigned max_links = dir->i_sb->s_max_links;
>  	struct dentry *de;
>  
>  	error = may_create_dentry(idmap, dir, dentry);
>  	if (error)
>  		goto err;
>  
> -	error = -EPERM;
> +	error = -EOPNOTSUPP;
>  	if (!dir->i_op->mkdir)
>  		goto err;
>  
> @@ -5250,22 +5319,12 @@ struct dentry *vfs_mkdir(struct mnt_idmap *idmap, struct inode *dir,
>  	if (error)
>  		goto err;
>  
> -	error = -EMLINK;
> -	if (max_links && dir->i_nlink >= max_links)
> -		goto err;
> -
> -	error = try_break_deleg(dir, delegated_inode);
> -	if (error)
> -		goto err;
> -
> -	de = dir->i_op->mkdir(idmap, dir, dentry, mode);
> -	error = PTR_ERR(de);
> -	if (IS_ERR(de))
> +	de = __vfs_mkdir(idmap, dir, dentry, mode, di);
> +	if (IS_ERR(de)) {
> +		error = PTR_ERR(de);
>  		goto err;
> -	if (de) {
> -		dput(dentry);
> -		dentry = de;
>  	}
> +	dentry = de;
>  	fsnotify_mkdir(dir, dentry);
>  	return dentry;
>  
> diff --git a/fs/nfs/dir.c b/fs/nfs/dir.c
> index e9ce1883288c..e44c7598b68e 100644
> --- a/fs/nfs/dir.c
> +++ b/fs/nfs/dir.c
> @@ -2314,6 +2314,9 @@ int nfs_atomic_open_v23(struct inode *dir, struct dentry *dentry,
>  	if (dentry->d_name.len > NFS_SERVER(dir)->namelen)
>  		return -ENAMETOOLONG;
>  
> +	if ((open_flags & O_MKDIR_MASK) == O_MKDIR_MASK)
> +		return -EINVAL;
> +
>  	if (open_flags & O_CREAT) {
>  		error = nfs_do_create(dir, dentry, mode, open_flags);
>  		if (!error) {
> diff --git a/fs/nfs/file.c b/fs/nfs/file.c
> index 25048a3c2364..467f6bc707da 100644
> --- a/fs/nfs/file.c
> +++ b/fs/nfs/file.c
> @@ -52,6 +52,9 @@ int nfs_check_flags(int flags)
>  	if ((flags & (O_APPEND | O_DIRECT)) == (O_APPEND | O_DIRECT))
>  		return -EINVAL;
>  
> +	if ((flags & O_MKDIR_MASK) == O_MKDIR_MASK)
> +		return -EINVAL;
> +
>  	return 0;
>  }
>  EXPORT_SYMBOL_GPL(nfs_check_flags);
> diff --git a/fs/open.c b/fs/open.c
> index 681d405bc61e..865ea6f70e8c 100644
> --- a/fs/open.c
> +++ b/fs/open.c
> @@ -1209,29 +1209,30 @@ inline int build_open_flags(const struct open_how *how, struct open_flags *op)
>  	if (WILL_CREATE(flags)) {
>  		if (how->mode & ~S_IALLUGO)
>  			return -EINVAL;
> -		op->mode = how->mode | S_IFREG;
> +		if ((flags & (O_MKDIR_MASK)) == O_MKDIR_MASK)
> +			op->mode = how->mode | S_IFDIR;
> +		else
> +			op->mode = how->mode | S_IFREG;
>  	} else {
>  		if (how->mode != 0)
>  			return -EINVAL;
>  		op->mode = 0;
>  	}
>  
> -	/*
> -	 * Block bugs where O_DIRECTORY | O_CREAT created regular files.
> -	 * Note, that blocking O_DIRECTORY | O_CREAT here also protects
> -	 * O_TMPFILE below which requires O_DIRECTORY being raised.
> -	 */
> -	if ((flags & (O_DIRECTORY | O_CREAT)) == (O_DIRECTORY | O_CREAT))
> -		return -EINVAL;
> -
>  	/* Now handle the creative implementation of O_TMPFILE. */
>  	if (flags & __O_TMPFILE) {
>  		/*
>  		 * In order to ensure programs get explicit errors when trying
>  		 * to use O_TMPFILE on old kernels we enforce that O_DIRECTORY
> -		 * is raised alongside __O_TMPFILE.
> +		 * is raised alongside __O_TMPFILE, but without O_CREAT. The
> +		 * reason for disallowing O_CREAT|O_TMPFILE is that
> +		 * O_DIRECTORY|O_CREAT used to work and created a regular file
> +		 * if nothing existed at the open path. Hence, allowing the
> +		 * combination would have caused O_CREAT|O_TMPFILE to create a
> +		 * regular (non-temporary) file on old kernels, while the caller
> +		 * would believe they created an actual O_TMPFILE.
>  		 */
> -		if (!(flags & O_DIRECTORY))
> +		if (!(flags & O_DIRECTORY) || (flags & O_CREAT))
>  			return -EINVAL;
>  		if (!(acc_mode & MAY_WRITE))
>  			return -EINVAL;
> @@ -1268,6 +1269,8 @@ inline int build_open_flags(const struct open_how *how, struct open_flags *op)
>  	op->intent = flags & O_PATH ? 0 : LOOKUP_OPEN;
>  
>  	if (flags & O_CREAT) {
> +		if ((flags & O_DIRECTORY) && (acc_mode & MAY_WRITE))
> +			return -EISDIR;

It seems odd that the MAY_WRITE test is only performed for O_CREAT.
Shouldn't any open of O_DIRECTORY|O_WRONLY fail, and shouldn't the one
test catch both create and non-create cases?


>  		op->intent |= LOOKUP_CREATE;
>  		if (flags & O_EXCL) {
>  			op->intent |= LOOKUP_EXCL;
> diff --git a/fs/smb/client/dir.c b/fs/smb/client/dir.c
> index e4295a5b55b3..ec8c54c91261 100644
> --- a/fs/smb/client/dir.c
> +++ b/fs/smb/client/dir.c
> @@ -526,6 +526,9 @@ int cifs_atomic_open(struct inode *dir, struct dentry *direntry,
>  	if (unlikely(cifs_forced_shutdown(cifs_sb)))
>  		return smb_EIO(smb_eio_trace_forced_shutdown);
>  
> +	if ((oflags & O_MKDIR_MASK) == O_MKDIR_MASK)
> +		return -EINVAL;
> +
>  	/*
>  	 * Posix open is only called (at lookup time) for file create now. For
>  	 * opens (rather than creates), because we do not know if it is a file
> diff --git a/fs/vboxsf/dir.c b/fs/vboxsf/dir.c
> index 42bedc4ec7af..aef5ca6730be 100644
> --- a/fs/vboxsf/dir.c
> +++ b/fs/vboxsf/dir.c
> @@ -318,6 +318,9 @@ static int vboxsf_dir_atomic_open(struct inode *parent, struct dentry *dentry,
>  	u64 handle;
>  	int err;
>  
> +	if ((flags & O_MKDIR_MASK) == O_MKDIR_MASK)
> +		return -EINVAL;
> +
>  	if (d_in_lookup(dentry)) {
>  		struct dentry *res = vboxsf_dir_lookup(parent, dentry, 0);
>  		if (res || d_really_is_positive(dentry))
> diff --git a/include/linux/fcntl.h b/include/linux/fcntl.h
> index a332e79b3207..e31f3a57f07c 100644
> --- a/include/linux/fcntl.h
> +++ b/include/linux/fcntl.h
> @@ -12,6 +12,8 @@
>  	 FASYNC	| O_DIRECT | O_LARGEFILE | O_DIRECTORY | O_NOFOLLOW | \
>  	 O_NOATIME | O_CLOEXEC | O_PATH | __O_TMPFILE)
>  
> +#define O_MKDIR_MASK	(O_CREAT | O_DIRECTORY)
> +
>  /* List of all valid flags for the how->resolve argument: */
>  #define VALID_RESOLVE_FLAGS \
>  	(RESOLVE_NO_XDEV | RESOLVE_NO_MAGICLINKS | RESOLVE_NO_SYMLINKS | \
> -- 
> 2.54.0
> 
> 
> 

Thanks,
NeilBrown

Re: [RFC PATCH v5 1/2] vfs: add O_CREAT|O_DIRECTORY to open*(2)

[Christian Brauner]

On Wed, May 27, 2026 at 05:27:54PM +1000, NeilBrown wrote:
> On Tue, 26 May 2026, 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 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
> > 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.
> > 
> > This commit also changes the error returned when a filesystem operation
> > is unsupported (i_op->mkdir/creat) to EOPNOTSUPP. Current error values
> > are inconsistent (both EPERM and EACCES are used) and confusing.
> 
> This commit description is good at justifying the change.  But it is not
> so good at explaining the details of how the change happens so that a
> reviewer can match the code with the explanation.  There is a lot
> happening here and I think I would rather it were split into a few
> separate patches so it is easier to comprehend.
> 
> I *think* the intention is that this new functionality would not be
> available on filesystems which support ->atomic_open, until those
> filesystems are given tailored support.  Is that correct?  Spelling that
> out at the start would be useful.  I would implement that by checking
> for O_DIRECTORY in atomic_open() and leaving the filesystems untouched.
> Then a patch later in the series would move that test into the various
> filesystems.
> 
> I'm not convinced the change to EOPNOTSUPP is a good idea.  It should
> be a separate patch so that it can be reviewed separately.
> 
> EOPNOTSUPP is documented as
>        EOPNOTSUPP      Operation not supported on socket (POSIX.1-2001).
> 
> and we have no sockets here!

Sorry, not even userspace tooling indicates that this has anything to do
with sockets.

    > which errno
    /usr/bin/errno

    > errno ENOTSUP
    ENOTSUP 95 Operation not supported
    
    > errno EOPNOTSUPP
    EOPNOTSUPP 95 Operation not supported

That's the canonical way of communicating that an operation is not
supported and widely used already. ENOTSUP as of today is a thing in
tools/ and nowhere else. Let's leave it there.

We have wide support and usage for EOPNOTSUPP already so let's stick
with that.

> ENOTSUP might be a reasonable choice - though it has the same numeric
> value and it often confused with EOPNOTSUPP in the kernel.
> I don't think EACCES is particularly good, but I don't think anything

No, that's mostly for LSMs and really not a good fit.

> else is enough better to justify a change.
> 
> Other comments interleaved...
> 
> > 
> > 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@xs4all.nl>
> > ---
> >  fs/9p/vfs_inode.c      |   3 +
> >  fs/9p/vfs_inode_dotl.c |   3 +
> >  fs/ceph/file.c         |   3 +
> >  fs/fuse/dir.c          |   3 +
> >  fs/gfs2/inode.c        |   3 +
> >  fs/namei.c             | 177 +++++++++++++++++++++++++++--------------
> >  fs/nfs/dir.c           |   3 +
> >  fs/nfs/file.c          |   3 +
> >  fs/open.c              |  25 +++---
> >  fs/smb/client/dir.c    |   3 +
> >  fs/vboxsf/dir.c        |   3 +
> >  include/linux/fcntl.h  |   2 +
> >  12 files changed, 161 insertions(+), 70 deletions(-)
> > 
> > diff --git a/fs/9p/vfs_inode.c b/fs/9p/vfs_inode.c
> > index f468acb8ee7d..d1925333d327 100644
> > --- a/fs/9p/vfs_inode.c
> > +++ b/fs/9p/vfs_inode.c
> > @@ -771,6 +771,9 @@ v9fs_vfs_atomic_open(struct inode *dir, struct dentry *dentry,
> >  	struct inode *inode;
> >  	int p9_omode;
> >  
> > +	if ((flags & O_MKDIR_MASK) == O_MKDIR_MASK)
> > +		return -EINVAL;
> > +
> 
> O_MKDIR_MASK is only ever used in this construct.  I would prefer
> something like
> 
> static inline bool O_IS_MKDIR(unsigned flags)
> {
>     return (flags & (O_CREAT|O_DIRECTORY)) == O_CREAT|O_DIRECTORY);
> }
> 
> so we would have
> 
>    if (O_IS_MKDIR(flags))
>          return -EINVAL;
> 
> 
> >  	if (d_in_lookup(dentry)) {
> >  		struct dentry *res = v9fs_vfs_lookup(dir, dentry, 0);
> >  		if (res || d_really_is_positive(dentry))
> > diff --git a/fs/9p/vfs_inode_dotl.c b/fs/9p/vfs_inode_dotl.c
> > index 141fb54db65d..9f4b865d07d7 100644
> > --- a/fs/9p/vfs_inode_dotl.c
> > +++ b/fs/9p/vfs_inode_dotl.c
> > @@ -239,6 +239,9 @@ v9fs_vfs_atomic_open_dotl(struct inode *dir, struct dentry *dentry,
> >  	struct v9fs_session_info *v9ses;
> >  	struct posix_acl *pacl = NULL, *dacl = NULL;
> >  
> > +	if ((flags & O_MKDIR_MASK) == O_MKDIR_MASK)
> > +		return -EINVAL;
> > +
> >  	if (d_in_lookup(dentry)) {
> >  		struct dentry *res = v9fs_vfs_lookup(dir, dentry, 0);
> >  		if (res || d_really_is_positive(dentry))
> > diff --git a/fs/ceph/file.c b/fs/ceph/file.c
> > index d54d71669176..9707d9ed17b6 100644
> > --- a/fs/ceph/file.c
> > +++ b/fs/ceph/file.c
> > @@ -813,6 +813,9 @@ int ceph_atomic_open(struct inode *dir, struct dentry *dentry,
> >  	if (dentry->d_name.len > NAME_MAX)
> >  		return -ENAMETOOLONG;
> >  
> > +	if ((flags & O_MKDIR_MASK) == O_MKDIR_MASK)
> > +		return -EINVAL;
> > +
> >  	err = ceph_wait_on_conflict_unlink(dentry);
> >  	if (err)
> >  		return err;
> > diff --git a/fs/fuse/dir.c b/fs/fuse/dir.c
> > index b658b6baf72f..4c59992b9867 100644
> > --- a/fs/fuse/dir.c
> > +++ b/fs/fuse/dir.c
> > @@ -940,6 +940,9 @@ static int fuse_atomic_open(struct inode *dir, struct dentry *entry,
> >  	if (fuse_is_bad(dir))
> >  		return -EIO;
> >  
> > +	if ((flags & O_MKDIR_MASK) == O_MKDIR_MASK)
> > +		return -EINVAL;
> > +
> >  	if (d_in_lookup(entry)) {
> >  		struct dentry *res = fuse_lookup(dir, entry, 0);
> >  		if (res || d_really_is_positive(entry))
> > diff --git a/fs/gfs2/inode.c b/fs/gfs2/inode.c
> > index e9bf4879c07f..21c6544fbee5 100644
> > --- a/fs/gfs2/inode.c
> > +++ b/fs/gfs2/inode.c
> > @@ -1384,6 +1384,9 @@ static int gfs2_atomic_open(struct inode *dir, struct dentry *dentry,
> >  {
> >  	bool excl = !!(flags & O_EXCL);
> >  
> > +	if ((flags & O_MKDIR_MASK) == O_MKDIR_MASK)
> > +		return -EINVAL;
> > +
> >  	if (d_in_lookup(dentry)) {
> >  		struct dentry *d = __gfs2_lookup(dir, dentry, file);
> >  		if (file->f_mode & FMODE_OPENED) {
> > diff --git a/fs/namei.c b/fs/namei.c
> > index c7fac83c9a85..9d9529ef30c4 100644
> > --- a/fs/namei.c
> > +++ b/fs/namei.c
> > @@ -2777,9 +2777,14 @@ static const char *path_init(struct nameidata *nd, unsigned flags)
> >  	return s;
> >  }
> >  
> > +static inline bool trailing_slashes(struct nameidata *nd)
> > +{
> > +	return (bool)nd->last.name[nd->last.len];
> > +}
> 
> Moving and reusing this function could be a patch on its own.  It is a
> nice tidy-up which is unrelated to the rest of the change.
> 
> > +
> >  static inline const char *lookup_last(struct nameidata *nd)
> >  {
> > -	if (nd->last_type == LAST_NORM && nd->last.name[nd->last.len])
> > +	if (nd->last_type == LAST_NORM && trailing_slashes(nd))
> >  		nd->flags |= LOOKUP_FOLLOW | LOOKUP_DIRECTORY;
> >  
> >  	return walk_component(nd, WALK_TRAILING);
> > @@ -4166,6 +4171,16 @@ static inline umode_t vfs_prepare_mode(struct mnt_idmap *idmap,
> >  	return mode;
> >  }
> >  
> > +static int __vfs_create(struct mnt_idmap *idmap, struct dentry *dentry, umode_t mode,
> > +			struct delegated_inode *di, bool excl)
> > +{
> > +	struct inode *dir = d_inode(dentry->d_parent);
> > +	int error = try_break_deleg(dir, di);
> > +	if (error)
> > +		return error;
> > +	return dir->i_op->create(idmap, dir, dentry, mode, excl);
> > +}
> 
> I don't understand why you have factored this out.  Why not leave the
> try_break_deleg() where it was?
> This is the sort of thing that really benefits from a few words in the
> commit message.
> 
> > +
> >  /**
> >   * vfs_create - create new file
> >   * @idmap:	idmap of the mount the inode was found from
> > @@ -4192,16 +4207,14 @@ int vfs_create(struct mnt_idmap *idmap, struct dentry *dentry, umode_t mode,
> >  		return error;
> >  
> >  	if (!dir->i_op->create)
> > -		return -EACCES;	/* shouldn't it be ENOSYS? */
> > +		return -EOPNOTSUPP;
> >  
> >  	mode = vfs_prepare_mode(idmap, dir, mode, S_IALLUGO, S_IFREG);
> >  	error = security_inode_create(dir, dentry, mode);
> >  	if (error)
> >  		return error;
> > -	error = try_break_deleg(dir, di);
> > -	if (error)
> > -		return error;
> > -	error = dir->i_op->create(idmap, dir, dentry, mode, true);
> > +
> > +	error = __vfs_create(idmap, dentry, mode, di, true);
> >  	if (!error)
> >  		fsnotify_create(dir, dentry);
> >  	return error;
> > @@ -4321,21 +4334,32 @@ 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)
> > +			umode_t mode, bool create_dir)
>                                       ^^^^^^
> 
> I don't like bool arguments much.  Particularly when code passes a
> literal "true" or "false" as in
> 
> 	create_error = may_o_create(idmap, path, dentry, mode, false);
> 
> as it isn't clear what the "false" is supposed to mean.
> Could we arrange to pass S_IFDIR or S_IFREG as appropriate?
> Or somehow make the intent of that 5th arg more obvious.

I agree, let's not do booleans if we can avoid them. But fwiw, some
userspace projects require naked booleans to come with a comment like:

may_o_create(..., /* create_dir */ true)

Which makes this easier to handle. But conventions like this are
difficult to enforce - especially in the kernel.

Years ago I added vfs_prepare_mode() which should handle all that
uniformly. Back then I didn't pass S_IFDIR and wrote:

 * Note that it's currently valid for @type to be 0 if a directory is created.
 * Filesystems raise that flag individually and we need to check whether each
 * filesystem can deal with receiving S_IFDIR from the vfs before we enforce a
 * non-zero type.

I kinda sense that I might've been overly cautious here. Some AI tooling
should be able to quickly figure out whether passing S_IFDIR is fine. In
which case this just becomes:

diff --git a/fs/namei.c b/fs/namei.c
index c7fac83c9a85..b4d136f43ee4 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -5245,7 +5245,7 @@ struct dentry *vfs_mkdir(struct mnt_idmap *idmap, struct inode *dir,
        if (!dir->i_op->mkdir)
                goto err;

-       mode = vfs_prepare_mode(idmap, dir, mode, S_IRWXUGO | S_ISVTX, 0);
+       mode = vfs_prepare_mode(idmap, dir, mode, S_IRWXUGO | S_ISVTX, S_IFDIR);
        error = security_inode_mkdir(dir, dentry, mode);
        if (error)
                goto err;

and then match on S_IFMT in may_o_create() etc.

> 
> >  {
> > -	int error = security_path_mknod(dir, dentry, mode, 0);
> > +	struct inode *dir_inode = dir->dentry->d_inode;
> > +	int error;
> > +
> > +	error = create_dir ? security_path_mkdir(dir, dentry, mode)
> > +			   e: 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);
> > +	return create_dir ? security_inode_mkdir(dir_inode, dentry, mode)
> > +			  : security_inode_create(dir_inode, dentry, mode);

Please, no ?: for stuff like this.

> > +}
> > +
> > +static inline umode_t o_create_mode(struct mnt_idmap *idmap,
> > +		const struct inode *dir, umode_t mode, bool create_dir)
> > +{
> > +	return create_dir ? vfs_prepare_mode(idmap, dir, mode, S_IRWXUGO | S_ISVTX, 0)
> > +			  : vfs_prepare_mode(idmap, dir, mode, S_IALLUGO, S_IFREG);
> 
> It isn't clear the purpose of this.

Also, let's please refrain from using ?: I really dislike it. LLMs love
to use it.

> In the create_dir case, shouldn't we pass S_IFDIR as the last arg: type?
> 
> The difference between "S_IRWXUGO | S_ISVTX" and "S_IALLUGO" is not
> immediately obvious, but the former excludes S_ISUID and S_ISGID.
> The setuid bit is meaningless on directories so there is little point in
> stripping it, but I don't object as long as the intent is documented
> here.
> The setgid bit is meaningful but vfs_prepare_mode() seems to already
> handle it correctly for directories if you pass the type as S_IFDIR.
> If you have a good reason to impose different handling here, that might
> be sensible, but it should be documented.
> 
> >  }
> >  
> >  /*
> > @@ -4388,6 +4412,9 @@ static struct dentry *atomic_open(const struct path *path, struct dentry *dentry
> >  	return dentry;
> >  }
> >  
> > +static struct dentry *__vfs_mkdir(struct mnt_idmap *, struct inode *,
> > +			 struct dentry *, umode_t,
> > +			 struct delegated_inode *);
> >  /*
> >   * Look up and maybe create and open the last component.
> >   *
> > @@ -4412,8 +4439,9 @@ static struct dentry *lookup_open(struct nameidata *nd, struct file *file,
> >  	struct inode *dir_inode = dir->d_inode;
> >  	int open_flag = op->open_flag;
> >  	struct dentry *dentry;
> > -	int error, create_error = 0;
> > +	int error = 0, create_error = 0;
> >  	umode_t mode = op->mode;
> > +	bool create_dir = (open_flag & O_MKDIR_MASK) == O_MKDIR_MASK;
> >  	DECLARE_WAIT_QUEUE_HEAD_ONSTACK(wq);
> >  
> >  	if (unlikely(IS_DEADDIR(dir_inode)))
> > @@ -4462,10 +4490,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->d_inode, mode, mode, mode);
> > +		mode = o_create_mode(idmap, dir_inode, mode, create_dir);
> >  		if (likely(got_write))
> >  			create_error = may_o_create(idmap, &nd->path,
> > -						    dentry, mode);
> > +						    dentry, mode, create_dir);
> >  		else
> >  			create_error = -EROFS;
> >  	}
> > @@ -4494,29 +4522,37 @@ static struct dentry *lookup_open(struct nameidata *nd, struct file *file,
> >  		}
> >  	}
> >  
> > +	if (unlikely(create_error) && !dentry->d_inode) {
> > +		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, 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;
> > +		if ((create_dir && !dir_inode->i_op->mkdir)
> > +		    || (!create_dir && !dir_inode->i_op->create)) {
> > +			error = -EOPNOTSUPP;
> >  			goto out_dput;
> >  		}
> >  
> > -		error = dir_inode->i_op->create(idmap, dir_inode, dentry,
> > -						mode, open_flag & O_EXCL);
> > +		if (create_dir) {
> > +			struct dentry *res = __vfs_mkdir(idmap, dir_inode, dentry, mode,
> > +							 delegated_inode);
> > +			if (IS_ERR(res))
> > +				error = PTR_ERR(res);
> > +			else
> > +				dentry = res;
> > +		} else {
> > +			error = __vfs_create(idmap, dentry, mode, delegated_inode,
> > +					     open_flag & O_EXCL);
> > +		}
> >  		if (error)
> >  			goto out_dput;
> >  	}
> > -	if (unlikely(create_error) && !dentry->d_inode) {
> > -		error = create_error;
> > -		goto out_dput;
> > -	}
> > +
> >  	return dentry;
> >  
> >  out_dput:
> > @@ -4524,17 +4560,12 @@ static struct dentry *lookup_open(struct nameidata *nd, struct file *file,
> >  	return ERR_PTR(error);
> >  }
> >  
> > -static inline bool trailing_slashes(struct nameidata *nd)
> > -{
> > -	return (bool)nd->last.name[nd->last.len];
> > -}
> > -
> >  static struct dentry *lookup_fast_for_open(struct nameidata *nd, int open_flag)
> >  {
> >  	struct dentry *dentry;
> >  
> >  	if (open_flag & O_CREAT) {
> > -		if (trailing_slashes(nd))
> > +		if (trailing_slashes(nd) && !(open_flag & O_DIRECTORY))
> >  			return ERR_PTR(-EISDIR);
> >  
> >  		/* Don't bother on an O_EXCL create */
> > @@ -4605,13 +4636,17 @@ static const char *open_last_lookups(struct nameidata *nd,
> >  		 */
> >  	}
> >  	if (open_flag & O_CREAT)
> > -		inode_lock(dir->d_inode);
> > +		inode_lock_nested(dir->d_inode, I_MUTEX_PARENT);
> >  	else
> >  		inode_lock_shared(dir->d_inode);
> >  	dentry = lookup_open(nd, file, op, got_write, &delegated_inode);
> >  	if (!IS_ERR(dentry)) {
> > -		if (file->f_mode & FMODE_CREATED)
> > -			fsnotify_create(dir->d_inode, dentry);
> > +		if (file->f_mode & FMODE_CREATED) {
> > +			if (open_flag & O_DIRECTORY)
> > +				fsnotify_mkdir(dir->d_inode, dentry);
> > +			else
> > +				fsnotify_create(dir->d_inode, dentry);
> > +		}
> >  		if (file->f_mode & FMODE_OPENED)
> >  			fsnotify_open(file);
> >  	}
> > @@ -4672,12 +4707,16 @@ static int do_open(struct nameidata *nd,
> >  	if (open_flag & O_CREAT) {
> >  		if ((open_flag & O_EXCL) && !(file->f_mode & FMODE_CREATED))
> >  			return -EEXIST;
> > -		if (d_is_dir(nd->path.dentry))
> > -			return -EISDIR;
> > -		error = may_create_in_sticky(idmap, nd,
> > -					     d_backing_inode(nd->path.dentry));
> > -		if (unlikely(error))
> > -			return error;
> > +		// there are no special rules for creating dirs in a sticky bit dir
> 
> Maybe there *should* be special rules for creating dirs.
> I don't know the details of the exploits that these rules protect
> against, but if we add O_CREAT|O_DIRECTORY and people start using
> it, and they don't do appropriate validation in sticky
> directories (and the whole point here is to avoid the need
> for validation) then maybe similar sorts of bugs and exploits
> could appear.
> 
> I would recommend that O_CREAT|O_DIRECTORY must never successfully open
> a directory with different ownership in a sticky directory.  There is no
> need for a sysctl, because there is no legacy behaviour to protect.

I think it should still be scoped under the sysctl for consistency. It's
equally annoying if the behavior subtly differs for userspace. So just
something like:

diff --git a/fs/namei.c b/fs/namei.c
index c7fac83c9a85..1f3bca9c7246 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -1442,6 +1442,12 @@ static int may_create_in_sticky(struct mnt_idmap *idmap, struct nameidata *nd,
                                              "sticky_create_regular");
                        return -EACCES;
                }
+
+               if (sysctl_protected_regular >= 2 && S_IFDIR(inode->i_mode)) {
+                       audit_log_path_denied(AUDIT_ANOM_CREAT,
+                                             "sticky_create_dir");
+                       return -EACCES;
+               }
        }

        return 0;

> Q: is O_EXCL supported with O_CREAT|O_DIRECTORY? I didn't notice
> and special handling, but I could easily have missed it.

If possible, it should.

> 
> 
> > +		if (!(open_flag & O_DIRECTORY)) {
> > +			if (d_is_dir(nd->path.dentry))
> > +				return -EISDIR;
> > +
> > +			error = may_create_in_sticky(idmap, nd,
> > +						     d_backing_inode(nd->path.dentry));
> > +			if (unlikely(error))
> > +				return error;
> > +		}
> >  	}
> >  	if ((nd->flags & LOOKUP_DIRECTORY) && !d_can_lookup(nd->path.dentry))
> >  		return -ENOTDIR;
> > @@ -5039,7 +5078,7 @@ struct file *dentry_create(struct path *path, int flags, umode_t mode,
> >  		path->dentry = dir;
> >  		mode = vfs_prepare_mode(idmap, dir_inode, mode, S_IALLUGO, S_IFREG);
> >  
> > -		create_error = may_o_create(idmap, path, dentry, mode);
> > +		create_error = may_o_create(idmap, path, dentry, mode, false);
> >  		if (create_error)
> >  			flags &= ~O_CREAT;
> >  
> > @@ -5207,6 +5246,37 @@ SYSCALL_DEFINE3(mknod, const char __user *, filename, umode_t, mode, unsigned, d
> >  	return filename_mknodat(AT_FDCWD, name, mode, dev);
> >  }
> >  
> > +static struct dentry *__vfs_mkdir(struct mnt_idmap *idmap, struct inode *dir,
> > +			 struct dentry *dentry, umode_t mode,
> > +			 struct delegated_inode *di)
> > +{
> > +	int error;
> > +	unsigned max_links = dir->i_sb->s_max_links;
> > +	struct dentry *de;
> > +
> > +	error = -EMLINK;
> > +	if (max_links && dir->i_nlink >= max_links)
> > +		goto err;
> > +
> > +	error = try_break_deleg(dir, di);
> > +	if (error)
> > +		goto err;
> > +
> > +	de = dir->i_op->mkdir(idmap, dir, dentry, mode);
> > +	if (IS_ERR(de)) {
> > +		error = PTR_ERR(de);
> > +		goto err;
> > +	}
> > +	if (de) {
> > +		dput(dentry);
> > +		dentry = de;
> > +	}
> > +	return dentry;
> > +
> > +err:
> > +	return ERR_PTR(error);
> > +}
> > +
> >  /**
> >   * vfs_mkdir - create directory returning correct dentry if possible
> >   * @idmap:		idmap of the mount the inode was found from
> > @@ -5231,17 +5301,16 @@ SYSCALL_DEFINE3(mknod, const char __user *, filename, umode_t, mode, unsigned, d
> >   */
> >  struct dentry *vfs_mkdir(struct mnt_idmap *idmap, struct inode *dir,
> >  			 struct dentry *dentry, umode_t mode,
> > -			 struct delegated_inode *delegated_inode)
> > +			 struct delegated_inode *di)
> >  {
> >  	int error;
> > -	unsigned max_links = dir->i_sb->s_max_links;
> >  	struct dentry *de;
> >  
> >  	error = may_create_dentry(idmap, dir, dentry);
> >  	if (error)
> >  		goto err;
> >  
> > -	error = -EPERM;
> > +	error = -EOPNOTSUPP;
> >  	if (!dir->i_op->mkdir)
> >  		goto err;
> >  
> > @@ -5250,22 +5319,12 @@ struct dentry *vfs_mkdir(struct mnt_idmap *idmap, struct inode *dir,
> >  	if (error)
> >  		goto err;
> >  
> > -	error = -EMLINK;
> > -	if (max_links && dir->i_nlink >= max_links)
> > -		goto err;
> > -
> > -	error = try_break_deleg(dir, delegated_inode);
> > -	if (error)
> > -		goto err;
> > -
> > -	de = dir->i_op->mkdir(idmap, dir, dentry, mode);
> > -	error = PTR_ERR(de);
> > -	if (IS_ERR(de))
> > +	de = __vfs_mkdir(idmap, dir, dentry, mode, di);
> > +	if (IS_ERR(de)) {
> > +		error = PTR_ERR(de);
> >  		goto err;
> > -	if (de) {
> > -		dput(dentry);
> > -		dentry = de;
> >  	}
> > +	dentry = de;
> >  	fsnotify_mkdir(dir, dentry);
> >  	return dentry;
> >  
> > diff --git a/fs/nfs/dir.c b/fs/nfs/dir.c
> > index e9ce1883288c..e44c7598b68e 100644
> > --- a/fs/nfs/dir.c
> > +++ b/fs/nfs/dir.c
> > @@ -2314,6 +2314,9 @@ int nfs_atomic_open_v23(struct inode *dir, struct dentry *dentry,
> >  	if (dentry->d_name.len > NFS_SERVER(dir)->namelen)
> >  		return -ENAMETOOLONG;
> >  
> > +	if ((open_flags & O_MKDIR_MASK) == O_MKDIR_MASK)
> > +		return -EINVAL;
> > +
> >  	if (open_flags & O_CREAT) {
> >  		error = nfs_do_create(dir, dentry, mode, open_flags);
> >  		if (!error) {
> > diff --git a/fs/nfs/file.c b/fs/nfs/file.c
> > index 25048a3c2364..467f6bc707da 100644
> > --- a/fs/nfs/file.c
> > +++ b/fs/nfs/file.c
> > @@ -52,6 +52,9 @@ int nfs_check_flags(int flags)
> >  	if ((flags & (O_APPEND | O_DIRECT)) == (O_APPEND | O_DIRECT))
> >  		return -EINVAL;
> >  
> > +	if ((flags & O_MKDIR_MASK) == O_MKDIR_MASK)
> > +		return -EINVAL;
> > +
> >  	return 0;
> >  }
> >  EXPORT_SYMBOL_GPL(nfs_check_flags);
> > diff --git a/fs/open.c b/fs/open.c
> > index 681d405bc61e..865ea6f70e8c 100644
> > --- a/fs/open.c
> > +++ b/fs/open.c
> > @@ -1209,29 +1209,30 @@ inline int build_open_flags(const struct open_how *how, struct open_flags *op)
> >  	if (WILL_CREATE(flags)) {
> >  		if (how->mode & ~S_IALLUGO)
> >  			return -EINVAL;
> > -		op->mode = how->mode | S_IFREG;
> > +		if ((flags & (O_MKDIR_MASK)) == O_MKDIR_MASK)
> > +			op->mode = how->mode | S_IFDIR;
> > +		else
> > +			op->mode = how->mode | S_IFREG;
> >  	} else {
> >  		if (how->mode != 0)
> >  			return -EINVAL;
> >  		op->mode = 0;
> >  	}
> >  
> > -	/*
> > -	 * Block bugs where O_DIRECTORY | O_CREAT created regular files.
> > -	 * Note, that blocking O_DIRECTORY | O_CREAT here also protects
> > -	 * O_TMPFILE below which requires O_DIRECTORY being raised.
> > -	 */
> > -	if ((flags & (O_DIRECTORY | O_CREAT)) == (O_DIRECTORY | O_CREAT))
> > -		return -EINVAL;
> > -
> >  	/* Now handle the creative implementation of O_TMPFILE. */
> >  	if (flags & __O_TMPFILE) {
> >  		/*
> >  		 * In order to ensure programs get explicit errors when trying
> >  		 * to use O_TMPFILE on old kernels we enforce that O_DIRECTORY
> > -		 * is raised alongside __O_TMPFILE.
> > +		 * is raised alongside __O_TMPFILE, but without O_CREAT. The
> > +		 * reason for disallowing O_CREAT|O_TMPFILE is that
> > +		 * O_DIRECTORY|O_CREAT used to work and created a regular file
> > +		 * if nothing existed at the open path. Hence, allowing the
> > +		 * combination would have caused O_CREAT|O_TMPFILE to create a
> > +		 * regular (non-temporary) file on old kernels, while the caller
> > +		 * would believe they created an actual O_TMPFILE.
> >  		 */
> > -		if (!(flags & O_DIRECTORY))
> > +		if (!(flags & O_DIRECTORY) || (flags & O_CREAT))
> >  			return -EINVAL;
> >  		if (!(acc_mode & MAY_WRITE))
> >  			return -EINVAL;
> > @@ -1268,6 +1269,8 @@ inline int build_open_flags(const struct open_how *how, struct open_flags *op)
> >  	op->intent = flags & O_PATH ? 0 : LOOKUP_OPEN;
> >  
> >  	if (flags & O_CREAT) {
> > +		if ((flags & O_DIRECTORY) && (acc_mode & MAY_WRITE))
> > +			return -EISDIR;
> 
> It seems odd that the MAY_WRITE test is only performed for O_CREAT.
> Shouldn't any open of O_DIRECTORY|O_WRONLY fail, and shouldn't the one
> test catch both create and non-create cases?
> 
> 
> >  		op->intent |= LOOKUP_CREATE;
> >  		if (flags & O_EXCL) {
> >  			op->intent |= LOOKUP_EXCL;
> > diff --git a/fs/smb/client/dir.c b/fs/smb/client/dir.c
> > index e4295a5b55b3..ec8c54c91261 100644
> > --- a/fs/smb/client/dir.c
> > +++ b/fs/smb/client/dir.c
> > @@ -526,6 +526,9 @@ int cifs_atomic_open(struct inode *dir, struct dentry *direntry,
> >  	if (unlikely(cifs_forced_shutdown(cifs_sb)))
> >  		return smb_EIO(smb_eio_trace_forced_shutdown);
> >  
> > +	if ((oflags & O_MKDIR_MASK) == O_MKDIR_MASK)
> > +		return -EINVAL;
> > +
> >  	/*
> >  	 * Posix open is only called (at lookup time) for file create now. For
> >  	 * opens (rather than creates), because we do not know if it is a file
> > diff --git a/fs/vboxsf/dir.c b/fs/vboxsf/dir.c
> > index 42bedc4ec7af..aef5ca6730be 100644
> > --- a/fs/vboxsf/dir.c
> > +++ b/fs/vboxsf/dir.c
> > @@ -318,6 +318,9 @@ static int vboxsf_dir_atomic_open(struct inode *parent, struct dentry *dentry,
> >  	u64 handle;
> >  	int err;
> >  
> > +	if ((flags & O_MKDIR_MASK) == O_MKDIR_MASK)
> > +		return -EINVAL;
> > +
> >  	if (d_in_lookup(dentry)) {
> >  		struct dentry *res = vboxsf_dir_lookup(parent, dentry, 0);
> >  		if (res || d_really_is_positive(dentry))
> > diff --git a/include/linux/fcntl.h b/include/linux/fcntl.h
> > index a332e79b3207..e31f3a57f07c 100644
> > --- a/include/linux/fcntl.h
> > +++ b/include/linux/fcntl.h
> > @@ -12,6 +12,8 @@
> >  	 FASYNC	| O_DIRECT | O_LARGEFILE | O_DIRECTORY | O_NOFOLLOW | \
> >  	 O_NOATIME | O_CLOEXEC | O_PATH | __O_TMPFILE)
> >  
> > +#define O_MKDIR_MASK	(O_CREAT | O_DIRECTORY)
> > +
> >  /* List of all valid flags for the how->resolve argument: */
> >  #define VALID_RESOLVE_FLAGS \
> >  	(RESOLVE_NO_XDEV | RESOLVE_NO_MAGICLINKS | RESOLVE_NO_SYMLINKS | \
> > -- 
> > 2.54.0
> > 
> > 
> > 
> 
> Thanks,
> NeilBrown

Re: [RFC PATCH v5 1/2] vfs: add O_CREAT|O_DIRECTORY to open*(2)

[NeilBrown]

On Wed, 27 May 2026, Christian Brauner wrote:
> > 
> > I'm not convinced the change to EOPNOTSUPP is a good idea.  It should
> > be a separate patch so that it can be reviewed separately.
> > 
> > EOPNOTSUPP is documented as
> >        EOPNOTSUPP      Operation not supported on socket (POSIX.1-2001).
> > 
> > and we have no sockets here!
> 
> Sorry, not even userspace tooling indicates that this has anything to do
> with sockets.
> 
>     > which errno
>     /usr/bin/errno
> 
>     > errno ENOTSUP
>     ENOTSUP 95 Operation not supported
>     
>     > errno EOPNOTSUPP
>     EOPNOTSUPP 95 Operation not supported

$ man 3 errno | grep EOPNOT
     EOPNOTSUPP      Operation not supported on socket (POSIX.1-2001).
                     (ENOTSUP and EOPNOTSUPP have the same value on Linux,  but


In any case, the error code change should be in a separate patch with a
clear justification and a Cc: to linux-api@vger.kernel.org.

linux-api would like to see the whole patch set I expect.

Thanks,
NeilBrown

Re: [RFC PATCH v5 1/2] vfs: add O_CREAT|O_DIRECTORY to open*(2)

[Christian Brauner]

On Thu, May 28, 2026 at 08:17:31AM +1000, NeilBrown wrote:
> On Wed, 27 May 2026, Christian Brauner wrote:
> > > 
> > > I'm not convinced the change to EOPNOTSUPP is a good idea.  It should
> > > be a separate patch so that it can be reviewed separately.
> > > 
> > > EOPNOTSUPP is documented as
> > >        EOPNOTSUPP      Operation not supported on socket (POSIX.1-2001).
> > > 
> > > and we have no sockets here!
> > 
> > Sorry, not even userspace tooling indicates that this has anything to do
> > with sockets.
> > 
> >     > which errno
> >     /usr/bin/errno
> > 
> >     > errno ENOTSUP
> >     ENOTSUP 95 Operation not supported
> >     
> >     > errno EOPNOTSUPP
> >     EOPNOTSUPP 95 Operation not supported
> 
> $ man 3 errno | grep EOPNOT
>      EOPNOTSUPP      Operation not supported on socket (POSIX.1-2001).
>                      (ENOTSUP and EOPNOTSUPP have the same value on Linux,  but

(ENOTSUP and EOPNOTSUPP have the same value on Linux, but according to POSIX.1 these error values should be distinct.)

Consistency be damned, I guess.

And if we start caring about POSIX again we should probably remove 90%
of our newer apis. It's irrelevant what the standard says. It matters
what we do in practice. Taking a tour through the kernel and trying to
prove that there's any 1:1 relationship between socket operations and
EOPNOTSUPP fails. Equally, going through large userspace projects to see
whether they treat EOPNOTSUPP as even vaguely socket related also fails.

Even glibc treats EOPNOTSUPP as the universal "not supported" sentinel
and raises it even independent of what the kernel returns. One example I
distincly remember is the fchmodat() AT_SYMLINK_NOFOLLOW userspace
emulation it had to do.

Maybe I really misunderstand the motivation for this discussion. But
this feels a bit like wasting time on something that we've already
established a while ago.

Re: [RFC PATCH v5 1/2] vfs: add O_CREAT|O_DIRECTORY to open*(2)

[NeilBrown]

On Thu, 28 May 2026, Christian Brauner wrote:
> On Thu, May 28, 2026 at 08:17:31AM +1000, NeilBrown wrote:
> > On Wed, 27 May 2026, Christian Brauner wrote:
> > > > 
> > > > I'm not convinced the change to EOPNOTSUPP is a good idea.  It should
> > > > be a separate patch so that it can be reviewed separately.
> > > > 
> > > > EOPNOTSUPP is documented as
> > > >        EOPNOTSUPP      Operation not supported on socket (POSIX.1-2001).
> > > > 
> > > > and we have no sockets here!
> > > 
> > > Sorry, not even userspace tooling indicates that this has anything to do
> > > with sockets.
> > > 
> > >     > which errno
> > >     /usr/bin/errno
> > > 
> > >     > errno ENOTSUP
> > >     ENOTSUP 95 Operation not supported
> > >     
> > >     > errno EOPNOTSUPP
> > >     EOPNOTSUPP 95 Operation not supported
> > 
> > $ man 3 errno | grep EOPNOT
> >      EOPNOTSUPP      Operation not supported on socket (POSIX.1-2001).
> >                      (ENOTSUP and EOPNOTSUPP have the same value on Linux,  but
> 
> (ENOTSUP and EOPNOTSUPP have the same value on Linux, but according to POSIX.1 these error values should be distinct.)
> 
> Consistency be damned, I guess.
> 
> And if we start caring about POSIX again we should probably remove 90%
> of our newer apis. It's irrelevant what the standard says. It matters
> what we do in practice. Taking a tour through the kernel and trying to
> prove that there's any 1:1 relationship between socket operations and
> EOPNOTSUPP fails. Equally, going through large userspace projects to see
> whether they treat EOPNOTSUPP as even vaguely socket related also fails.
> 
> Even glibc treats EOPNOTSUPP as the universal "not supported" sentinel
> and raises it even independent of what the kernel returns. One example I
> distincly remember is the fchmodat() AT_SYMLINK_NOFOLLOW userspace
> emulation it had to do.
> 
> Maybe I really misunderstand the motivation for this discussion. But
> this feels a bit like wasting time on something that we've already
> established a while ago.
> 

Maybe someone should update the man-page?

My main point was not that EOPNOTSUPP was a poor choice, but that an API
change needs to be managed properly (and I'm not sure it is worth it in
this case).

Thanks,
NeilBrown

Re: [RFC PATCH v5 1/2] vfs: add O_CREAT|O_DIRECTORY to open*(2)

[Christian Brauner]

On Fri, May 29, 2026 at 09:58:29AM +1000, NeilBrown wrote:
> On Thu, 28 May 2026, Christian Brauner wrote:
> > On Thu, May 28, 2026 at 08:17:31AM +1000, NeilBrown wrote:
> > > On Wed, 27 May 2026, Christian Brauner wrote:
> > > > > 
> > > > > I'm not convinced the change to EOPNOTSUPP is a good idea.  It should
> > > > > be a separate patch so that it can be reviewed separately.
> > > > > 
> > > > > EOPNOTSUPP is documented as
> > > > >        EOPNOTSUPP      Operation not supported on socket (POSIX.1-2001).
> > > > > 
> > > > > and we have no sockets here!
> > > > 
> > > > Sorry, not even userspace tooling indicates that this has anything to do
> > > > with sockets.
> > > > 
> > > >     > which errno
> > > >     /usr/bin/errno
> > > > 
> > > >     > errno ENOTSUP
> > > >     ENOTSUP 95 Operation not supported
> > > >     
> > > >     > errno EOPNOTSUPP
> > > >     EOPNOTSUPP 95 Operation not supported
> > > 
> > > $ man 3 errno | grep EOPNOT
> > >      EOPNOTSUPP      Operation not supported on socket (POSIX.1-2001).
> > >                      (ENOTSUP and EOPNOTSUPP have the same value on Linux,  but
> > 
> > (ENOTSUP and EOPNOTSUPP have the same value on Linux, but according to POSIX.1 these error values should be distinct.)
> > 
> > Consistency be damned, I guess.
> > 
> > And if we start caring about POSIX again we should probably remove 90%
> > of our newer apis. It's irrelevant what the standard says. It matters
> > what we do in practice. Taking a tour through the kernel and trying to
> > prove that there's any 1:1 relationship between socket operations and
> > EOPNOTSUPP fails. Equally, going through large userspace projects to see
> > whether they treat EOPNOTSUPP as even vaguely socket related also fails.
> > 
> > Even glibc treats EOPNOTSUPP as the universal "not supported" sentinel
> > and raises it even independent of what the kernel returns. One example I
> > distincly remember is the fchmodat() AT_SYMLINK_NOFOLLOW userspace
> > emulation it had to do.
> > 
> > Maybe I really misunderstand the motivation for this discussion. But
> > this feels a bit like wasting time on something that we've already
> > established a while ago.
> > 
> 
> Maybe someone should update the man-page?

Yeah, someone did yesterday actually and it was merged the same day.

> My main point was not that EOPNOTSUPP was a poor choice, but that an API
> change needs to be managed properly (and I'm not sure it is worth it in
> this case).

Right. That's certainly a valid concern!

Re: [RFC PATCH v5 1/2] vfs: add O_CREAT|O_DIRECTORY to open*(2)

[NeilBrown]

On Fri, 29 May 2026, Christian Brauner wrote:
> On Fri, May 29, 2026 at 09:58:29AM +1000, NeilBrown wrote:
> > On Thu, 28 May 2026, Christian Brauner wrote:
> > > On Thu, May 28, 2026 at 08:17:31AM +1000, NeilBrown wrote:
> > > > On Wed, 27 May 2026, Christian Brauner wrote:
> > > > > > 
> > > > > > I'm not convinced the change to EOPNOTSUPP is a good idea.  It should
> > > > > > be a separate patch so that it can be reviewed separately.
> > > > > > 
> > > > > > EOPNOTSUPP is documented as
> > > > > >        EOPNOTSUPP      Operation not supported on socket (POSIX.1-2001).
> > > > > > 
> > > > > > and we have no sockets here!
> > > > > 
> > > > > Sorry, not even userspace tooling indicates that this has anything to do
> > > > > with sockets.
> > > > > 
> > > > >     > which errno
> > > > >     /usr/bin/errno
> > > > > 
> > > > >     > errno ENOTSUP
> > > > >     ENOTSUP 95 Operation not supported
> > > > >     
> > > > >     > errno EOPNOTSUPP
> > > > >     EOPNOTSUPP 95 Operation not supported
> > > > 
> > > > $ man 3 errno | grep EOPNOT
> > > >      EOPNOTSUPP      Operation not supported on socket (POSIX.1-2001).
> > > >                      (ENOTSUP and EOPNOTSUPP have the same value on Linux,  but
> > > 
> > > (ENOTSUP and EOPNOTSUPP have the same value on Linux, but according to POSIX.1 these error values should be distinct.)
> > > 
> > > Consistency be damned, I guess.
> > > 
> > > And if we start caring about POSIX again we should probably remove 90%
> > > of our newer apis. It's irrelevant what the standard says. It matters
> > > what we do in practice. Taking a tour through the kernel and trying to
> > > prove that there's any 1:1 relationship between socket operations and
> > > EOPNOTSUPP fails. Equally, going through large userspace projects to see
> > > whether they treat EOPNOTSUPP as even vaguely socket related also fails.
> > > 
> > > Even glibc treats EOPNOTSUPP as the universal "not supported" sentinel
> > > and raises it even independent of what the kernel returns. One example I
> > > distincly remember is the fchmodat() AT_SYMLINK_NOFOLLOW userspace
> > > emulation it had to do.
> > > 
> > > Maybe I really misunderstand the motivation for this discussion. But
> > > this feels a bit like wasting time on something that we've already
> > > established a while ago.
> > > 
> > 
> > Maybe someone should update the man-page?
> 
> Yeah, someone did yesterday actually and it was merged the same day.

That's very efficient!!   Thanks Askar!

NeilBrown

> 
> > My main point was not that EOPNOTSUPP was a poor choice, but that an API
> > change needs to be managed properly (and I'm not sure it is worth it in
> > this case).
> 
> Right. That's certainly a valid concern!
> 

Re: [RFC PATCH v5 1/2] vfs: add O_CREAT|O_DIRECTORY to open*(2)

[Jori Koolstra]

> Op 27-05-2026 13:43 CEST schreef Christian Brauner <brauner@kernel.org>:
> 
> I agree, let's not do booleans if we can avoid them. But fwiw, some
> userspace projects require naked booleans to come with a comment like:
> 
> may_o_create(..., /* create_dir */ true)
> 
> Which makes this easier to handle. But conventions like this are
> difficult to enforce - especially in the kernel.
> 
> Years ago I added vfs_prepare_mode() which should handle all that
> uniformly. Back then I didn't pass S_IFDIR and wrote:
> 
>  * Note that it's currently valid for @type to be 0 if a directory is created.
>  * Filesystems raise that flag individually and we need to check whether each
>  * filesystem can deal with receiving S_IFDIR from the vfs before we enforce a
>  * non-zero type.

Yeah, I think this confused me. Now that I look at it again we can just get
rid of that bool flag in may_o_create() since we just prepared the mode with
vfs_prepare_mode(). It would be nice to also remove it from o_create_mode(),
but off the top of my head, nothing is stopping you from passing a mode type
to open(2) that has more than one bit set (e.g. both S_IFDIR and S_IFREG), and
this is precisely what vfs_prepare_mode() is for to fix.

I was planning to get rid off this comment and enforce S_IFDIR, but I think
that should be separate from this patchset. Agree?

> 
> I kinda sense that I might've been overly cautious here. Some AI tooling
> should be able to quickly figure out whether passing S_IFDIR is fine. In
> which case this just becomes:
> 
> diff --git a/fs/namei.c b/fs/namei.c
> index c7fac83c9a85..b4d136f43ee4 100644
> --- a/fs/namei.c
> +++ b/fs/namei.c
> @@ -5245,7 +5245,7 @@ struct dentry *vfs_mkdir(struct mnt_idmap *idmap, struct inode *dir,
>         if (!dir->i_op->mkdir)
>                 goto err;
> 
> -       mode = vfs_prepare_mode(idmap, dir, mode, S_IRWXUGO | S_ISVTX, 0);
> +       mode = vfs_prepare_mode(idmap, dir, mode, S_IRWXUGO | S_ISVTX, S_IFDIR);
>         error = security_inode_mkdir(dir, dentry, mode);
>         if (error)
>                 goto err;
> 
> and then match on S_IFMT in may_o_create() etc.
> 
> > 
> > >  {
> > > -	int error = security_path_mknod(dir, dentry, mode, 0);
> > > +	struct inode *dir_inode = dir->dentry->d_inode;
> > > +	int error;
> > > +
> > > +	error = create_dir ? security_path_mkdir(dir, dentry, mode)
> > > +			   e: 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);
> > > +	return create_dir ? security_inode_mkdir(dir_inode, dentry, mode)
> > > +			  : security_inode_create(dir_inode, dentry, mode);
> 
> Please, no ?: for stuff like this.
> 
> > > +}
> > > +
> > > +static inline umode_t o_create_mode(struct mnt_idmap *idmap,
> > > +		const struct inode *dir, umode_t mode, bool create_dir)
> > > +{
> > > +	return create_dir ? vfs_prepare_mode(idmap, dir, mode, S_IRWXUGO | S_ISVTX, 0)
> > > +			  : vfs_prepare_mode(idmap, dir, mode, S_IALLUGO, S_IFREG);
> > 
> > It isn't clear the purpose of this.
> 
> Also, let's please refrain from using ?: I really dislike it. LLMs love
> to use it.
> 
> > In the create_dir case, shouldn't we pass S_IFDIR as the last arg: type?
> > 
> > The difference between "S_IRWXUGO | S_ISVTX" and "S_IALLUGO" is not
> > immediately obvious, but the former excludes S_ISUID and S_ISGID.
> > The setuid bit is meaningless on directories so there is little point in
> > stripping it, but I don't object as long as the intent is documented
> > here.
> > The setgid bit is meaningful but vfs_prepare_mode() seems to already
> > handle it correctly for directories if you pass the type as S_IFDIR.
> > If you have a good reason to impose different handling here, that might
> > be sensible, but it should be documented.
> > 
> > >  }
> > >  
> > >  /*
> > > @@ -4388,6 +4412,9 @@ static struct dentry *atomic_open(const struct path *path, struct dentry *dentry
> > >  	return dentry;
> > >  }
> > >  
> > > +static struct dentry *__vfs_mkdir(struct mnt_idmap *, struct inode *,
> > > +			 struct dentry *, umode_t,
> > > +			 struct delegated_inode *);
> > >  /*
> > >   * Look up and maybe create and open the last component.
> > >   *
> > > @@ -4412,8 +4439,9 @@ static struct dentry *lookup_open(struct nameidata *nd, struct file *file,
> > >  	struct inode *dir_inode = dir->d_inode;
> > >  	int open_flag = op->open_flag;
> > >  	struct dentry *dentry;
> > > -	int error, create_error = 0;
> > > +	int error = 0, create_error = 0;
> > >  	umode_t mode = op->mode;
> > > +	bool create_dir = (open_flag & O_MKDIR_MASK) == O_MKDIR_MASK;
> > >  	DECLARE_WAIT_QUEUE_HEAD_ONSTACK(wq);
> > >  
> > >  	if (unlikely(IS_DEADDIR(dir_inode)))
> > > @@ -4462,10 +4490,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->d_inode, mode, mode, mode);
> > > +		mode = o_create_mode(idmap, dir_inode, mode, create_dir);
> > >  		if (likely(got_write))
> > >  			create_error = may_o_create(idmap, &nd->path,
> > > -						    dentry, mode);
> > > +						    dentry, mode, create_dir);
> > >  		else
> > >  			create_error = -EROFS;
> > >  	}
> > > @@ -4494,29 +4522,37 @@ static struct dentry *lookup_open(struct nameidata *nd, struct file *file,
> > >  		}
> > >  	}
> > >  
> > > +	if (unlikely(create_error) && !dentry->d_inode) {
> > > +		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, 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;
> > > +		if ((create_dir && !dir_inode->i_op->mkdir)
> > > +		    || (!create_dir && !dir_inode->i_op->create)) {
> > > +			error = -EOPNOTSUPP;
> > >  			goto out_dput;
> > >  		}
> > >  
> > > -		error = dir_inode->i_op->create(idmap, dir_inode, dentry,
> > > -						mode, open_flag & O_EXCL);
> > > +		if (create_dir) {
> > > +			struct dentry *res = __vfs_mkdir(idmap, dir_inode, dentry, mode,
> > > +							 delegated_inode);
> > > +			if (IS_ERR(res))
> > > +				error = PTR_ERR(res);
> > > +			else
> > > +				dentry = res;
> > > +		} else {
> > > +			error = __vfs_create(idmap, dentry, mode, delegated_inode,
> > > +					     open_flag & O_EXCL);
> > > +		}
> > >  		if (error)
> > >  			goto out_dput;
> > >  	}
> > > -	if (unlikely(create_error) && !dentry->d_inode) {
> > > -		error = create_error;
> > > -		goto out_dput;
> > > -	}
> > > +
> > >  	return dentry;
> > >  
> > >  out_dput:
> > > @@ -4524,17 +4560,12 @@ static struct dentry *lookup_open(struct nameidata *nd, struct file *file,
> > >  	return ERR_PTR(error);
> > >  }
> > >  
> > > -static inline bool trailing_slashes(struct nameidata *nd)
> > > -{
> > > -	return (bool)nd->last.name[nd->last.len];
> > > -}
> > > -
> > >  static struct dentry *lookup_fast_for_open(struct nameidata *nd, int open_flag)
> > >  {
> > >  	struct dentry *dentry;
> > >  
> > >  	if (open_flag & O_CREAT) {
> > > -		if (trailing_slashes(nd))
> > > +		if (trailing_slashes(nd) && !(open_flag & O_DIRECTORY))
> > >  			return ERR_PTR(-EISDIR);
> > >  
> > >  		/* Don't bother on an O_EXCL create */
> > > @@ -4605,13 +4636,17 @@ static const char *open_last_lookups(struct nameidata *nd,
> > >  		 */
> > >  	}
> > >  	if (open_flag & O_CREAT)
> > > -		inode_lock(dir->d_inode);
> > > +		inode_lock_nested(dir->d_inode, I_MUTEX_PARENT);
> > >  	else
> > >  		inode_lock_shared(dir->d_inode);
> > >  	dentry = lookup_open(nd, file, op, got_write, &delegated_inode);
> > >  	if (!IS_ERR(dentry)) {
> > > -		if (file->f_mode & FMODE_CREATED)
> > > -			fsnotify_create(dir->d_inode, dentry);
> > > +		if (file->f_mode & FMODE_CREATED) {
> > > +			if (open_flag & O_DIRECTORY)
> > > +				fsnotify_mkdir(dir->d_inode, dentry);
> > > +			else
> > > +				fsnotify_create(dir->d_inode, dentry);
> > > +		}
> > >  		if (file->f_mode & FMODE_OPENED)
> > >  			fsnotify_open(file);
> > >  	}
> > > @@ -4672,12 +4707,16 @@ static int do_open(struct nameidata *nd,
> > >  	if (open_flag & O_CREAT) {
> > >  		if ((open_flag & O_EXCL) && !(file->f_mode & FMODE_CREATED))
> > >  			return -EEXIST;
> > > -		if (d_is_dir(nd->path.dentry))
> > > -			return -EISDIR;
> > > -		error = may_create_in_sticky(idmap, nd,
> > > -					     d_backing_inode(nd->path.dentry));
> > > -		if (unlikely(error))
> > > -			return error;
> > > +		// there are no special rules for creating dirs in a sticky bit dir
> > 
> > Maybe there *should* be special rules for creating dirs.
> > I don't know the details of the exploits that these rules protect
> > against, but if we add O_CREAT|O_DIRECTORY and people start using
> > it, and they don't do appropriate validation in sticky
> > directories (and the whole point here is to avoid the need
> > for validation) then maybe similar sorts of bugs and exploits
> > could appear.
> > 
> > I would recommend that O_CREAT|O_DIRECTORY must never successfully open
> > a directory with different ownership in a sticky directory.  There is no
> > need for a sysctl, because there is no legacy behaviour to protect.
> 
> I think it should still be scoped under the sysctl for consistency. It's
> equally annoying if the behavior subtly differs for userspace. So just
> something like:
> 
> diff --git a/fs/namei.c b/fs/namei.c
> index c7fac83c9a85..1f3bca9c7246 100644
> --- a/fs/namei.c
> +++ b/fs/namei.c
> @@ -1442,6 +1442,12 @@ static int may_create_in_sticky(struct mnt_idmap *idmap, struct nameidata *nd,
>                                               "sticky_create_regular");
>                         return -EACCES;
>                 }
> +
> +               if (sysctl_protected_regular >= 2 && S_IFDIR(inode->i_mode)) {
> +                       audit_log_path_denied(AUDIT_ANOM_CREAT,
> +                                             "sticky_create_dir");
> +                       return -EACCES;
> +               }
>         }
> 
>         return 0;
> 
> > Q: is O_EXCL supported with O_CREAT|O_DIRECTORY? I didn't notice
> > and special handling, but I could easily have missed it.
> 
> If possible, it should.
> 

This works out of the box, the enforcement is implemented in do_open() as

	if (open_flag & O_CREAT) {
		if ((open_flag & O_EXCL) && !(file->f_mode & FMODE_CREATED))
			return -EEXIST;
        ...
        }

and nothing special has to be done to make this work for directories.

> > 
> > 
> > > +		if (!(open_flag & O_DIRECTORY)) {
> > > +			if (d_is_dir(nd->path.dentry))
> > > +				return -EISDIR;
> > > +
> > > +			error = may_create_in_sticky(idmap, nd,
> > > +						     d_backing_inode(nd->path.dentry));
> > > +			if (unlikely(error))
> > > +				return error;
> > > +		}
> > >  	}
> > >  	if ((nd->flags & LOOKUP_DIRECTORY) && !d_can_lookup(nd->path.dentry))
> > >  		return -ENOTDIR;
> > > @@ -5039,7 +5078,7 @@ struct file *dentry_create(struct path *path, int flags, umode_t mode,
> > >  		path->dentry = dir;
> > >  		mode = vfs_prepare_mode(idmap, dir_inode, mode, S_IALLUGO, S_IFREG);
> > >  
> > > -		create_error = may_o_create(idmap, path, dentry, mode);
> > > +		create_error = may_o_create(idmap, path, dentry, mode, false);
> > >  		if (create_error)
> > >  			flags &= ~O_CREAT;
> > >  
> > > @@ -5207,6 +5246,37 @@ SYSCALL_DEFINE3(mknod, const char __user *, filename, umode_t, mode, unsigned, d
> > >  	return filename_mknodat(AT_FDCWD, name, mode, dev);
> > >  }
> > >  
> > > +static struct dentry *__vfs_mkdir(struct mnt_idmap *idmap, struct inode *dir,
> > > +			 struct dentry *dentry, umode_t mode,
> > > +			 struct delegated_inode *di)
> > > +{
> > > +	int error;
> > > +	unsigned max_links = dir->i_sb->s_max_links;
> > > +	struct dentry *de;
> > > +
> > > +	error = -EMLINK;
> > > +	if (max_links && dir->i_nlink >= max_links)
> > > +		goto err;
> > > +
> > > +	error = try_break_deleg(dir, di);
> > > +	if (error)
> > > +		goto err;
> > > +
> > > +	de = dir->i_op->mkdir(idmap, dir, dentry, mode);
> > > +	if (IS_ERR(de)) {
> > > +		error = PTR_ERR(de);
> > > +		goto err;
> > > +	}
> > > +	if (de) {
> > > +		dput(dentry);
> > > +		dentry = de;
> > > +	}
> > > +	return dentry;
> > > +
> > > +err:
> > > +	return ERR_PTR(error);
> > > +}
> > > +
> > >  /**
> > >   * vfs_mkdir - create directory returning correct dentry if possible
> > >   * @idmap:		idmap of the mount the inode was found from
> > > @@ -5231,17 +5301,16 @@ SYSCALL_DEFINE3(mknod, const char __user *, filename, umode_t, mode, unsigned, d
> > >   */
> > >  struct dentry *vfs_mkdir(struct mnt_idmap *idmap, struct inode *dir,
> > >  			 struct dentry *dentry, umode_t mode,
> > > -			 struct delegated_inode *delegated_inode)
> > > +			 struct delegated_inode *di)
> > >  {
> > >  	int error;
> > > -	unsigned max_links = dir->i_sb->s_max_links;
> > >  	struct dentry *de;
> > >  
> > >  	error = may_create_dentry(idmap, dir, dentry);
> > >  	if (error)
> > >  		goto err;
> > >  
> > > -	error = -EPERM;
> > > +	error = -EOPNOTSUPP;
> > >  	if (!dir->i_op->mkdir)
> > >  		goto err;
> > >  
> > > @@ -5250,22 +5319,12 @@ struct dentry *vfs_mkdir(struct mnt_idmap *idmap, struct inode *dir,
> > >  	if (error)
> > >  		goto err;
> > >  
> > > -	error = -EMLINK;
> > > -	if (max_links && dir->i_nlink >= max_links)
> > > -		goto err;
> > > -
> > > -	error = try_break_deleg(dir, delegated_inode);
> > > -	if (error)
> > > -		goto err;
> > > -
> > > -	de = dir->i_op->mkdir(idmap, dir, dentry, mode);
> > > -	error = PTR_ERR(de);
> > > -	if (IS_ERR(de))
> > > +	de = __vfs_mkdir(idmap, dir, dentry, mode, di);
> > > +	if (IS_ERR(de)) {
> > > +		error = PTR_ERR(de);
> > >  		goto err;
> > > -	if (de) {
> > > -		dput(dentry);
> > > -		dentry = de;
> > >  	}
> > > +	dentry = de;
> > >  	fsnotify_mkdir(dir, dentry);
> > >  	return dentry;
> > >  
> > > diff --git a/fs/nfs/dir.c b/fs/nfs/dir.c
> > > index e9ce1883288c..e44c7598b68e 100644
> > > --- a/fs/nfs/dir.c
> > > +++ b/fs/nfs/dir.c
> > > @@ -2314,6 +2314,9 @@ int nfs_atomic_open_v23(struct inode *dir, struct dentry *dentry,
> > >  	if (dentry->d_name.len > NFS_SERVER(dir)->namelen)
> > >  		return -ENAMETOOLONG;
> > >  
> > > +	if ((open_flags & O_MKDIR_MASK) == O_MKDIR_MASK)
> > > +		return -EINVAL;
> > > +
> > >  	if (open_flags & O_CREAT) {
> > >  		error = nfs_do_create(dir, dentry, mode, open_flags);
> > >  		if (!error) {
> > > diff --git a/fs/nfs/file.c b/fs/nfs/file.c
> > > index 25048a3c2364..467f6bc707da 100644
> > > --- a/fs/nfs/file.c
> > > +++ b/fs/nfs/file.c
> > > @@ -52,6 +52,9 @@ int nfs_check_flags(int flags)
> > >  	if ((flags & (O_APPEND | O_DIRECT)) == (O_APPEND | O_DIRECT))
> > >  		return -EINVAL;
> > >  
> > > +	if ((flags & O_MKDIR_MASK) == O_MKDIR_MASK)
> > > +		return -EINVAL;
> > > +
> > >  	return 0;
> > >  }
> > >  EXPORT_SYMBOL_GPL(nfs_check_flags);
> > > diff --git a/fs/open.c b/fs/open.c
> > > index 681d405bc61e..865ea6f70e8c 100644
> > > --- a/fs/open.c
> > > +++ b/fs/open.c
> > > @@ -1209,29 +1209,30 @@ inline int build_open_flags(const struct open_how *how, struct open_flags *op)
> > >  	if (WILL_CREATE(flags)) {
> > >  		if (how->mode & ~S_IALLUGO)
> > >  			return -EINVAL;
> > > -		op->mode = how->mode | S_IFREG;
> > > +		if ((flags & (O_MKDIR_MASK)) == O_MKDIR_MASK)
> > > +			op->mode = how->mode | S_IFDIR;
> > > +		else
> > > +			op->mode = how->mode | S_IFREG;
> > >  	} else {
> > >  		if (how->mode != 0)
> > >  			return -EINVAL;
> > >  		op->mode = 0;
> > >  	}
> > >  
> > > -	/*
> > > -	 * Block bugs where O_DIRECTORY | O_CREAT created regular files.
> > > -	 * Note, that blocking O_DIRECTORY | O_CREAT here also protects
> > > -	 * O_TMPFILE below which requires O_DIRECTORY being raised.
> > > -	 */
> > > -	if ((flags & (O_DIRECTORY | O_CREAT)) == (O_DIRECTORY | O_CREAT))
> > > -		return -EINVAL;
> > > -
> > >  	/* Now handle the creative implementation of O_TMPFILE. */
> > >  	if (flags & __O_TMPFILE) {
> > >  		/*
> > >  		 * In order to ensure programs get explicit errors when trying
> > >  		 * to use O_TMPFILE on old kernels we enforce that O_DIRECTORY
> > > -		 * is raised alongside __O_TMPFILE.
> > > +		 * is raised alongside __O_TMPFILE, but without O_CREAT. The
> > > +		 * reason for disallowing O_CREAT|O_TMPFILE is that
> > > +		 * O_DIRECTORY|O_CREAT used to work and created a regular file
> > > +		 * if nothing existed at the open path. Hence, allowing the
> > > +		 * combination would have caused O_CREAT|O_TMPFILE to create a
> > > +		 * regular (non-temporary) file on old kernels, while the caller
> > > +		 * would believe they created an actual O_TMPFILE.
> > >  		 */
> > > -		if (!(flags & O_DIRECTORY))
> > > +		if (!(flags & O_DIRECTORY) || (flags & O_CREAT))
> > >  			return -EINVAL;
> > >  		if (!(acc_mode & MAY_WRITE))
> > >  			return -EINVAL;
> > > @@ -1268,6 +1269,8 @@ inline int build_open_flags(const struct open_how *how, struct open_flags *op)
> > >  	op->intent = flags & O_PATH ? 0 : LOOKUP_OPEN;
> > >  
> > >  	if (flags & O_CREAT) {
> > > +		if ((flags & O_DIRECTORY) && (acc_mode & MAY_WRITE))
> > > +			return -EISDIR;
> > 
> > It seems odd that the MAY_WRITE test is only performed for O_CREAT.
> > Shouldn't any open of O_DIRECTORY|O_WRONLY fail, and shouldn't the one
> > test catch both create and non-create cases?
> > 
> > 
> > >  		op->intent |= LOOKUP_CREATE;
> > >  		if (flags & O_EXCL) {
> > >  			op->intent |= LOOKUP_EXCL;
> > > diff --git a/fs/smb/client/dir.c b/fs/smb/client/dir.c
> > > index e4295a5b55b3..ec8c54c91261 100644
> > > --- a/fs/smb/client/dir.c
> > > +++ b/fs/smb/client/dir.c
> > > @@ -526,6 +526,9 @@ int cifs_atomic_open(struct inode *dir, struct dentry *direntry,
> > >  	if (unlikely(cifs_forced_shutdown(cifs_sb)))
> > >  		return smb_EIO(smb_eio_trace_forced_shutdown);
> > >  
> > > +	if ((oflags & O_MKDIR_MASK) == O_MKDIR_MASK)
> > > +		return -EINVAL;
> > > +
> > >  	/*
> > >  	 * Posix open is only called (at lookup time) for file create now. For
> > >  	 * opens (rather than creates), because we do not know if it is a file
> > > diff --git a/fs/vboxsf/dir.c b/fs/vboxsf/dir.c
> > > index 42bedc4ec7af..aef5ca6730be 100644
> > > --- a/fs/vboxsf/dir.c
> > > +++ b/fs/vboxsf/dir.c
> > > @@ -318,6 +318,9 @@ static int vboxsf_dir_atomic_open(struct inode *parent, struct dentry *dentry,
> > >  	u64 handle;
> > >  	int err;
> > >  
> > > +	if ((flags & O_MKDIR_MASK) == O_MKDIR_MASK)
> > > +		return -EINVAL;
> > > +
> > >  	if (d_in_lookup(dentry)) {
> > >  		struct dentry *res = vboxsf_dir_lookup(parent, dentry, 0);
> > >  		if (res || d_really_is_positive(dentry))
> > > diff --git a/include/linux/fcntl.h b/include/linux/fcntl.h
> > > index a332e79b3207..e31f3a57f07c 100644
> > > --- a/include/linux/fcntl.h
> > > +++ b/include/linux/fcntl.h
> > > @@ -12,6 +12,8 @@
> > >  	 FASYNC	| O_DIRECT | O_LARGEFILE | O_DIRECTORY | O_NOFOLLOW | \
> > >  	 O_NOATIME | O_CLOEXEC | O_PATH | __O_TMPFILE)
> > >  
> > > +#define O_MKDIR_MASK	(O_CREAT | O_DIRECTORY)
> > > +
> > >  /* List of all valid flags for the how->resolve argument: */
> > >  #define VALID_RESOLVE_FLAGS \
> > >  	(RESOLVE_NO_XDEV | RESOLVE_NO_MAGICLINKS | RESOLVE_NO_SYMLINKS | \
> > > -- 
> > > 2.54.0
> > > 
> > > 
> > > 
> > 
> > Thanks,
> > NeilBrown


Best,
Jori.

Re: [RFC PATCH v5 1/2] vfs: add O_CREAT|O_DIRECTORY to open*(2)

[Christian Brauner]

On Mon, Jun 01, 2026 at 10:52:24PM +0200, Jori Koolstra wrote:
> 
> > Op 27-05-2026 13:43 CEST schreef Christian Brauner <brauner@kernel.org>:
> > 
> > I agree, let's not do booleans if we can avoid them. But fwiw, some
> > userspace projects require naked booleans to come with a comment like:
> > 
> > may_o_create(..., /* create_dir */ true)
> > 
> > Which makes this easier to handle. But conventions like this are
> > difficult to enforce - especially in the kernel.
> > 
> > Years ago I added vfs_prepare_mode() which should handle all that
> > uniformly. Back then I didn't pass S_IFDIR and wrote:
> > 
> >  * Note that it's currently valid for @type to be 0 if a directory is created.
> >  * Filesystems raise that flag individually and we need to check whether each
> >  * filesystem can deal with receiving S_IFDIR from the vfs before we enforce a
> >  * non-zero type.
> 
> Yeah, I think this confused me. Now that I look at it again we can just get
> rid of that bool flag in may_o_create() since we just prepared the mode with
> vfs_prepare_mode(). It would be nice to also remove it from o_create_mode(),
> but off the top of my head, nothing is stopping you from passing a mode type
> to open(2) that has more than one bit set (e.g. both S_IFDIR and S_IFREG), and
> this is precisely what vfs_prepare_mode() is for to fix.
> 
> I was planning to get rid off this comment and enforce S_IFDIR, but I think
> that should be separate from this patchset. Agree?

Sure.

Re: [RFC PATCH v5 1/2] vfs: add O_CREAT|O_DIRECTORY to open*(2)

[Jori Koolstra]

> Op 27-05-2026 09:27 CEST schreef NeilBrown <neilb@ownmail.net>:
>
> > @@ -1268,6 +1269,8 @@ inline int build_open_flags(const struct open_how *how, struct open_flags *op)
> >  	op->intent = flags & O_PATH ? 0 : LOOKUP_OPEN;
> >  
> >  	if (flags & O_CREAT) {
> > +		if ((flags & O_DIRECTORY) && (acc_mode & MAY_WRITE))
> > +			return -EISDIR;
> 
> It seems odd that the MAY_WRITE test is only performed for O_CREAT.
> Shouldn't any open of O_DIRECTORY|O_WRONLY fail, and shouldn't the one
> test catch both create and non-create cases?
> 

I get your point, it does look odd. But AFAIS O_DIRECTORY|O_WRONLY is currently only blocked
after path resolution and final lookup. So that was the behavior I tried to keep. On the
other hand, right now O_DIRECTORY|O_CREAT is already blocked in build_open_flags().

But if that is true, and you have atomic_open() defined for your filesystem, then you
can lookup and open a directory with O_DIRECTORY|O_WRONLY, while it fails with EISDIR later
(if the fs atomic_open() does not error on O_DIRECTORY|O_WRONLY, which I would guess it
mostly does). It will then fput_close() the file, so there is no leak, but I don't think
that is wanted behavior.

@Christian, what do you think?

> 
> >  		op->intent |= LOOKUP_CREATE;
> >  		if (flags & O_EXCL) {
> >  			op->intent |= LOOKUP_EXCL;
> > diff --git a/fs/smb/client/dir.c b/fs/smb/client/dir.c
> > index e4295a5b55b3..ec8c54c91261 100644
> > --- a/fs/smb/client/dir.c
> > +++ b/fs/smb/client/dir.c
> > @@ -526,6 +526,9 @@ int cifs_atomic_open(struct inode *dir, struct dentry *direntry,
> >  	if (unlikely(cifs_forced_shutdown(cifs_sb)))
> >  		return smb_EIO(smb_eio_trace_forced_shutdown);
> >  
> > +	if ((oflags & O_MKDIR_MASK) == O_MKDIR_MASK)
> > +		return -EINVAL;
> > +
> >  	/*
> >  	 * Posix open is only called (at lookup time) for file create now. For
> >  	 * opens (rather than creates), because we do not know if it is a file
> > diff --git a/fs/vboxsf/dir.c b/fs/vboxsf/dir.c
> > index 42bedc4ec7af..aef5ca6730be 100644
> > --- a/fs/vboxsf/dir.c
> > +++ b/fs/vboxsf/dir.c
> > @@ -318,6 +318,9 @@ static int vboxsf_dir_atomic_open(struct inode *parent, struct dentry *dentry,
> >  	u64 handle;
> >  	int err;
> >  
> > +	if ((flags & O_MKDIR_MASK) == O_MKDIR_MASK)
> > +		return -EINVAL;
> > +
> >  	if (d_in_lookup(dentry)) {
> >  		struct dentry *res = vboxsf_dir_lookup(parent, dentry, 0);
> >  		if (res || d_really_is_positive(dentry))
> > diff --git a/include/linux/fcntl.h b/include/linux/fcntl.h
> > index a332e79b3207..e31f3a57f07c 100644
> > --- a/include/linux/fcntl.h
> > +++ b/include/linux/fcntl.h
> > @@ -12,6 +12,8 @@
> >  	 FASYNC	| O_DIRECT | O_LARGEFILE | O_DIRECTORY | O_NOFOLLOW | \
> >  	 O_NOATIME | O_CLOEXEC | O_PATH | __O_TMPFILE)
> >  
> > +#define O_MKDIR_MASK	(O_CREAT | O_DIRECTORY)
> > +
> >  /* List of all valid flags for the how->resolve argument: */
> >  #define VALID_RESOLVE_FLAGS \
> >  	(RESOLVE_NO_XDEV | RESOLVE_NO_MAGICLINKS | RESOLVE_NO_SYMLINKS | \
> > -- 
> > 2.54.0
> > 
> > 
> > 
> 
> Thanks,
> NeilBrown

There is another point, I maybe should have mentioned in the cover letter: I have not attempted
to handle dangling symlinks for O_MKDIR. Not because I think they are a great idea (as Aleksa
has mentioned, but I am not very familiar with the dragons it entails), but I wanted to discuss
what behavior we want in this case. Do we say that we never do a mkdir after following a lookup
last symlink? I don't think that state is even recorded right now.

Best,
Jori.

Re: [RFC PATCH v5 1/2] vfs: add O_CREAT|O_DIRECTORY to open*(2)

[NeilBrown]

On Tue, 02 Jun 2026, Jori Koolstra wrote:
> > Op 27-05-2026 09:27 CEST schreef NeilBrown <neilb@ownmail.net>:
> >
> > > @@ -1268,6 +1269,8 @@ inline int build_open_flags(const struct open_how *how, struct open_flags *op)
> > >  	op->intent = flags & O_PATH ? 0 : LOOKUP_OPEN;
> > >  
> > >  	if (flags & O_CREAT) {
> > > +		if ((flags & O_DIRECTORY) && (acc_mode & MAY_WRITE))
> > > +			return -EISDIR;
> > 
> > It seems odd that the MAY_WRITE test is only performed for O_CREAT.
> > Shouldn't any open of O_DIRECTORY|O_WRONLY fail, and shouldn't the one
> > test catch both create and non-create cases?
> > 
> 
> I get your point, it does look odd. But AFAIS O_DIRECTORY|O_WRONLY is currently only blocked
> after path resolution and final lookup. So that was the behavior I tried to keep. On the
> other hand, right now O_DIRECTORY|O_CREAT is already blocked in build_open_flags().

I see that currently O_WRONLY for directories is only blocked in
may_open() which happens after we have the inode for the target, so
after any create.

So I agree that we need to put something in build_open_flags() which
happens much earlier before path-walk even starts.

I think it would be best to always block an O_WRONLY/O_RDWR combined
with O_DIRECTORY.  It can never succeed so there is no point in even
starting the path-walk.

But I too would like to know what Christian or Al think.

> 
> But if that is true, and you have atomic_open() defined for your filesystem, then you
> can lookup and open a directory with O_DIRECTORY|O_WRONLY, while it fails with EISDIR later
> (if the fs atomic_open() does not error on O_DIRECTORY|O_WRONLY, which I would guess it
> mostly does). It will then fput_close() the file, so there is no leak, but I don't think
> that is wanted behavior.
> 
> @Christian, what do you think?
> 
> > 
> > >  		op->intent |= LOOKUP_CREATE;
> > >  		if (flags & O_EXCL) {
> > >  			op->intent |= LOOKUP_EXCL;
> > > diff --git a/fs/smb/client/dir.c b/fs/smb/client/dir.c
> > > index e4295a5b55b3..ec8c54c91261 100644
> > > --- a/fs/smb/client/dir.c
> > > +++ b/fs/smb/client/dir.c
> > > @@ -526,6 +526,9 @@ int cifs_atomic_open(struct inode *dir, struct dentry *direntry,
> > >  	if (unlikely(cifs_forced_shutdown(cifs_sb)))
> > >  		return smb_EIO(smb_eio_trace_forced_shutdown);
> > >  
> > > +	if ((oflags & O_MKDIR_MASK) == O_MKDIR_MASK)
> > > +		return -EINVAL;
> > > +
> > >  	/*
> > >  	 * Posix open is only called (at lookup time) for file create now. For
> > >  	 * opens (rather than creates), because we do not know if it is a file
> > > diff --git a/fs/vboxsf/dir.c b/fs/vboxsf/dir.c
> > > index 42bedc4ec7af..aef5ca6730be 100644
> > > --- a/fs/vboxsf/dir.c
> > > +++ b/fs/vboxsf/dir.c
> > > @@ -318,6 +318,9 @@ static int vboxsf_dir_atomic_open(struct inode *parent, struct dentry *dentry,
> > >  	u64 handle;
> > >  	int err;
> > >  
> > > +	if ((flags & O_MKDIR_MASK) == O_MKDIR_MASK)
> > > +		return -EINVAL;
> > > +
> > >  	if (d_in_lookup(dentry)) {
> > >  		struct dentry *res = vboxsf_dir_lookup(parent, dentry, 0);
> > >  		if (res || d_really_is_positive(dentry))
> > > diff --git a/include/linux/fcntl.h b/include/linux/fcntl.h
> > > index a332e79b3207..e31f3a57f07c 100644
> > > --- a/include/linux/fcntl.h
> > > +++ b/include/linux/fcntl.h
> > > @@ -12,6 +12,8 @@
> > >  	 FASYNC	| O_DIRECT | O_LARGEFILE | O_DIRECTORY | O_NOFOLLOW | \
> > >  	 O_NOATIME | O_CLOEXEC | O_PATH | __O_TMPFILE)
> > >  
> > > +#define O_MKDIR_MASK	(O_CREAT | O_DIRECTORY)
> > > +
> > >  /* List of all valid flags for the how->resolve argument: */
> > >  #define VALID_RESOLVE_FLAGS \
> > >  	(RESOLVE_NO_XDEV | RESOLVE_NO_MAGICLINKS | RESOLVE_NO_SYMLINKS | \
> > > -- 
> > > 2.54.0
> > > 
> > > 
> > > 
> > 
> > Thanks,
> > NeilBrown
> 
> There is another point, I maybe should have mentioned in the cover letter: I have not attempted
> to handle dangling symlinks for O_MKDIR. Not because I think they are a great idea (as Aleksa
> has mentioned, but I am not very familiar with the dragons it entails), but I wanted to discuss
> what behavior we want in this case. Do we say that we never do a mkdir after following a lookup
> last symlink? I don't think that state is even recorded right now.

I think the state might be recorded in nd->depth.  But you probably
don't want to use that directly.  Maybe forcing LOOKUP_FOLLOW to be
cleared if O_CREAT|O_DIRECTORY is set would be good.  But what would
stop you opening an existing directory through a symlink....

Probably we need a clear statement of intended semantics which we can
review, agree on, then implement.  Have you looked at preparing a patch
for man-pages to document the change in behaviour for openat etc?

Thanks,
NeilBrown


> 
> Best,
> Jori.
> 

Re: [RFC PATCH v5 1/2] vfs: add O_CREAT|O_DIRECTORY to open*(2)

[Christian Brauner]

On Tue, Jun 02, 2026 at 08:58:23AM +1000, NeilBrown wrote:
> On Tue, 02 Jun 2026, Jori Koolstra wrote:
> > > Op 27-05-2026 09:27 CEST schreef NeilBrown <neilb@ownmail.net>:
> > >
> > > > @@ -1268,6 +1269,8 @@ inline int build_open_flags(const struct open_how *how, struct open_flags *op)
> > > >  	op->intent = flags & O_PATH ? 0 : LOOKUP_OPEN;
> > > >  
> > > >  	if (flags & O_CREAT) {
> > > > +		if ((flags & O_DIRECTORY) && (acc_mode & MAY_WRITE))
> > > > +			return -EISDIR;
> > > 
> > > It seems odd that the MAY_WRITE test is only performed for O_CREAT.
> > > Shouldn't any open of O_DIRECTORY|O_WRONLY fail, and shouldn't the one
> > > test catch both create and non-create cases?
> > > 
> > 
> > I get your point, it does look odd. But AFAIS O_DIRECTORY|O_WRONLY is currently only blocked
> > after path resolution and final lookup. So that was the behavior I tried to keep. On the
> > other hand, right now O_DIRECTORY|O_CREAT is already blocked in build_open_flags().
> 
> I see that currently O_WRONLY for directories is only blocked in
> may_open() which happens after we have the inode for the target, so
> after any create.

Yes, that's not acceptable.

> So I agree that we need to put something in build_open_flags() which
> happens much earlier before path-walk even starts.
> I think it would be best to always block an O_WRONLY/O_RDWR combined
> with O_DIRECTORY.  It can never succeed so there is no point in even
> starting the path-walk.

Yes, I agree. This would change error codes but I don't think it
matters:

* O_WRONLY | O_DIRECTORY on non-directory -> ENOTDIR
* O_WRONLY | O_DIRECTORY on     directory -> EISDIR

I don't think that really matters and we should be able to collapse this
to ENOTDIR.

> But I too would like to know what Christian or Al think.
> 
> > 
> > But if that is true, and you have atomic_open() defined for your filesystem, then you
> > can lookup and open a directory with O_DIRECTORY|O_WRONLY, while it fails with EISDIR later
> > (if the fs atomic_open() does not error on O_DIRECTORY|O_WRONLY, which I would guess it
> > mostly does). It will then fput_close() the file, so there is no leak, but I don't think
> > that is wanted behavior.
> > 
> > @Christian, what do you think?
> > 
> > > 
> > > >  		op->intent |= LOOKUP_CREATE;
> > > >  		if (flags & O_EXCL) {
> > > >  			op->intent |= LOOKUP_EXCL;
> > > > diff --git a/fs/smb/client/dir.c b/fs/smb/client/dir.c
> > > > index e4295a5b55b3..ec8c54c91261 100644
> > > > --- a/fs/smb/client/dir.c
> > > > +++ b/fs/smb/client/dir.c
> > > > @@ -526,6 +526,9 @@ int cifs_atomic_open(struct inode *dir, struct dentry *direntry,
> > > >  	if (unlikely(cifs_forced_shutdown(cifs_sb)))
> > > >  		return smb_EIO(smb_eio_trace_forced_shutdown);
> > > >  
> > > > +	if ((oflags & O_MKDIR_MASK) == O_MKDIR_MASK)
> > > > +		return -EINVAL;
> > > > +
> > > >  	/*
> > > >  	 * Posix open is only called (at lookup time) for file create now. For
> > > >  	 * opens (rather than creates), because we do not know if it is a file
> > > > diff --git a/fs/vboxsf/dir.c b/fs/vboxsf/dir.c
> > > > index 42bedc4ec7af..aef5ca6730be 100644
> > > > --- a/fs/vboxsf/dir.c
> > > > +++ b/fs/vboxsf/dir.c
> > > > @@ -318,6 +318,9 @@ static int vboxsf_dir_atomic_open(struct inode *parent, struct dentry *dentry,
> > > >  	u64 handle;
> > > >  	int err;
> > > >  
> > > > +	if ((flags & O_MKDIR_MASK) == O_MKDIR_MASK)
> > > > +		return -EINVAL;
> > > > +
> > > >  	if (d_in_lookup(dentry)) {
> > > >  		struct dentry *res = vboxsf_dir_lookup(parent, dentry, 0);
> > > >  		if (res || d_really_is_positive(dentry))
> > > > diff --git a/include/linux/fcntl.h b/include/linux/fcntl.h
> > > > index a332e79b3207..e31f3a57f07c 100644
> > > > --- a/include/linux/fcntl.h
> > > > +++ b/include/linux/fcntl.h
> > > > @@ -12,6 +12,8 @@
> > > >  	 FASYNC	| O_DIRECT | O_LARGEFILE | O_DIRECTORY | O_NOFOLLOW | \
> > > >  	 O_NOATIME | O_CLOEXEC | O_PATH | __O_TMPFILE)
> > > >  
> > > > +#define O_MKDIR_MASK	(O_CREAT | O_DIRECTORY)
> > > > +
> > > >  /* List of all valid flags for the how->resolve argument: */
> > > >  #define VALID_RESOLVE_FLAGS \
> > > >  	(RESOLVE_NO_XDEV | RESOLVE_NO_MAGICLINKS | RESOLVE_NO_SYMLINKS | \
> > > > -- 
> > > > 2.54.0
> > > > 
> > > > 
> > > > 
> > > 
> > > Thanks,
> > > NeilBrown
> > 
> > There is another point, I maybe should have mentioned in the cover letter: I have not attempted
> > to handle dangling symlinks for O_MKDIR. Not because I think they are a great idea (as Aleksa
> > has mentioned, but I am not very familiar with the dragons it entails), but I wanted to discuss
> > what behavior we want in this case. Do we say that we never do a mkdir after following a lookup
> > last symlink? I don't think that state is even recorded right now.
> 
> I think the state might be recorded in nd->depth.  But you probably
> don't want to use that directly.  Maybe forcing LOOKUP_FOLLOW to be
> cleared if O_CREAT|O_DIRECTORY is set would be good.  But what would
> stop you opening an existing directory through a symlink....
> 
> Probably we need a clear statement of intended semantics which we can
> review, agree on, then implement.  Have you looked at preparing a patch
> for man-pages to document the change in behaviour for openat etc?

Ugh, dangling symlinks. Actually, scratch that: Ugh, symlinks. So
O_CREAT without O_NOFOLLOW allows you to create the target of a dangling
symlink iirc. I always forget that. I think this is a very subtle bug
and maybe - with both eyes closed - a feature at times.

We should straighten the behavior for O_DIRECTORY | O_CREAT and we
agreed on that during LSFMM. It would be nice if we could get away with
simply implying O_NOFOLLOW but I think you're right, Neil, that this
prevents a valid O_CREAT | O_DIRECTORY on an existing directory which we
can't do. Makes this kind of a pointless excercise.

But this shouldn't be all that crazy to do right. Using the O_CREAT as
an _example_ for what we'd need:

    fs: refuse O_CREAT through a dangling symlink

    open(O_CREAT) without O_EXCL follows a trailing symlink and, when the
    symlink target does not exist, creates it.  Refuse to create through a
    dangling symlink instead.

    In lookup_open() a negative target reached with nd->depth > 0 was
    arrived at by following a trailing symlink; since the dentry is negative
    the symlink is dangling. Set create_error to -ELOOP in that case.
    Reusing the existing create_error path strips O_CREAT for both the
    generic and ->atomic_open create paths and only reports the error when
    the target is actually negative, so opening an existing target through a
    symlink, interior symlinks, and O_EXCL (which never follows the trailing
    link) are all unaffected.

    Hastily-Cobbled-Together-by: Christian Brauner (Amutable) <brauner@kernel.org>

diff --git a/fs/namei.c b/fs/namei.c
index c7fac83c9a85..d20bbcc7e8d3 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -4468,6 +4468,9 @@ static struct dentry *lookup_open(struct nameidata *nd, struct file *file,
                                                    dentry, mode);
                else
                        create_error = -EROFS;
+               /* refuse to create through a dangling (trailing) symlink */
+               if (unlikely(nd->depth) && !create_error)
+                       create_error = -ELOOP;
        }
        if (create_error)
                open_flag &= ~O_CREAT;

It can't be that easy...

Re: [RFC PATCH v5 1/2] vfs: add O_CREAT|O_DIRECTORY to open*(2)

[Jori Koolstra]

> Op 02-06-2026 17:44 CEST schreef Christian Brauner <brauner@kernel.org>:
> 
> Yes, I agree. This would change error codes but I don't think it
> matters:
> 
> * O_WRONLY | O_DIRECTORY on non-directory -> ENOTDIR
> * O_WRONLY | O_DIRECTORY on     directory -> EISDIR
> 
> I don't think that really matters and we should be able to collapse this
> to ENOTDIR.

I will pick this up in the next version of O_CREAT|O_DIRECTORY. I think that
makes most sense.

I have an outstanding patch for changing the EACCES/EPERM to EOPNOTSUPP;
Jeff and Jan were skeptical, but I want to know your opinion as well.
I feel the the scenario where userspace has no fall-through but does
handle every single -E listed in the man-page quite unlikely, so I say
lets change them and we'll hear from them if somehow someone relied on
this weird way of error handling.

> > > 
> > > There is another point, I maybe should have mentioned in the cover letter: I have not attempted
> > > to handle dangling symlinks for O_MKDIR. Not because I think they are a great idea (as Aleksa
> > > has mentioned, but I am not very familiar with the dragons it entails), but I wanted to discuss
> > > what behavior we want in this case. Do we say that we never do a mkdir after following a lookup
> > > last symlink? I don't think that state is even recorded right now.
> > 
> > I think the state might be recorded in nd->depth.  But you probably
> > don't want to use that directly.  Maybe forcing LOOKUP_FOLLOW to be
> > cleared if O_CREAT|O_DIRECTORY is set would be good.  But what would
> > stop you opening an existing directory through a symlink....
> > 
> > Probably we need a clear statement of intended semantics which we can
> > review, agree on, then implement.  Have you looked at preparing a patch
> > for man-pages to document the change in behaviour for openat etc?
> 
> Ugh, dangling symlinks. Actually, scratch that: Ugh, symlinks. So
> O_CREAT without O_NOFOLLOW allows you to create the target of a dangling
> symlink iirc. I always forget that. I think this is a very subtle bug
> and maybe - with both eyes closed - a feature at times.
> 
> We should straighten the behavior for O_DIRECTORY | O_CREAT and we
> agreed on that during LSFMM. It would be nice if we could get away with
> simply implying O_NOFOLLOW but I think you're right, Neil, that this
> prevents a valid O_CREAT | O_DIRECTORY on an existing directory which we
> can't do. Makes this kind of a pointless excercise.
> 
> But this shouldn't be all that crazy to do right. Using the O_CREAT as
> an _example_ for what we'd need:
> 
>     fs: refuse O_CREAT through a dangling symlink
> 
>     open(O_CREAT) without O_EXCL follows a trailing symlink and, when the
>     symlink target does not exist, creates it.  Refuse to create through a
>     dangling symlink instead.
> 
>     In lookup_open() a negative target reached with nd->depth > 0 was
>     arrived at by following a trailing symlink; since the dentry is negative
>     the symlink is dangling. Set create_error to -ELOOP in that case.
>     Reusing the existing create_error path strips O_CREAT for both the
>     generic and ->atomic_open create paths and only reports the error when
>     the target is actually negative, so opening an existing target through a
>     symlink, interior symlinks, and O_EXCL (which never follows the trailing
>     link) are all unaffected.
> 
>     Hastily-Cobbled-Together-by: Christian Brauner (Amutable) <brauner@kernel.org>
> 
> diff --git a/fs/namei.c b/fs/namei.c
> index c7fac83c9a85..d20bbcc7e8d3 100644
> --- a/fs/namei.c
> +++ b/fs/namei.c
> @@ -4468,6 +4468,9 @@ static struct dentry *lookup_open(struct nameidata *nd, struct file *file,
>                                                     dentry, mode);
>                 else
>                         create_error = -EROFS;
> +               /* refuse to create through a dangling (trailing) symlink */
> +               if (unlikely(nd->depth) && !create_error)
> +                       create_error = -ELOOP;
>         }
>         if (create_error)
>                 open_flag &= ~O_CREAT;
> 
> It can't be that easy...

This is what I suggested above, correct, in terms of behavior?

In terms of the patch, I think this will work, but struct nameidata could really
use some commentary for its fields. I spent the last two hours verifying that
nd->depth really does what I thought it did, and I am still not 100% positive.
AFAIS, nd->depth indeed tracks the current symlink depth, which outside of
link_path_walk() reduces to the number of trailing links followed.

But if Neil's rework of lookup_open() is merged we lose access here to nd.
@Neil, have you thought about what would be a good way to resolve that?

Re: [RFC PATCH v5 1/2] vfs: add O_CREAT|O_DIRECTORY to open*(2)

[NeilBrown]

On Wed, 03 Jun 2026, Jori Koolstra wrote:
> > Op 02-06-2026 17:44 CEST schreef Christian Brauner <brauner@kernel.org>:
> > 
> > Yes, I agree. This would change error codes but I don't think it
> > matters:
> > 
> > * O_WRONLY | O_DIRECTORY on non-directory -> ENOTDIR
> > * O_WRONLY | O_DIRECTORY on     directory -> EISDIR
> > 
> > I don't think that really matters and we should be able to collapse this
> > to ENOTDIR.
> 
> I will pick this up in the next version of O_CREAT|O_DIRECTORY. I think that
> makes most sense.
> 
> I have an outstanding patch for changing the EACCES/EPERM to EOPNOTSUPP;
> Jeff and Jan were skeptical, but I want to know your opinion as well.
> I feel the the scenario where userspace has no fall-through but does
> handle every single -E listed in the man-page quite unlikely, so I say
> lets change them and we'll hear from them if somehow someone relied on
> this weird way of error handling.

Please cc linux-api@vger.kernel.org on code and discussions that involve
API changes.  I have cc:ed them on this reply.

Thanks,
NeilBrown


> 
> > > > 
> > > > There is another point, I maybe should have mentioned in the cover letter: I have not attempted
> > > > to handle dangling symlinks for O_MKDIR. Not because I think they are a great idea (as Aleksa
> > > > has mentioned, but I am not very familiar with the dragons it entails), but I wanted to discuss
> > > > what behavior we want in this case. Do we say that we never do a mkdir after following a lookup
> > > > last symlink? I don't think that state is even recorded right now.
> > > 
> > > I think the state might be recorded in nd->depth.  But you probably
> > > don't want to use that directly.  Maybe forcing LOOKUP_FOLLOW to be
> > > cleared if O_CREAT|O_DIRECTORY is set would be good.  But what would
> > > stop you opening an existing directory through a symlink....
> > > 
> > > Probably we need a clear statement of intended semantics which we can
> > > review, agree on, then implement.  Have you looked at preparing a patch
> > > for man-pages to document the change in behaviour for openat etc?
> > 
> > Ugh, dangling symlinks. Actually, scratch that: Ugh, symlinks. So
> > O_CREAT without O_NOFOLLOW allows you to create the target of a dangling
> > symlink iirc. I always forget that. I think this is a very subtle bug
> > and maybe - with both eyes closed - a feature at times.
> > 
> > We should straighten the behavior for O_DIRECTORY | O_CREAT and we
> > agreed on that during LSFMM. It would be nice if we could get away with
> > simply implying O_NOFOLLOW but I think you're right, Neil, that this
> > prevents a valid O_CREAT | O_DIRECTORY on an existing directory which we
> > can't do. Makes this kind of a pointless excercise.
> > 
> > But this shouldn't be all that crazy to do right. Using the O_CREAT as
> > an _example_ for what we'd need:
> > 
> >     fs: refuse O_CREAT through a dangling symlink
> > 
> >     open(O_CREAT) without O_EXCL follows a trailing symlink and, when the
> >     symlink target does not exist, creates it.  Refuse to create through a
> >     dangling symlink instead.
> > 
> >     In lookup_open() a negative target reached with nd->depth > 0 was
> >     arrived at by following a trailing symlink; since the dentry is negative
> >     the symlink is dangling. Set create_error to -ELOOP in that case.
> >     Reusing the existing create_error path strips O_CREAT for both the
> >     generic and ->atomic_open create paths and only reports the error when
> >     the target is actually negative, so opening an existing target through a
> >     symlink, interior symlinks, and O_EXCL (which never follows the trailing
> >     link) are all unaffected.
> > 
> >     Hastily-Cobbled-Together-by: Christian Brauner (Amutable) <brauner@kernel.org>
> > 
> > diff --git a/fs/namei.c b/fs/namei.c
> > index c7fac83c9a85..d20bbcc7e8d3 100644
> > --- a/fs/namei.c
> > +++ b/fs/namei.c
> > @@ -4468,6 +4468,9 @@ static struct dentry *lookup_open(struct nameidata *nd, struct file *file,
> >                                                     dentry, mode);
> >                 else
> >                         create_error = -EROFS;
> > +               /* refuse to create through a dangling (trailing) symlink */
> > +               if (unlikely(nd->depth) && !create_error)
> > +                       create_error = -ELOOP;
> >         }
> >         if (create_error)
> >                 open_flag &= ~O_CREAT;
> > 
> > It can't be that easy...
> 
> This is what I suggested above, correct, in terms of behavior?
> 
> In terms of the patch, I think this will work, but struct nameidata could really
> use some commentary for its fields. I spent the last two hours verifying that
> nd->depth really does what I thought it did, and I am still not 100% positive.
> AFAIS, nd->depth indeed tracks the current symlink depth, which outside of
> link_path_walk() reduces to the number of trailing links followed.
> 
> But if Neil's rework of lookup_open() is merged we lose access here to nd.
> @Neil, have you thought about what would be a good way to resolve that?
> 

Re: [RFC PATCH v5 1/2] vfs: add O_CREAT|O_DIRECTORY to open*(2)

[Askar Safin]

Jori Koolstra <jkoolstra@xs4all.nl>:
> +static inline umode_t o_create_mode(struct mnt_idmap *idmap,
> +		const struct inode *dir, umode_t mode, bool create_dir)
> +{
> +	return create_dir ? vfs_prepare_mode(idmap, dir, mode, S_IRWXUGO | S_ISVTX, 0)
> +			  : vfs_prepare_mode(idmap, dir, mode, S_IALLUGO, S_IFREG);

checkpatch says here:
WARNING: Symbolic permissions 'S_IRWXUGO' are not preferred. Consider using octal permissions '0777'.
#4361: FILE: fs/namei.c:4361:
+       return create_dir ? vfs_prepare_mode(idmap, dir, mode, S_IRWXUGO | S_ISVTX, 0)


-- 
Askar Safin

[RFC PATCH v5 2/2] selftest: add tests for open*(O_CREAT|O_DIRECTORY)

[Jori Koolstra]

Add some tests for the new valid O_CREAT|O_DIRECTORY flag combination for
open*(2) to test compliance and to showcase its behaviour.

Signed-off-by: Jori Koolstra <jkoolstra@xs4all.nl>
---
 .../testing/selftests/filesystems/.gitignore  |   1 +
 tools/testing/selftests/filesystems/Makefile  |   4 +-
 tools/testing/selftests/filesystems/fclog.c   |   1 +
 .../filesystems/open_o_creat_o_dir.c          | 197 ++++++++++++++++++
 4 files changed, 201 insertions(+), 2 deletions(-)
 create mode 100644 tools/testing/selftests/filesystems/open_o_creat_o_dir.c

diff --git a/tools/testing/selftests/filesystems/.gitignore b/tools/testing/selftests/filesystems/.gitignore
index 64ac0dfa46b7..f257b3ddb479 100644
--- a/tools/testing/selftests/filesystems/.gitignore
+++ b/tools/testing/selftests/filesystems/.gitignore
@@ -1,4 +1,5 @@
 # SPDX-License-Identifier: GPL-2.0-only
+open_o_creat_o_dir
 dnotify_test
 devpts_pts
 fclog
diff --git a/tools/testing/selftests/filesystems/Makefile b/tools/testing/selftests/filesystems/Makefile
index 85427d7f19b9..ec7f93b700d2 100644
--- a/tools/testing/selftests/filesystems/Makefile
+++ b/tools/testing/selftests/filesystems/Makefile
@@ -1,7 +1,7 @@
 # SPDX-License-Identifier: GPL-2.0
 
-CFLAGS += $(KHDR_INCLUDES)
-TEST_GEN_PROGS := devpts_pts file_stressor anon_inode_test kernfs_test fclog
+CFLAGS += $(KHDR_INCLUDES) $(TOOLS_INCLUDES)
+TEST_GEN_PROGS := open_o_creat_o_dir devpts_pts file_stressor anon_inode_test kernfs_test fclog
 TEST_GEN_PROGS_EXTENDED := dnotify_test
 
 include ../lib.mk
diff --git a/tools/testing/selftests/filesystems/fclog.c b/tools/testing/selftests/filesystems/fclog.c
index 551c4a0f395a..33ed59286a2d 100644
--- a/tools/testing/selftests/filesystems/fclog.c
+++ b/tools/testing/selftests/filesystems/fclog.c
@@ -4,6 +4,7 @@
  * Copyright (C) 2025 SUSE LLC.
  */
 
+#include <fcntl.h>
 #include <assert.h>
 #include <errno.h>
 #include <sched.h>
diff --git a/tools/testing/selftests/filesystems/open_o_creat_o_dir.c b/tools/testing/selftests/filesystems/open_o_creat_o_dir.c
new file mode 100644
index 000000000000..03b5edcffeef
--- /dev/null
+++ b/tools/testing/selftests/filesystems/open_o_creat_o_dir.c
@@ -0,0 +1,197 @@
+// SPDX-License-Identifier: GPL-2.0
+#include <sys/stat.h>
+#include <sys/syscall.h>
+#include <errno.h>
+#include <limits.h>
+#include <fcntl.h>
+
+#include "kselftest_harness.h"
+
+static inline int open_o_creat_o_dir(int dfd, const char *pathname,
+				     mode_t mode, unsigned int flags)
+{
+	return syscall(__NR_openat, dfd, pathname,
+		       flags | O_DIRECTORY | O_CREAT, mode);
+}
+
+#define open_o_creat_o_dir_checked_flags(dfd, pathname, flags) ({	\
+	struct stat __st;						\
+	int __fd = open_o_creat_o_dir(dfd, pathname, S_IRWXU, flags);	\
+	ASSERT_GE(__fd, 0);						\
+	ASSERT_EQ(fstat(__fd, &__st), 0);				\
+	EXPECT_TRUE(S_ISDIR(__st.st_mode));				\
+	__fd;								\
+})
+
+#define open_o_creat_o_dir_checked(dfd, pathname) \
+	open_o_creat_o_dir_checked_flags(dfd, pathname, 0)
+
+FIXTURE(open_o_creat_o_dir) {
+	char dirpath[PATH_MAX];
+	int dfd;
+};
+
+FIXTURE_SETUP(open_o_creat_o_dir)
+{
+	strcpy(self->dirpath, "/tmp/open_o_creat_o_dir_test.XXXXXX");
+	ASSERT_NE(mkdtemp(self->dirpath), NULL);
+	self->dfd = open(self->dirpath, O_DIRECTORY);
+	ASSERT_GE(self->dfd, 0);
+}
+
+FIXTURE_TEARDOWN(open_o_creat_o_dir)
+{
+	close(self->dfd);
+	rmdir(self->dirpath);
+}
+
+/* Does open_o_creat_o_dir return a fd at all? */
+TEST_F(open_o_creat_o_dir, returns_fd)
+{
+	int fd = open_o_creat_o_dir_checked(self->dfd, "newdir");
+	EXPECT_EQ(close(fd), 0);
+	EXPECT_EQ(unlinkat(self->dfd, "newdir", AT_REMOVEDIR), 0);
+}
+
+/* The fd must refer to the directory that was just created. */
+TEST_F(open_o_creat_o_dir, fd_is_created_dir)
+{
+	int fd;
+	struct stat st_via_fd, st_via_path;
+	char path[PATH_MAX];
+
+	fd = open_o_creat_o_dir_checked(self->dfd, "checkdir");
+
+	ASSERT_EQ(fstat(fd, &st_via_fd), 0);
+
+	snprintf(path, sizeof(path), "%s/checkdir", self->dirpath);
+	ASSERT_EQ(stat(path, &st_via_path), 0);
+
+	EXPECT_EQ(st_via_fd.st_ino, st_via_path.st_ino);
+	EXPECT_EQ(st_via_fd.st_dev, st_via_path.st_dev);
+
+	EXPECT_EQ(close(fd), 0);
+	EXPECT_EQ(rmdir(path), 0);
+}
+
+/* Missing parent component must fail with ENOENT. */
+TEST_F(open_o_creat_o_dir, enoent_missing_parent)
+{
+	EXPECT_EQ(open_o_creat_o_dir(self->dfd, "nonexistent/child", S_IRWXU, 0), -1);
+	EXPECT_EQ(errno, ENOENT);
+}
+
+/* An invalid dfd must fail with EBADF. */
+TEST_F(open_o_creat_o_dir, ebadf)
+{
+	EXPECT_EQ(open_o_creat_o_dir(-42, "badfdir", S_IRWXU, 0), -1);
+	EXPECT_EQ(errno, EBADF);
+}
+
+/* A dfd that points to a file (not a directory) must fail with ENOTDIR. */
+TEST_F(open_o_creat_o_dir, enotdir_dfd)
+{
+	int file_fd;
+
+	file_fd = openat(self->dfd, "file",
+			 O_CREAT | O_WRONLY, S_IRWXU);
+	ASSERT_GE(file_fd, 0);
+
+	EXPECT_EQ(open_o_creat_o_dir(file_fd, "subdir", S_IRWXU, 0), -1);
+	EXPECT_EQ(errno, ENOTDIR);
+
+	EXPECT_EQ(close(file_fd), 0);
+	EXPECT_EQ(unlinkat(self->dfd, "file", 0), 0);
+}
+
+/*
+ * O_EXCL together with O_CREAT|O_DIRECTORY must fail with EEXIST when
+ * the target directory already exists.
+ */
+TEST_F(open_o_creat_o_dir, o_excl_eexist)
+{
+	int fd;
+
+	fd = open_o_creat_o_dir_checked_flags(self->dfd, "excldir", O_EXCL);
+	EXPECT_EQ(close(fd), 0);
+
+	EXPECT_EQ(open_o_creat_o_dir(self->dfd, "excldir", S_IRWXU, O_EXCL), -1);
+	EXPECT_EQ(errno, EEXIST);
+
+	EXPECT_EQ(unlinkat(self->dfd, "excldir", AT_REMOVEDIR), 0);
+}
+
+/*
+ * O_CREAT|O_DIRECTORY on a path that already exists as a regular file
+ * must fail with ENOTDIR.
+ */
+TEST_F(open_o_creat_o_dir, existing_file_enotdir)
+{
+	int file_fd;
+
+	file_fd = openat(self->dfd, "regfile",
+			 O_CREAT | O_WRONLY, S_IRWXU);
+	ASSERT_GE(file_fd, 0);
+	EXPECT_EQ(close(file_fd), 0);
+
+	EXPECT_EQ(open_o_creat_o_dir(self->dfd, "regfile", S_IRWXU, 0), -1);
+	EXPECT_EQ(errno, ENOTDIR);
+
+	EXPECT_EQ(unlinkat(self->dfd, "regfile", 0), 0);
+}
+
+/*
+ * O_CREAT|O_DIRECTORY combined with a writable access mode must be
+ * rejected: a directory cannot be opened for writing.
+ */
+TEST_F(open_o_creat_o_dir, rejects_writable_acc_mode)
+{
+	EXPECT_EQ(open_o_creat_o_dir(self->dfd, "rdwrdir", S_IRWXU, O_RDWR), -1);
+	EXPECT_EQ(errno, EISDIR);
+	/* Clean up if the kernel created the directory anyway. */
+	unlinkat(self->dfd, "rdwrdir", AT_REMOVEDIR);
+}
+
+/*
+ * openat(O_CREAT) with a trailing slash but without O_DIRECTORY
+ * must fail with EISDIR and must not create anything at the path.
+ */
+TEST_F(open_o_creat_o_dir, trailing_slash_no_o_dir)
+{
+	int fd;
+	struct stat st;
+
+	fd = openat(self->dfd, "trailing/", O_CREAT | O_WRONLY, S_IRWXU);
+	EXPECT_EQ(fd, -1);
+	EXPECT_EQ(errno, EISDIR);
+
+	EXPECT_EQ(fstatat(self->dfd, "trailing", &st, 0), -1);
+	EXPECT_EQ(errno, ENOENT);
+
+	/* Best-effort cleanup in case the kernel left a file behind. */
+	if (fd >= 0)
+		close(fd);
+	unlinkat(self->dfd, "trailing", 0);
+}
+
+/*
+ * The returned fd must be usable as a dfd for further *at() calls.
+ */
+TEST_F(open_o_creat_o_dir, fd_usable_as_dfd)
+{
+	int parent_fd, child_fd;
+	char path[PATH_MAX];
+
+	parent_fd = open_o_creat_o_dir_checked(self->dfd, "parent");
+	child_fd = open_o_creat_o_dir_checked(parent_fd, "child");
+
+	EXPECT_EQ(close(child_fd), 0);
+	EXPECT_EQ(close(parent_fd), 0);
+
+	snprintf(path, sizeof(path), "%s/parent/child", self->dirpath);
+	EXPECT_EQ(rmdir(path), 0);
+	snprintf(path, sizeof(path), "%s/parent", self->dirpath);
+	EXPECT_EQ(rmdir(path), 0);
+}
+
+TEST_HARNESS_MAIN
-- 
2.54.0

Re: [RFC PATCH v5 0/2] vfs: add O_CREAT|O_DIRECTORY to open*(2)

[Askar Safin]

Jori Koolstra <jkoolstra@xs4all.nl>:
> This series implements new semantics for the O_CREAT|O_DIRECTORY flag
> combination for open*(2): perform a mkdir and open the resulting
> directory; return a pinning fd (which mkdir does not).

Al Viro strongly opposed this idea back in 2020:
> For fuck sake, *NO*!
> We don't need any more multiplexors from hell.

https://lore.kernel.org/all/20200313182844.GO23230@ZenIV.linux.org.uk/

So, at my opinion, at very least, Ack-By from Al Viro is mandatory.

-- 
Askar Safin