[PATCH v7 00/12] vfs: add O_CREAT|O_DIRECTORY to open*(2)

[PATCH v7 00/12] 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).

The v6 was found by Brauner to contain a bug because ->mkdir
implementations may leave the dentry negative, and do_open() cannot deal
with that (NULL pointer deref.) This behavior occurs in kernfs and
tracefs at least. Since atomic mkdir is wanted for cgroupfs (as per
Brauner at LPC26), kernfs is changed to return the new kernfs_node from
the kernfs_syscall_ops ->mkdir() callbacks and splicing its inode.

Filesystems that do not support O_CREAT|O_DIRECTORY set
SB_I_NO_ATOMIC_MKDIR, and lookup_open() refuses the open with EOPNOTSUPP
for them before doing any lookup. These are all ->atomic_open()
filesystems, which cannot create directories without protocol
extensions, and tracefs, whose ->mkdir() drops the parent's i_rwsem so
that the directory at the name may not be the one created by the call
(or at least it has to be verified that this plays well with the
atomicity guarantee that O_CREAT|O_DIRECTORY attempts to provide.) For
ecryptfs and apparmorfs there are similar concerns and hence they are
also blocked for now.

Most of the work happens in "vfs: add O_CREAT|O_DIRECTORY to open*(2),"
as may be expected. Before that there is some clean-up work and
preparation. The "vfs: short-circuit MAY_WRITE access for O_DIRECTORY
opens" patch is also worth paying extra attention to as it
short-circuits doomed opening of directories as writable. This check (to
prevent one from opening directories as writable) is currently done very
late in do_open(), after an inode has been obtained. However, when
introducing O_CREAT|O_DIRECTORY this is unacceptable as it would create
the directory and then still fail. This patch does however change some
user visible error codes.

Moreover, "vfs: change ->create/->mkdir operations unavailable errno"
also changes the ->create and ->mkdir unavailable errnos to -EOPNOTSUPP.
A previous attempt to do this more widely stranded before.[1] However,
that regression was only for vfat and does seem specific to changing the
return code from vfs_symlink(). We can drop this patch if needed, but it
would be _really_ ugly.

Changes from vn to v(n+1):
v6: https://lore.kernel.org/linux-fsdevel/20260913185016.523376-1-jkoolstra@xs4all.nl/
   - Change kernfs to instantiate ->mkdir dentry.
   - Fix kernfs locking when initializing a new inode to hold
     kernfs_iattr_rwsem.
   - Add flag SB_I_NO_ATOMIC_MKDIR to disable O_CREAT|O_DIRECTORY on
     unsupported filesystems (instead of only looking at ->atomic_open.)
   - Add tests to verify behavior of O_CREAT|O_DIRECTORY on cgroupfs and
     tracefs.
v5: https://lore.kernel.org/linux-fsdevel/20260823160706.358293-1-jkoolstra@xs4all.nl/
   - Uniformize ->create and ->mkdir unavailable errnos to -EOPNOTSUPP.
   - Return -EOPNOTSUPP instead of -ENOENT when O_CREAT|O_DIRECTORY is
     not supported by a ->atomic_open filesystem.
   - Added test cases for dangling symlink and sticky directory
     behaviour, and a test that verifies O_TMPFILE|O_CREAT is still
     -EINVAL.
   - Split off "vfs: lookup_open(): lock the parent as I_MUTEX_PARENT"
     as a separate patch.
   - Fixed the issues pointed out by Brauner in v5.
v4: https://lore.kernel.org/linux-fsdevel/20260712175539.1565444-1-jkoolstra@xs4all.nl/
   - rebased on 7.2, which includes my changes to auditing in
     lookup_open() as well as Neil Brown's changes to the same function
     for the implementation of vfs_lookup_open().
v3: https://lore.kernel.org/linux-fsdevel/20260704164149.3480051-1-jkoolstra@xs4all.nl/
   - address the inconsistency in calling audit_inode_child() in
     lookup_open() versus vfs_create() in a separate series.
   - fclog.c selftest header fix moved to separate patch.
   - add a "with trailing slash test" success test to the included
     selftests. 
   - pass struct qstr by pointer to trailing_slashes() and add a comment
     on what it does
   - changed commit message of "vfs: add O_CREAT|O_DIRECTORY to
     open*(2)" to address comments of Brauner

[1]: https://lore.kernel.org/linux-fsdevel/90228c39-04e7-41ef-ad91-84a8bb866650@sirena.org.uk/


Jori Koolstra (12):
  fs/namei.c: use trailing_slashes()
  vfs: prepare vfs_creat|mkdir_no_perm for reuse in lookup_open()
  vfs: lookup_open(): move setting FMODE_CREATED down
  vfs: move ->create check in lookup_open() to before try_break_deleg()
  vfs: lookup_open(): use vfs_create_no_perm()
  vfs: lookup_open(): lock the parent as I_MUTEX_PARENT
  kernfs: hold kernfs_iattr_rwsem when initializing a new inode
  vfs: add O_CREAT|O_DIRECTORY to open*(2)
  vfs: use finish_lookup() helper in __lookup_slow() and lookup_open()
  vfs: short-circuit MAY_WRITE access for O_DIRECTORY opens
  vfs: change ->create/->mkdir operations unavailable errno
  selftest: add tests for open*(O_CREAT|O_DIRECTORY)

 arch/alpha/include/uapi/asm/errno.h           |   2 +-
 arch/mips/include/uapi/asm/errno.h            |   2 +-
 arch/parisc/include/uapi/asm/errno.h          |   2 +-
 arch/sparc/include/uapi/asm/errno.h           |   2 +-
 fs/9p/vfs_super.c                             |   1 +
 fs/ceph/super.c                               |   1 +
 fs/ecryptfs/main.c                            |   1 +
 fs/fuse/inode.c                               |   1 +
 fs/gfs2/ops_fstype.c                          |   1 +
 fs/kernfs/dir.c                               |  30 +-
 fs/kernfs/inode.c                             |  37 +-
 fs/kernfs/kernfs-internal.h                   |   2 +
 fs/kernfs/mount.c                             |   3 -
 fs/namei.c                                    | 275 +++++++++-----
 fs/nfs/super.c                                |   1 +
 fs/open.c                                     |  42 ++-
 fs/resctrl/rdtgroup.c                         |  35 +-
 fs/smb/client/cifsfs.c                        |   1 +
 fs/tracefs/inode.c                            |   1 +
 fs/vboxsf/super.c                             |   1 +
 include/linux/fcntl.h                         |   6 +
 include/linux/fs/super_types.h                |   1 +
 include/linux/kernfs.h                        |   7 +-
 include/uapi/asm-generic/errno.h              |   2 +-
 kernel/cgroup/cgroup-internal.h               |   3 +-
 kernel/cgroup/cgroup.c                        |  16 +-
 security/apparmor/apparmorfs.c                |   1 +
 .../testing/selftests/filesystems/.gitignore  |   1 +
 tools/testing/selftests/filesystems/Makefile  |   2 +-
 .../filesystems/open_o_creat_o_dir.c          | 346 ++++++++++++++++++
 .../testing/selftests/filesystems/wrappers.h  |  11 +
 31 files changed, 691 insertions(+), 146 deletions(-)
 create mode 100644 tools/testing/selftests/filesystems/open_o_creat_o_dir.c


base-commit: 90201351a862182bdabfc1ad1c97b1e1b6ed5931
-- 
2.56.0

[PATCH v7 01/12] fs/namei.c: use trailing_slashes()

[Jori Koolstra]

There are several places in fs/namei.c that can use the
trailing_slashes() function to improve context. To allow this broader
use its signature is changed to take a struct qstr instead of a struct
nameidata.

Reviewed-by: NeilBrown <neil@brown.name>
Signed-off-by: Jori Koolstra <jkoolstra@xs4all.nl>
---
 fs/namei.c | 28 +++++++++++++++-------------
 1 file changed, 15 insertions(+), 13 deletions(-)

diff --git a/fs/namei.c b/fs/namei.c
index c87c14ee1a25..839258260ad5 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -2810,9 +2810,16 @@ static const char *path_init(struct nameidata *nd, unsigned flags)
 	return s;
 }
 
+static inline bool trailing_slashes(const struct qstr *last)
+{
+	/* last->len is set by hash_name() to the length of the current
+	 * component ->name, terminating with '/' or a NUL character. */
+	return (bool)last->name[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->last))
 		nd->flags |= LOOKUP_FOLLOW | LOOKUP_DIRECTORY;
 
 	return walk_component(nd, WALK_TRAILING);
@@ -4724,17 +4731,12 @@ struct file *vfs_lookup_open(struct path *parent, struct qstr *last,
 }
 EXPORT_SYMBOL_FOR_MODULES(vfs_lookup_open, "nfsd");
 
-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->last))
 			return ERR_PTR(-EISDIR);
 
 		/* Don't bother on an O_EXCL create */
@@ -4742,7 +4744,7 @@ static struct dentry *lookup_fast_for_open(struct nameidata *nd, int open_flag)
 			return NULL;
 	}
 
-	if (trailing_slashes(nd))
+	if (trailing_slashes(&nd->last))
 		nd->flags |= LOOKUP_FOLLOW | LOOKUP_DIRECTORY;
 
 	dentry = lookup_fast(nd);
@@ -5116,7 +5118,7 @@ static struct dentry *filename_create(int dfd, struct filename *name,
 	 * Do the final lookup.  Suppress 'create' if there is a trailing
 	 * '/', and a directory wasn't requested.
 	 */
-	if (last.name[last.len] && !want_dir)
+	if (trailing_slashes(&last) && !want_dir)
 		create_flags &= ~LOOKUP_CREATE;
 	dentry = start_dirop(path->dentry, &last, reval_flag | create_flags);
 	if (IS_ERR(dentry))
@@ -5732,7 +5734,7 @@ int filename_unlinkat(int dfd, struct filename *name)
 		goto exit_drop_write;
 
 	/* Why not before? Because we want correct error value */
-	if (unlikely(last.name[last.len])) {
+	if (unlikely(trailing_slashes(&last))) {
 		if (d_is_dir(dentry))
 			error = -EISDIR;
 		else
@@ -6334,16 +6336,16 @@ int filename_renameat2(int olddfd, struct filename *from,
 	if (flags & RENAME_EXCHANGE) {
 		if (!d_is_dir(rd.new_dentry)) {
 			error = -ENOTDIR;
-			if (new_last.name[new_last.len])
+			if (trailing_slashes(&new_last))
 				goto exit_unlock;
 		}
 	}
 	/* unless the source is a directory trailing slashes give -ENOTDIR */
 	if (!d_is_dir(rd.old_dentry)) {
 		error = -ENOTDIR;
-		if (old_last.name[old_last.len])
+		if (trailing_slashes(&old_last))
 			goto exit_unlock;
-		if (!(flags & RENAME_EXCHANGE) && new_last.name[new_last.len])
+		if (!(flags & RENAME_EXCHANGE) && trailing_slashes(&new_last))
 			goto exit_unlock;
 	}
 
-- 
2.56.0

[PATCH v7 02/12] vfs: prepare vfs_creat|mkdir_no_perm for reuse in lookup_open()

[Jori Koolstra]

To implement O_CREAT|O_DIRECTORY we will have to repeat some of the
logic that is now in vfs_mkdir() (e.g. do error checks in the same
order). Separate this out in vfs_mkdir_no_perm(), which does all the
non-permission related work of vfs_mkdir(). Permission checking for the
lookup_open() path is timed differently because we may just be doing an
open and no create. Similar considerations give rise to
vfs_create_no_perm().

Signed-off-by: Jori Koolstra <jkoolstra@xs4all.nl>
---
 fs/namei.c | 77 +++++++++++++++++++++++++++++++++++++-----------------
 1 file changed, 53 insertions(+), 24 deletions(-)

diff --git a/fs/namei.c b/fs/namei.c
index 839258260ad5..ab09dd7f7475 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -4195,6 +4195,24 @@ static inline umode_t vfs_prepare_mode(struct mnt_idmap *idmap,
 	return mode;
 }
 
+static inline
+int vfs_create_no_perm(struct mnt_idmap *idmap, struct dentry *dentry,
+		       umode_t mode, struct delegated_inode *di)
+{
+	struct inode *dir = d_inode(dentry->d_parent);
+	int error;
+
+	error = try_break_deleg(dir, LEASE_BREAK_DIR_CREATE, di);
+	if (error)
+		return error;
+
+	error = dir->i_op->create(idmap, dir, dentry, mode);
+	if (!error)
+		fsnotify_create(dir, dentry);
+
+	return error;
+}
+
 /**
  * vfs_create - create new file
  * @idmap:	idmap of the mount the inode was found from
@@ -4227,13 +4245,8 @@ int vfs_create(struct mnt_idmap *idmap, struct dentry *dentry, umode_t mode,
 	error = security_inode_create(dir, dentry, mode);
 	if (error)
 		return error;
-	error = try_break_deleg(dir, LEASE_BREAK_DIR_CREATE, di);
-	if (error)
-		return error;
-	error = dir->i_op->create(idmap, dir, dentry, mode);
-	if (!error)
-		fsnotify_create(dir, dentry);
-	return error;
+
+	return vfs_create_no_perm(idmap, dentry, mode, di);
 }
 EXPORT_SYMBOL(vfs_create);
 
@@ -4447,6 +4460,7 @@ static struct dentry *atomic_open(const struct path *path, struct dentry *dentry
 		dput(dentry);
 		dentry = ERR_PTR(error);
 	}
+
 	return dentry;
 }
 
@@ -4573,6 +4587,7 @@ static struct dentry *lookup_open(struct nameidata *nd, struct file *file,
 			dentry = res;
 		}
 	}
+
 	if (dentry->d_inode || !(op->open_flag & O_CREAT)) {
 		/*
 		 * No need to create a file.  If lookup returned a positive
@@ -5387,6 +5402,33 @@ SYSCALL_DEFINE3(mknod, const char __user *, filename, umode_t, mode, unsigned, d
 	return filename_mknodat(AT_FDCWD, name, mode, dev);
 }
 
+static inline
+struct dentry *vfs_mkdir_no_perm(struct mnt_idmap *idmap, struct inode *dir,
+				 struct dentry *dentry, umode_t mode,
+				 struct delegated_inode *di)
+{
+	unsigned max_links = dir->i_sb->s_max_links;
+	struct dentry *de;
+	int error;
+
+	if (max_links && dir->i_nlink >= max_links)
+		return ERR_PTR(-EMLINK);
+
+	error = try_break_deleg(dir, LEASE_BREAK_DIR_CREATE, di);
+	if (error)
+		return ERR_PTR(error);
+
+	de = dir->i_op->mkdir(idmap, dir, dentry, mode);
+	if (IS_ERR(de))
+		return de;
+	if (de) {
+		dput(dentry);
+		dentry = de;
+	}
+	fsnotify_mkdir(dir, dentry);
+	return dentry;
+}
+
 /**
  * vfs_mkdir - create directory returning correct dentry if possible
  * @idmap:		idmap of the mount the inode was found from
@@ -5414,7 +5456,6 @@ struct dentry *vfs_mkdir(struct mnt_idmap *idmap, struct inode *dir,
 			 struct delegated_inode *delegated_inode)
 {
 	int error;
-	unsigned max_links = dir->i_sb->s_max_links;
 	struct dentry *de;
 
 	error = may_create_dentry(idmap, dir, dentry);
@@ -5430,24 +5471,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, LEASE_BREAK_DIR_CREATE, delegated_inode);
-	if (error)
+	de = vfs_mkdir_no_perm(idmap, dir, dentry, mode, delegated_inode);
+	if (IS_ERR(de)) {
+		error = PTR_ERR(de);
 		goto err;
-
-	de = dir->i_op->mkdir(idmap, dir, dentry, mode);
-	error = PTR_ERR(de);
-	if (IS_ERR(de))
-		goto err;
-	if (de) {
-		dput(dentry);
-		dentry = de;
 	}
-	fsnotify_mkdir(dir, dentry);
-	return dentry;
+	return de;
 
 err:
 	end_creating(dentry);
-- 
2.56.0

[PATCH v7 03/12] vfs: lookup_open(): move setting FMODE_CREATED down

[Jori Koolstra]

In preparation for using vfs_create_no_perm() in lookup_open() we need
to move setting FMODE_CREATED on the file mode to either before or after
that call, as currently it is in the middle. If try_break_deleg() fails
it is currently not set, but vfs_create_no_perm() includes a
try_break_deleg().

Going up the call chain of lookup_open() we see that it is only used in
open_last_lookups() if no error is returned from lookup_open(), so we
can safely move it to after the filesystem create() call. This also
makes more sense when reading the code as you don't have to wonder what
the implications are of setting FMODE_CREATED before the create() call.

Reviewed-by: NeilBrown <neil@brown.name>
Signed-off-by: Jori Koolstra <jkoolstra@xs4all.nl>
---
 fs/namei.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/fs/namei.c b/fs/namei.c
index ab09dd7f7475..a979488c2167 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -4609,7 +4609,6 @@ static struct dentry *lookup_open(struct nameidata *nd, struct file *file,
 	if (error)
 		goto out_dput;
 
-	file->f_mode |= FMODE_CREATED;
 	if (!dir_inode->i_op->create) {
 		error = -EACCES;
 		goto out_dput;
@@ -4618,6 +4617,8 @@ static struct dentry *lookup_open(struct nameidata *nd, struct file *file,
 	error = dir_inode->i_op->create(idmap, dir_inode, dentry, mode);
 	if (error)
 		goto out_dput;
+
+	file->f_mode |= FMODE_CREATED;
 out:
 	if (!IS_ERR(dentry)) {
 		if (file->f_mode & FMODE_CREATED)
-- 
2.56.0

[PATCH v7 04/12] vfs: move ->create check in lookup_open() to before try_break_deleg()

[Jori Koolstra]

The i_op->create check in lookup_open() takes place after the
try_break_deleg() call. This does not match the order when doing a
regular file create via mknod(2). There the call order is:

	filename_mknodat()
	vfs_create()
	i_op->create check
	try_break_deleg()

Move the i_op->create check to before try_break_deleg() in
lookup_open().

Reviewed-by: NeilBrown <neil@brown.name>
Signed-off-by: Jori Koolstra <jkoolstra@xs4all.nl>
---
 fs/namei.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/fs/namei.c b/fs/namei.c
index a979488c2167..000d8841a8ec 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -4605,15 +4605,15 @@ static struct dentry *lookup_open(struct nameidata *nd, struct file *file,
 		goto out_dput;
 	}
 
-	error = try_break_deleg(dir_inode, LEASE_BREAK_DIR_CREATE, &delegated_inode);
-	if (error)
-		goto out_dput;
-
 	if (!dir_inode->i_op->create) {
 		error = -EACCES;
 		goto out_dput;
 	}
 
+	error = try_break_deleg(dir_inode, LEASE_BREAK_DIR_CREATE, &delegated_inode);
+	if (error)
+		goto out_dput;
+
 	error = dir_inode->i_op->create(idmap, dir_inode, dentry, mode);
 	if (error)
 		goto out_dput;
-- 
2.56.0

[PATCH v7 05/12] vfs: lookup_open(): use vfs_create_no_perm()

[Jori Koolstra]

We can replace the code in the no create_error/negative dentry found
from lookup case in lookup_open() with the vfs_create_no_perm() helper.

Reviewed-by: NeilBrown <neil@brown.name>
Signed-off-by: Jori Koolstra <jkoolstra@xs4all.nl>
---
 fs/namei.c | 25 +++++++------------------
 1 file changed, 7 insertions(+), 18 deletions(-)

diff --git a/fs/namei.c b/fs/namei.c
index 000d8841a8ec..b864aa426102 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -4459,8 +4459,14 @@ static struct dentry *atomic_open(const struct path *path, struct dentry *dentry
 		}
 		dput(dentry);
 		dentry = ERR_PTR(error);
+	} else {
+		if (file->f_mode & FMODE_CREATED)
+			fsnotify_create(dir_inode, dentry);
+		if (file->f_mode & FMODE_OPENED)
+			fsnotify_open(file);
 	}
 
+
 	return dentry;
 }
 
@@ -4610,22 +4616,12 @@ static struct dentry *lookup_open(struct nameidata *nd, struct file *file,
 		goto out_dput;
 	}
 
-	error = try_break_deleg(dir_inode, LEASE_BREAK_DIR_CREATE, &delegated_inode);
-	if (error)
-		goto out_dput;
-
-	error = dir_inode->i_op->create(idmap, dir_inode, dentry, mode);
+	error = vfs_create_no_perm(idmap, dentry, mode, &delegated_inode);
 	if (error)
 		goto out_dput;
 
 	file->f_mode |= FMODE_CREATED;
 out:
-	if (!IS_ERR(dentry)) {
-		if (file->f_mode & FMODE_CREATED)
-			fsnotify_create(dir_inode, dentry);
-		if (file->f_mode & FMODE_OPENED)
-			fsnotify_open(file);
-	}
 	if ((open_flag & O_CREAT) || create_error)
 		inode_unlock(dir_inode);
 	else
@@ -5245,13 +5241,6 @@ struct file *dentry_create(struct path *path, int flags, umode_t mode,
 			/* Drop the extra reference */
 			dput(orig_dentry);
 
-		if (!error) {
-			if (file->f_mode & FMODE_CREATED)
-				fsnotify_create(dir->d_inode, dentry);
-			if (file->f_mode & FMODE_OPENED)
-				fsnotify_open(file);
-		}
-
 		path->dentry = dentry;
 
 	} else {
-- 
2.56.0

[PATCH v7 06/12] vfs: lookup_open(): lock the parent as I_MUTEX_PARENT

[Jori Koolstra]

lookup_open() takes the parent directory's i_rwsem with inode_lock(),
that is, in the I_MUTEX_NORMAL class, before creating the last
component. Every other VFS path that locks a directory in order to
create something in it goes through start_dirop() and friends, which
use inode_lock_nested(dir, I_MUTEX_PARENT).

Lock the parent as I_MUTEX_PARENT here too, so that lookup_open()
matches the rest of the VFS before it starts calling ->mkdir(), which
until now was only ever reached with the parent held that way.

No functional change intended.

Signed-off-by: Jori Koolstra <jkoolstra@xs4all.nl>
---
 fs/namei.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/namei.c b/fs/namei.c
index b864aa426102..0cae8b4c071d 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -4511,7 +4511,7 @@ static struct dentry *lookup_open(struct nameidata *nd, struct file *file,
 		 */
 	}
 	if (open_flag & O_CREAT)
-		inode_lock(dir_inode);
+		inode_lock_nested(dir_inode, I_MUTEX_PARENT);
 	else
 		inode_lock_shared(dir_inode);
 
-- 
2.56.0

[PATCH v7 07/12] kernfs: hold kernfs_iattr_rwsem when initializing a new inode

[Jori Koolstra]

kernfs_refresh_inode() copies the uid, gid, timestamps and nlink of a
kernfs_node into its VFS inode. Since commit 9caf69614225 ("kernfs:
Introduce separate rwsem to protect inode attributes.") these are
protected by kernfs_iattr_rwsem.

kernfs_init_inode() calls kernfs_refresh_inode(), but was not converted.
Its callers in turn hold kernfs_rwsem (kernfs_iop_lookup(),
kernfs_fill_super()) or no lock at all (__kernfs_fh_to_dentry()),
neither of which excludes a concurrent
kernfs_setattr()/kernfs_iop_setattr().

Take kernfs_iattr_rwsem for reading around the refresh in
kernfs_init_inode(). This nests inside kernfs_rwsem, which matches
kernfs_link_sibling() and kernfs_unlink_sibling().

Also, kernfs_get_inode() then does not need to hold kernfs_rwsem, so
drop it from kernfs_fill_super().

Fixes: 9caf69614225 ("kernfs: Introduce separate rwsem to protect inode attributes.")
Signed-off-by: Jori Koolstra <jkoolstra@xs4all.nl>
---
 fs/kernfs/inode.c | 3 ++-
 fs/kernfs/mount.c | 3 ---
 2 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/fs/kernfs/inode.c b/fs/kernfs/inode.c
index abb286bc3474..83f2007498db 100644
--- a/fs/kernfs/inode.c
+++ b/fs/kernfs/inode.c
@@ -204,7 +204,8 @@ static void kernfs_init_inode(struct kernfs_node *kn, struct inode *inode)
 	inode->i_generation = kernfs_gen(kn);
 
 	set_default_inode_attr(inode, kn->mode);
-	kernfs_refresh_inode(kn, inode);
+	scoped_guard(rwsem_read, &kernfs_root(kn)->kernfs_iattr_rwsem)
+		kernfs_refresh_inode(kn, inode);
 
 	/* initialize inode according to type */
 	switch (kernfs_type(kn)) {
diff --git a/fs/kernfs/mount.c b/fs/kernfs/mount.c
index a57399021c8b..202a5ca2bd34 100644
--- a/fs/kernfs/mount.c
+++ b/fs/kernfs/mount.c
@@ -283,7 +283,6 @@ struct dentry *kernfs_node_dentry(struct kernfs_node *kn,
 static int kernfs_fill_super(struct super_block *sb, struct kernfs_fs_context *kfc)
 {
 	struct kernfs_super_info *info = kernfs_info(sb);
-	struct kernfs_root *kf_root = kfc->root;
 	struct inode *inode;
 	struct dentry *root;
 
@@ -304,9 +303,7 @@ static int kernfs_fill_super(struct super_block *sb, struct kernfs_fs_context *k
 	sb->s_shrink->seeks = 0;
 
 	/* get root inode, initialize and unlock it */
-	down_read(&kf_root->kernfs_rwsem);
 	inode = kernfs_get_inode(sb, info->root->kn);
-	up_read(&kf_root->kernfs_rwsem);
 	if (!inode) {
 		pr_debug("kernfs: could not get root inode\n");
 		return -ENOMEM;
-- 
2.56.0

[PATCH v7 08/12] 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 a proper API is only partially possible:
the caller can immediately fstat() what was opened to verify that it
has the expected inode type, owner and mode. But besides being easy to
get wrong, this cannot establish who created the directory: a directory
created by another process with identical credentials is
indistinguishable from one the caller created itself, so the caller
cannot tell whether the directory is its own to manage.

Historically, the O_CREAT|O_DIRECTORY behaviour was to return ENOTDIR if
a regular file exists at the open path; EISDIR if a directory exists at
the path; and to create a regular file if no file exists at the path.
This behaviour changed accidentally with
commit 973d4b73fbaf ("do_last(): rejoin the common path even earlier in
FMODE_{OPENED,CREATED} case") causing ENOTDIR to return in the last case
while still creating the file. As this change was not detected for a
long time, Brauner proposed to adopt the more consistent NetBSD
behaviour, i.e. to return EINVAL on the O_CREAT|O_DIRECTORY combination.
This change was applied in commit 43b450632676 ("open: return EINVAL for
O_DIRECTORY | O_CREAT") in March, 2023. As the EINVAL behaviour has been
in the kernel for about 3 years now, no rollback is expected as a result
of userspace reliance on old behaviour, leaving us free to reassign the
O_CREAT|O_DIRECTORY semantics.

O_CREAT|O_DIRECTORY must be combined with O_EXCL; otherwise open fails
with EINVAL. As with O_CREAT|O_EXCL, a trailing symlink is not followed,
and any existing entry at the path results in EEXIST.

Filesystems that do not support O_CREAT|O_DIRECTORY set
SB_I_NO_ATOMIC_MKDIR, and lookup_open() refuses the open with EOPNOTSUPP
for them before doing any lookup. These are all ->atomic_open()
filesystems, which cannot create directories without protocol
extensions, and tracefs, whose ->mkdir() drops the parent's i_rwsem so
that the directory at the name may not be the one created by the call
(or at least it has to be verified that this plays well with the
atomicity guarantee that O_CREAT|O_DIRECTORY attempts to provide.) For
ecryptfs and apparmorfs there are similar concerns and hence they are
also blocked for now.

kernfs ->mkdir() did not instantiate the dentry; this is fixed by
returning the new kernfs_node from the kernfs_syscall_ops ->mkdir()
callbacks and splicing its inode.

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_super.c               |   1 +
 fs/ceph/super.c                 |   1 +
 fs/ecryptfs/main.c              |   1 +
 fs/fuse/inode.c                 |   1 +
 fs/gfs2/ops_fstype.c            |   1 +
 fs/kernfs/dir.c                 |  30 ++++++++--
 fs/kernfs/inode.c               |  34 +++++++++++
 fs/kernfs/kernfs-internal.h     |   2 +
 fs/namei.c                      | 101 +++++++++++++++++++++++++-------
 fs/nfs/super.c                  |   1 +
 fs/open.c                       |  33 +++++++----
 fs/resctrl/rdtgroup.c           |  35 ++++++-----
 fs/smb/client/cifsfs.c          |   1 +
 fs/tracefs/inode.c              |   1 +
 fs/vboxsf/super.c               |   1 +
 include/linux/fcntl.h           |   6 ++
 include/linux/fs/super_types.h  |   1 +
 include/linux/kernfs.h          |   7 ++-
 kernel/cgroup/cgroup-internal.h |   3 +-
 kernel/cgroup/cgroup.c          |  16 +++--
 security/apparmor/apparmorfs.c  |   1 +
 21 files changed, 218 insertions(+), 60 deletions(-)

diff --git a/fs/9p/vfs_super.c b/fs/9p/vfs_super.c
index 94d6b02c221b..a3377bc34ac0 100644
--- a/fs/9p/vfs_super.c
+++ b/fs/9p/vfs_super.c
@@ -40,6 +40,7 @@ static int v9fs_fill_super(struct super_block *sb)
 	sb->s_blocksize_bits = fls(v9ses->maxdata - 1);
 	sb->s_blocksize = 1 << sb->s_blocksize_bits;
 	sb->s_magic = V9FS_MAGIC;
+	sb->s_iflags |= SB_I_NO_ATOMIC_MKDIR;
 	if (v9fs_proto_dotl(v9ses)) {
 		sb->s_op = &v9fs_super_ops_dotl;
 		if (!(v9ses->flags & V9FS_NO_XATTR))
diff --git a/fs/ceph/super.c b/fs/ceph/super.c
index 72935f665f11..fa1cf1f4f57d 100644
--- a/fs/ceph/super.c
+++ b/fs/ceph/super.c
@@ -1226,6 +1226,7 @@ static int ceph_set_super(struct super_block *s, struct fs_context *fc)
 	fsc->max_file_size = 1ULL << 40; /* temp value until we get mdsmap */
 
 	s->s_op = &ceph_super_ops;
+	s->s_iflags |= SB_I_NO_ATOMIC_MKDIR;
 	set_default_d_op(s, &ceph_dentry_ops);
 	s->s_export_op = &ceph_export_ops;
 
diff --git a/fs/ecryptfs/main.c b/fs/ecryptfs/main.c
index 5f37cddb956f..8ed6076d50af 100644
--- a/fs/ecryptfs/main.c
+++ b/fs/ecryptfs/main.c
@@ -477,6 +477,7 @@ static int ecryptfs_get_tree(struct fs_context *fc)
 	/* ->kill_sb() will take care of sbi after that point */
 	sbi = NULL;
 	s->s_op = &ecryptfs_sops;
+	s->s_iflags |= SB_I_NO_ATOMIC_MKDIR;
 	s->s_xattr = ecryptfs_xattr_handlers;
 	set_default_d_op(s, &ecryptfs_dops);
 
diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c
index e9552be3637b..cf766c6feea4 100644
--- a/fs/fuse/inode.c
+++ b/fs/fuse/inode.c
@@ -1608,6 +1608,7 @@ static void fuse_sb_defaults(struct super_block *sb)
 	sb->s_iflags |= SB_I_IMA_UNVERIFIABLE_SIGNATURE;
 	sb->s_iflags |= SB_I_NOIDMAP;
 	sb->s_iflags |= SB_I_NO_DATA_INTEGRITY;
+	sb->s_iflags |= SB_I_NO_ATOMIC_MKDIR;
 	if (sb->s_user_ns != &init_user_ns)
 		sb->s_iflags |= SB_I_UNTRUSTED_MOUNTER;
 	sb->s_flags &= ~(SB_NOSEC | SB_I_VERSION);
diff --git a/fs/gfs2/ops_fstype.c b/fs/gfs2/ops_fstype.c
index 718e0da7dfce..695835819a5d 100644
--- a/fs/gfs2/ops_fstype.c
+++ b/fs/gfs2/ops_fstype.c
@@ -1139,6 +1139,7 @@ static int gfs2_fill_super(struct super_block *sb, struct fs_context *fc)
 	sb->s_flags |= SB_NOSEC;
 	sb->s_magic = GFS2_MAGIC;
 	sb->s_op = &gfs2_super_ops;
+	sb->s_iflags |= SB_I_NO_ATOMIC_MKDIR;
 
 	set_default_d_op(sb, &gfs2_dops);
 	sb->s_export_op = &gfs2_export_ops;
diff --git a/fs/kernfs/dir.c b/fs/kernfs/dir.c
index 82bbaeb326aa..42b3810a04be 100644
--- a/fs/kernfs/dir.c
+++ b/fs/kernfs/dir.c
@@ -1294,18 +1294,40 @@ static struct dentry *kernfs_iop_mkdir(struct mnt_idmap *idmap,
 {
 	struct kernfs_node *parent = dir->i_private;
 	struct kernfs_syscall_ops *scops = kernfs_root(parent)->syscall_ops;
-	int ret;
+	struct kernfs_node *kn;
+	struct inode *inode;
 
 	if (!scops || !scops->mkdir)
 		return ERR_PTR(-EPERM);
 
-	if (!kernfs_get_active(parent))
+	inode = new_inode(dir->i_sb);
+	if (!inode)
+		return ERR_PTR(-ENOMEM);
+
+	if (!kernfs_get_active(parent)) {
+		iput(inode);
 		return ERR_PTR(-ENODEV);
+	}
 
-	ret = scops->mkdir(parent, dentry->d_name.name, mode);
+	kn = scops->mkdir(parent, dentry->d_name.name, mode);
 
 	kernfs_put_active(parent);
-	return ERR_PTR(ret);
+	if (IS_ERR(kn)) {
+		iput(inode);
+		return ERR_CAST(kn);
+	}
+
+	inode = kernfs_get_inode_prealloc(inode, kn);
+	kernfs_put(kn);
+
+	/*
+	 * We may race with kernfs_fh_to_dentry() which calls d_obtain_alias(),
+	 * so the inode may already have a disconnected alias. Use
+	 * d_splice_alias() to move it into place, which does not accept
+	 * hashed dentries, so drop the negative dentry first.
+	 */
+	d_drop(dentry);
+	return d_splice_alias(inode, dentry);
 }
 
 static int kernfs_iop_rmdir(struct inode *dir, struct dentry *dentry)
diff --git a/fs/kernfs/inode.c b/fs/kernfs/inode.c
index 83f2007498db..a25409036492 100644
--- a/fs/kernfs/inode.c
+++ b/fs/kernfs/inode.c
@@ -255,6 +255,40 @@ struct inode *kernfs_get_inode(struct super_block *sb, struct kernfs_node *kn)
 	return inode;
 }
 
+/**
+ *	kernfs_get_inode_prealloc - get inode for kernfs_node
+ *	@prealloc_inode: inode allocated with new_inode() on the target superblock
+ *	@kn: kernfs_node to get the inode for
+ *
+ *	Like kernfs_get_inode(), but cannot fail. @prealloc_inode is
+ *	initialized for @kn, unless @kn already has an inode on the super
+ *	block, in which case @prealloc_inode is released and the existing inode
+ *	is returned instead.
+ *
+ *	Locking:
+ *	Kernel thread context (may sleep).
+ *
+ *	Return:
+ *	Pointer to the inode of @kn.
+ */
+struct inode *kernfs_get_inode_prealloc(struct inode *prealloc_inode,
+					struct kernfs_node *kn)
+{
+	struct inode *old;
+
+	prealloc_inode->i_ino = kernfs_ino(kn);
+	while (insert_inode_locked(prealloc_inode)) {
+		old = ilookup(prealloc_inode->i_sb, prealloc_inode->i_ino);
+		if (old) {
+			iput(prealloc_inode);
+			return old;
+		}
+	}
+
+	kernfs_init_inode(kn, prealloc_inode);
+	return prealloc_inode;
+}
+
 /*
  * The kernfs_node serves as both an inode and a directory entry for
  * kernfs.  To prevent the kernfs inode numbers from being freed
diff --git a/fs/kernfs/kernfs-internal.h b/fs/kernfs/kernfs-internal.h
index aa784b540b36..e4273f104a24 100644
--- a/fs/kernfs/kernfs-internal.h
+++ b/fs/kernfs/kernfs-internal.h
@@ -170,6 +170,8 @@ extern struct kmem_cache *kernfs_node_cache, *kernfs_iattrs_cache;
  * inode.c
  */
 extern const struct xattr_handler * const kernfs_xattr_handlers[];
+struct inode *kernfs_get_inode_prealloc(struct inode *inode,
+					struct kernfs_node *kn);
 void kernfs_evict_inode(struct inode *inode);
 int kernfs_iop_permission(struct mnt_idmap *idmap,
 			  struct inode *inode, int mask);
diff --git a/fs/namei.c b/fs/namei.c
index 0cae8b4c071d..c16150965041 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -4363,21 +4363,43 @@ static inline int open_to_namei_flags(int flag)
 
 static int may_o_create(struct mnt_idmap *idmap,
 			const struct path *dir, struct dentry *dentry,
-			umode_t mode)
+			int open_flag, umode_t mode)
 {
-	int error = security_path_mknod(dir, dentry, mode, 0);
+	struct inode *dir_inode = dir->dentry->d_inode;
+	bool create_dir = O_IS_MKDIR(open_flag);
+	int error;
+
+	WARN_ON_ONCE(create_dir && !S_ISDIR(mode));
+
+	if (create_dir)
+		error = security_path_mkdir(dir, dentry, mode);
+	else
+		error = security_path_mknod(dir, dentry, mode, 0);
 	if (error)
 		return error;
 
 	if (!fsuidgid_has_mapping(dir->dentry->d_sb, idmap))
 		return -EOVERFLOW;
 
-	error = inode_permission(idmap, dir->dentry->d_inode,
-				 MAY_WRITE | MAY_EXEC);
+	error = inode_permission(idmap, dir_inode, MAY_WRITE | MAY_EXEC);
 	if (error)
 		return error;
 
-	return security_inode_create(dir->dentry->d_inode, dentry, mode);
+	if (create_dir)
+		error = security_inode_mkdir(dir_inode, dentry, mode);
+	else
+		error = security_inode_create(dir_inode, dentry, mode);
+
+	return error;
+}
+
+static inline umode_t o_create_mode(struct mnt_idmap *idmap,
+		const struct inode *dir, int open_flag, umode_t mode)
+{
+	if (O_IS_MKDIR(open_flag))
+		return vfs_prepare_mode(idmap, dir, mode, S_IRWXUGO | S_ISVTX, S_IFDIR);
+	else
+		return vfs_prepare_mode(idmap, dir, mode, S_IALLUGO, S_IFREG);
 }
 
 /**
@@ -4413,8 +4435,10 @@ static struct dentry *atomic_open(const struct path *path, struct dentry *dentry
 
 	file->__f_path.dentry = DENTRY_NOT_SET;
 	file->__f_path.mnt = path->mnt;
+
 	error = dir_inode->i_op->atomic_open(dir_inode, dentry, file,
-				       open_to_namei_flags(open_flag), mode);
+					     open_to_namei_flags(open_flag), mode);
+
 	d_lookup_done(dentry);
 
 	if (!error) {
@@ -4458,18 +4482,24 @@ static struct dentry *atomic_open(const struct path *path, struct dentry *dentry
 			error = create_error;
 		}
 		dput(dentry);
-		dentry = ERR_PTR(error);
-	} else {
-		if (file->f_mode & FMODE_CREATED)
-			fsnotify_create(dir_inode, dentry);
-		if (file->f_mode & FMODE_OPENED)
-			fsnotify_open(file);
+		return ERR_PTR(error);
 	}
 
+	if (file->f_mode & FMODE_CREATED) {
+		if (d_is_dir(dentry))
+			fsnotify_mkdir(dir_inode, dentry);
+		else
+			fsnotify_create(dir_inode, dentry);
+	}
+	if (file->f_mode & FMODE_OPENED)
+		fsnotify_open(file);
 
 	return dentry;
 }
 
+static inline
+struct dentry *vfs_mkdir_no_perm(struct mnt_idmap *, struct inode *, struct dentry *,
+				 umode_t, struct delegated_inode *);
 /*
  * Look up and maybe create and open the last component.
  *
@@ -4491,6 +4521,7 @@ static struct dentry *lookup_open(struct nameidata *nd, struct file *file,
 	struct mnt_idmap *idmap;
 	struct dentry *dir = nd->path.dentry;
 	struct inode *dir_inode = dir->d_inode;
+	bool create_dir = O_IS_MKDIR(op->open_flag);
 	int open_flag;
 	struct dentry *dentry;
 	int error, create_error;
@@ -4503,6 +4534,9 @@ static struct dentry *lookup_open(struct nameidata *nd, struct file *file,
 	mode = op->mode;
 	create_error = 0;
 
+	if (create_dir && (dir_inode->i_sb->s_iflags & SB_I_NO_ATOMIC_MKDIR))
+		return ERR_PTR(-EOPNOTSUPP);
+
 	if (open_flag & (O_CREAT | O_TRUNC | O_WRONLY | O_RDWR)) {
 		got_write = !mnt_want_write(nd->path.mnt);
 		/*
@@ -4563,10 +4597,10 @@ static struct dentry *lookup_open(struct nameidata *nd, struct file *file,
 	if (open_flag & O_CREAT) {
 		if (open_flag & O_EXCL)
 			open_flag &= ~O_TRUNC;
-		mode = vfs_prepare_mode(idmap, dir_inode, mode, mode, mode);
+		mode = o_create_mode(idmap, dir_inode, open_flag, mode);
 		if (likely(got_write))
 			create_error = may_o_create(idmap, &nd->path,
-						    dentry, mode);
+						    dentry, open_flag, mode);
 		else
 			create_error = -EROFS;
 	}
@@ -4611,12 +4645,32 @@ static struct dentry *lookup_open(struct nameidata *nd, struct file *file,
 		goto out_dput;
 	}
 
-	if (!dir_inode->i_op->create) {
+	/* mimic operation missing errnos of vfs_mkdir/vfs_create */
+	if (create_dir && !dir_inode->i_op->mkdir) {
+		error = -EPERM;
+		goto out_dput;
+	}
+	if (!create_dir && !dir_inode->i_op->create) {
 		error = -EACCES;
 		goto out_dput;
 	}
 
-	error = vfs_create_no_perm(idmap, dentry, mode, &delegated_inode);
+	if (create_dir) {
+		struct dentry *res = vfs_mkdir_no_perm(idmap, dir_inode, dentry, mode,
+						       &delegated_inode);
+		error = PTR_ERR_OR_ZERO(res);
+		if (!error) {
+			dentry = res;
+			/*
+			 * Filesystems whose ->mkdir() may leave the dentry
+			 * negative must set SB_I_NO_ATOMIC_MKDIR.
+			 */
+			if (WARN_ON_ONCE(d_is_negative(dentry)))
+				error = -EIO;
+		}
+	} else {
+		error = vfs_create_no_perm(idmap, dentry, mode, &delegated_inode);
+	}
 	if (error)
 		goto out_dput;
 
@@ -4748,7 +4802,7 @@ 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->last))
+		if (trailing_slashes(&nd->last) && !(open_flag & O_DIRECTORY))
 			return ERR_PTR(-EISDIR);
 
 		/* Don't bother on an O_EXCL create */
@@ -4786,6 +4840,13 @@ static const char *open_last_lookups(struct nameidata *nd,
 	if (nd->last_type != LAST_NORM) {
 		if (nd->depth)
 			put_link(nd);
+		/*
+		 * Return -EOPNOTSUPP consistently, not -EEXIST, for paths
+		 * ending in . or .., as lookup_open() does for other names.
+		 */
+		if (O_IS_MKDIR(open_flag) &&
+		    (nd->path.dentry->d_sb->s_iflags & SB_I_NO_ATOMIC_MKDIR))
+			return ERR_PTR(-EOPNOTSUPP);
 		return handle_dots(nd, nd->last_type);
 	}
 
@@ -4849,7 +4910,7 @@ 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))
+		if (!(open_flag & O_DIRECTORY) && d_is_dir(nd->path.dentry))
 			return -EISDIR;
 		error = may_create_in_sticky(idmap, nd,
 					     d_backing_inode(nd->path.dentry));
@@ -5188,7 +5249,7 @@ inline struct dentry *start_creating_user_path(
 EXPORT_SYMBOL(start_creating_user_path);
 
 /**
- * dentry_create - Create and open a file
+ * dentry_create - Create and open a regular file
  * @path: path to create
  * @flags: O\_ flags
  * @mode: mode bits for new file
@@ -5225,7 +5286,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, flags, mode);
 		if (create_error)
 			flags &= ~O_CREAT;
 
diff --git a/fs/nfs/super.c b/fs/nfs/super.c
index cb19f1540d98..f4452b630f52 100644
--- a/fs/nfs/super.c
+++ b/fs/nfs/super.c
@@ -1077,6 +1077,7 @@ static void nfs_fill_super(struct super_block *sb, struct nfs_fs_context *ctx)
 	sb->s_blocksize = 0;
 	sb->s_xattr = server->nfs_client->cl_nfs_mod->xattr;
 	sb->s_op = server->nfs_client->cl_nfs_mod->sops;
+	sb->s_iflags |= SB_I_NO_ATOMIC_MKDIR;
 	if (server->bsize)
 		sb->s_blocksize =
 			nfs_block_size(server->bsize, &sb->s_blocksize_bits);
diff --git a/fs/open.c b/fs/open.c
index 6b1c14e684a9..4177f7226537 100644
--- a/fs/open.c
+++ b/fs/open.c
@@ -1239,29 +1239,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 (O_IS_MKDIR(flags))
+			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;
@@ -1319,11 +1320,19 @@ 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) {
-		op->intent |= LOOKUP_CREATE;
+		if (flags & O_DIRECTORY) {
+			/*
+			 * O_CREAT|O_DIRECTORY is provided to prevent TOCTOUs, so
+			 * it makes little sense to allow it without O_EXCL.
+			 */
+			if (!(flags & O_EXCL))
+				return -EINVAL;
+		}
 		if (flags & O_EXCL) {
 			op->intent |= LOOKUP_EXCL;
 			flags |= O_NOFOLLOW;
 		}
+		op->intent |= LOOKUP_CREATE;
 	}
 
 	if (flags & O_DIRECTORY)
diff --git a/fs/resctrl/rdtgroup.c b/fs/resctrl/rdtgroup.c
index 5dcbb0a964e8..d7b13b63c6ad 100644
--- a/fs/resctrl/rdtgroup.c
+++ b/fs/resctrl/rdtgroup.c
@@ -4042,15 +4042,16 @@ static void mkdir_rdt_prepare_clean(struct rdtgroup *rgrp)
  * and monitor group(ctrl_mon). This is a resource group
  * to monitor a subset of tasks and cpus in its parent ctrl_mon group.
  */
-static int rdtgroup_mkdir_mon(struct kernfs_node *parent_kn,
-			      const char *name, umode_t mode)
+static struct kernfs_node *rdtgroup_mkdir_mon(struct kernfs_node *parent_kn,
+					      const char *name, umode_t mode)
 {
 	struct rdtgroup *rdtgrp, *prgrp;
+	struct kernfs_node *kn;
 	int ret;
 
 	ret = mkdir_rdt_prepare(parent_kn, name, mode, RDTMON_GROUP, &rdtgrp);
 	if (ret)
-		return ret;
+		return ERR_PTR(ret);
 
 	prgrp = rdtgrp->mon.parent;
 	rdtgrp->closid = prgrp->closid;
@@ -4069,17 +4070,22 @@ static int rdtgroup_mkdir_mon(struct kernfs_node *parent_kn,
 	 */
 	list_add_tail(&rdtgrp->mon.crdtgrp_list, &prgrp->mon.crdtgrp_list);
 
+	kn = rdtgrp->kn;
+	kernfs_get(kn);
+	rdtgroup_kn_unlock(parent_kn);
+	return kn;
+
 out_unlock:
 	rdtgroup_kn_unlock(parent_kn);
-	return ret;
+	return ERR_PTR(ret);
 }
 
 /*
  * These are rdtgroups created under the root directory. Can be used
  * to allocate and monitor resources.
  */
-static int rdtgroup_mkdir_ctrl_mon(struct kernfs_node *parent_kn,
-				   const char *name, umode_t mode)
+static struct kernfs_node *rdtgroup_mkdir_ctrl_mon(struct kernfs_node *parent_kn,
+						   const char *name, umode_t mode)
 {
 	struct rdtgroup *rdtgrp;
 	struct kernfs_node *kn;
@@ -4088,7 +4094,7 @@ static int rdtgroup_mkdir_ctrl_mon(struct kernfs_node *parent_kn,
 
 	ret = mkdir_rdt_prepare(parent_kn, name, mode, RDTCTRL_GROUP, &rdtgrp);
 	if (ret)
-		return ret;
+		return ERR_PTR(ret);
 
 	kn = rdtgrp->kn;
 	ret = closid_alloc();
@@ -4127,7 +4133,9 @@ static int rdtgroup_mkdir_ctrl_mon(struct kernfs_node *parent_kn,
 			rdtgrp->mba_mbps_event = mba_mbps_default_event;
 	}
 
-	goto out_unlock;
+	kernfs_get(kn);
+	rdtgroup_kn_unlock(parent_kn);
+	return kn;
 
 out_del_list:
 	list_del(&rdtgrp->rdtgroup_list);
@@ -4137,17 +4145,16 @@ static int rdtgroup_mkdir_ctrl_mon(struct kernfs_node *parent_kn,
 	closid_free(closid);
 out_common_fail:
 	mkdir_rdt_prepare_clean(rdtgrp);
-out_unlock:
 	rdtgroup_kn_unlock(parent_kn);
-	return ret;
+	return ERR_PTR(ret);
 }
 
-static int rdtgroup_mkdir(struct kernfs_node *parent_kn, const char *name,
-			  umode_t mode)
+static struct kernfs_node *rdtgroup_mkdir(struct kernfs_node *parent_kn,
+					  const char *name, umode_t mode)
 {
 	/* Do not accept '\n' to avoid unparsable situation. */
 	if (strchr(name, '\n'))
-		return -EINVAL;
+		return ERR_PTR(-EINVAL);
 
 	/*
 	 * If the parent directory is the root directory and RDT
@@ -4161,7 +4168,7 @@ static int rdtgroup_mkdir(struct kernfs_node *parent_kn, const char *name,
 	if (resctrl_arch_mon_capable())
 		return rdtgroup_mkdir_mon(parent_kn, name, mode);
 
-	return -EPERM;
+	return ERR_PTR(-EPERM);
 }
 
 static int rdtgroup_rmdir_mon(struct rdtgroup *rdtgrp, cpumask_var_t tmpmask)
diff --git a/fs/smb/client/cifsfs.c b/fs/smb/client/cifsfs.c
index 98b610b2a114..93a5ead81c82 100644
--- a/fs/smb/client/cifsfs.c
+++ b/fs/smb/client/cifsfs.c
@@ -247,6 +247,7 @@ cifs_read_super(struct super_block *sb)
 
 	sb->s_magic = CIFS_SUPER_MAGIC;
 	sb->s_op = &cifs_super_ops;
+	sb->s_iflags |= SB_I_NO_ATOMIC_MKDIR;
 	sb->s_xattr = cifs_xattr_handlers;
 	rc = super_setup_bdi(sb);
 	if (rc)
diff --git a/fs/tracefs/inode.c b/fs/tracefs/inode.c
index f3d6188a3b7b..0f64310c3095 100644
--- a/fs/tracefs/inode.c
+++ b/fs/tracefs/inode.c
@@ -471,6 +471,7 @@ static int tracefs_fill_super(struct super_block *sb, struct fs_context *fc)
 		return err;
 
 	sb->s_op = &tracefs_super_operations;
+	sb->s_iflags |= SB_I_NO_ATOMIC_MKDIR;
 	tracefs_apply_options(sb, false);
 	set_default_d_op(sb, &tracefs_dentry_operations);
 
diff --git a/fs/vboxsf/super.c b/fs/vboxsf/super.c
index a61fbab51d37..190b76f04d4f 100644
--- a/fs/vboxsf/super.c
+++ b/fs/vboxsf/super.c
@@ -196,6 +196,7 @@ static int vboxsf_fill_super(struct super_block *sb, struct fs_context *fc)
 	sb->s_blocksize = 1024;
 	sb->s_maxbytes = MAX_LFS_FILESIZE;
 	sb->s_op = &vboxsf_super_ops;
+	sb->s_iflags |= SB_I_NO_ATOMIC_MKDIR;
 	set_default_d_op(sb, &vboxsf_dentry_ops);
 
 	iroot = iget_locked(sb, 0);
diff --git a/include/linux/fcntl.h b/include/linux/fcntl.h
index 6ad6b9e7a226..204e16bbe263 100644
--- a/include/linux/fcntl.h
+++ b/include/linux/fcntl.h
@@ -30,6 +30,12 @@
  */
 #define __O_REGULAR		(1 << 30)
 
+#define O_MKDIR_MASK	(O_CREAT | O_DIRECTORY)
+static inline bool O_IS_MKDIR(unsigned int flags)
+{
+	return (flags & O_MKDIR_MASK) == O_MKDIR_MASK;
+}
+
 /* List of all valid flags for the how->resolve argument: */
 #define VALID_RESOLVE_FLAGS \
 	(RESOLVE_NO_XDEV | RESOLVE_NO_MAGICLINKS | RESOLVE_NO_SYMLINKS | \
diff --git a/include/linux/fs/super_types.h b/include/linux/fs/super_types.h
index ecd96aeb1cee..521b00c47d0a 100644
--- a/include/linux/fs/super_types.h
+++ b/include/linux/fs/super_types.h
@@ -352,5 +352,6 @@ struct super_block {
 #define SB_I_NOIDMAP	0x00002000	/* No idmapped mounts on this superblock */
 #define SB_I_ALLOW_HSM	0x00004000	/* Allow HSM events on this superblock */
 #define SB_I_NO_DATA_INTEGRITY	0x00008000 /* fs cannot guarantee data persistence on sync */
+#define SB_I_NO_ATOMIC_MKDIR	0x00010000 /* no O_CREAT|O_DIRECTORY on this fs */
 
 #endif /* _LINUX_FS_SUPER_TYPES_H */
diff --git a/include/linux/kernfs.h b/include/linux/kernfs.h
index 351a5101c862..0aced5004340 100644
--- a/include/linux/kernfs.h
+++ b/include/linux/kernfs.h
@@ -241,13 +241,14 @@ struct kernfs_node {
  * syscalls.  These optional callbacks are invoked on the matching syscalls
  * and can perform any kernfs operations which don't necessarily have to be
  * the exact operation requested.  An active reference is held for each
- * kernfs_node parameter.
+ * kernfs_node parameter.  @mkdir returns the kernfs_node of the new
+ * directory with a reference held, or an ERR_PTR() on failure.
  */
 struct kernfs_syscall_ops {
 	int (*show_options)(struct seq_file *sf, struct kernfs_root *root);
 
-	int (*mkdir)(struct kernfs_node *parent, const char *name,
-		     umode_t mode);
+	struct kernfs_node *(*mkdir)(struct kernfs_node *parent,
+				     const char *name, umode_t mode);
 	int (*rmdir)(struct kernfs_node *kn);
 	int (*rename)(struct kernfs_node *kn, struct kernfs_node *new_parent,
 		      const char *new_name);
diff --git a/kernel/cgroup/cgroup-internal.h b/kernel/cgroup/cgroup-internal.h
index 58797123b752..eca8d3088539 100644
--- a/kernel/cgroup/cgroup-internal.h
+++ b/kernel/cgroup/cgroup-internal.h
@@ -256,7 +256,8 @@ void cgroup_procs_write_finish(struct task_struct *task,
 
 void cgroup_lock_and_drain_offline(struct cgroup *cgrp);
 
-int cgroup_mkdir(struct kernfs_node *parent_kn, const char *name, umode_t mode);
+struct kernfs_node *cgroup_mkdir(struct kernfs_node *parent_kn, const char *name,
+				 umode_t mode);
 int cgroup_rmdir(struct kernfs_node *kn);
 int cgroup_show_path(struct seq_file *sf, struct kernfs_node *kf_node,
 		     struct kernfs_root *kf_root);
diff --git a/kernel/cgroup/cgroup.c b/kernel/cgroup/cgroup.c
index 227d09704ca5..b3793b5842ab 100644
--- a/kernel/cgroup/cgroup.c
+++ b/kernel/cgroup/cgroup.c
@@ -6113,18 +6113,20 @@ static bool cgroup_check_hierarchy_limits(struct cgroup *parent)
 	return ret;
 }
 
-int cgroup_mkdir(struct kernfs_node *parent_kn, const char *name, umode_t mode)
+struct kernfs_node *cgroup_mkdir(struct kernfs_node *parent_kn, const char *name,
+				 umode_t mode)
 {
 	struct cgroup *parent, *cgrp;
+	struct kernfs_node *kn;
 	int ret;
 
 	/* do not accept '\n' to prevent making /proc/<pid>/cgroup unparsable */
 	if (strchr(name, '\n'))
-		return -EINVAL;
+		return ERR_PTR(-EINVAL);
 
 	parent = cgroup_kn_lock_live(parent_kn, false);
 	if (!parent)
-		return -ENODEV;
+		return ERR_PTR(-ENODEV);
 
 	if (!cgroup_check_hierarchy_limits(parent)) {
 		ret = -EAGAIN;
@@ -6156,14 +6158,16 @@ int cgroup_mkdir(struct kernfs_node *parent_kn, const char *name, umode_t mode)
 	/* let's create and online css's */
 	kernfs_activate(cgrp->kn);
 
-	ret = 0;
-	goto out_unlock;
+	kn = cgrp->kn;
+	kernfs_get(kn);
+	cgroup_kn_unlock(parent_kn);
+	return kn;
 
 out_destroy:
 	cgroup_destroy_locked(cgrp);
 out_unlock:
 	cgroup_kn_unlock(parent_kn);
-	return ret;
+	return ERR_PTR(ret);
 }
 
 /*
diff --git a/security/apparmor/apparmorfs.c b/security/apparmor/apparmorfs.c
index 5b42140e12e8..8ae096cc1af6 100644
--- a/security/apparmor/apparmorfs.c
+++ b/security/apparmor/apparmorfs.c
@@ -248,6 +248,7 @@ static int apparmorfs_fill_super(struct super_block *sb, struct fs_context *fc)
 	if (error)
 		return error;
 	sb->s_op = &aafs_super_ops;
+	sb->s_iflags |= SB_I_NO_ATOMIC_MKDIR;
 
 	return 0;
 }
-- 
2.56.0

[PATCH v7 09/12] vfs: use finish_lookup() helper in __lookup_slow() and lookup_open()

[Jori Koolstra]

The same code around calling ->lookup() on an in-lookup dentry is
open-coded in both __lookup_slow() and lookup_open(). lookup_open() is
becoming quite unwieldy so factor this out.

No functional change intended.

Signed-off-by: Jori Koolstra <jkoolstra@xs4all.nl>
---
 fs/namei.c | 51 ++++++++++++++++++++++++++++-----------------------
 1 file changed, 28 insertions(+), 23 deletions(-)

diff --git a/fs/namei.c b/fs/namei.c
index c16150965041..e7d3d28f3fd7 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -1889,12 +1889,34 @@ static struct dentry *lookup_fast(struct nameidata *nd)
 	return dentry;
 }
 
+/*
+ * If @dentry is in-lookup, look it up in @dir through ->lookup().  Returns
+ * the dentry to use from now on: @dentry itself, or the dentry or ERR_PTR()
+ * that ->lookup() returned instead, in which case @dentry is dropped.
+ */
+static struct dentry *finish_lookup(struct inode *dir, struct dentry *dentry,
+				unsigned int flags)
+{
+	struct dentry *res;
+
+	if (!d_in_lookup(dentry))
+		return dentry;
+
+	res = dir->i_op->lookup(dir, dentry, flags);
+	d_lookup_done(dentry);
+	if (unlikely(res)) {
+		dput(dentry);
+		return res;
+	}
+	return dentry;
+}
+
 /* Fast lookup failed, do it the slow way */
 static struct dentry *__lookup_slow(const struct qstr *name,
 				    struct dentry *dir,
 				    unsigned int flags)
 {
-	struct dentry *dentry, *old;
+	struct dentry *dentry;
 	struct inode *inode = dir->d_inode;
 
 	/* Don't go there if it's already dead */
@@ -1913,17 +1935,10 @@ static struct dentry *__lookup_slow(const struct qstr *name,
 				goto again;
 			}
 			dput(dentry);
-			dentry = ERR_PTR(error);
-		}
-	} else {
-		old = inode->i_op->lookup(inode, dentry, flags);
-		d_lookup_done(dentry);
-		if (unlikely(old)) {
-			dput(dentry);
-			dentry = old;
+			return ERR_PTR(error);
 		}
 	}
-	return dentry;
+	return finish_lookup(inode, dentry, flags);
 }
 
 static noinline struct dentry *lookup_slow(const struct qstr *name,
@@ -4614,19 +4629,9 @@ static struct dentry *lookup_open(struct nameidata *nd, struct file *file,
 		goto out;
 	}
 
-	if (d_in_lookup(dentry)) {
-		struct dentry *res = dir_inode->i_op->lookup(dir_inode, dentry,
-							     nd->flags);
-		d_lookup_done(dentry);
-		if (unlikely(res)) {
-			if (IS_ERR(res)) {
-				error = PTR_ERR(res);
-				goto out_dput;
-			}
-			dput(dentry);
-			dentry = res;
-		}
-	}
+	dentry = finish_lookup(dir_inode, dentry, nd->flags);
+	if (IS_ERR(dentry))
+		goto out;
 
 	if (dentry->d_inode || !(op->open_flag & O_CREAT)) {
 		/*
-- 
2.56.0

[PATCH v7 10/12] vfs: short-circuit MAY_WRITE access for O_DIRECTORY opens

[Jori Koolstra]

Requesting write access on a directory can never succeed. Rather
than performing a path-walk to determine whether the target is
actually a directory (-EISDIR) or not (-ENOTDIR), or does not exist
(-ENOENT), etc., we short-circuit to -ENOTDIR.

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 via O_CREAT|O_DIRECTORY.

The advantage of short-circuiting is that we don't have to add even more
logic to lookup_open() to differentiate -EISDIR/-ENOTDIR. Also, for
filesystems that define ->atomic_open, handling this cannot even be done
at the VFS level, as we can't know ahead what the result of the lookup
will be.

Suggested-by: Christian Brauner (Amutable) <brauner@kernel.org>
Reviewed-by: NeilBrown <neil@brown.name>
Signed-off-by: Jori Koolstra <jkoolstra@xs4all.nl>
---
 fs/open.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/fs/open.c b/fs/open.c
index 4177f7226537..e8cb9153ea57 100644
--- a/fs/open.c
+++ b/fs/open.c
@@ -1319,6 +1319,15 @@ inline int build_open_flags(const struct open_how *how, struct open_flags *op)
 
 	op->intent = flags & O_PATH ? 0 : LOOKUP_OPEN;
 
+	/*
+	 * Requesting write access on a directory can never succeed. Rather
+	 * than performing a path-walk to determine whether the target is
+	 * actually a directory (-EISDIR) or not (-ENOTDIR), we short-circuit
+	 * to -ENOTDIR.
+	 */
+	if ((flags & O_DIRECTORY) && !(flags & __O_TMPFILE) && (acc_mode & MAY_WRITE))
+		return -ENOTDIR;
+
 	if (flags & O_CREAT) {
 		if (flags & O_DIRECTORY) {
 			/*
-- 
2.56.0

[PATCH v7 11/12] vfs: change ->create/->mkdir operations unavailable errno

[Jori Koolstra]

Currently filesystems return -EACCES for missing ->create and -EPERM for
missing ->mkdir in vfs_create/vfs_mkdir. Instead of duplicating these
dubious and inconsistent error codes to lookup_open(), change all to
-EOPNOTSUPP.

Signed-off-by: Jori Koolstra <jkoolstra@xs4all.nl>
---
 arch/alpha/include/uapi/asm/errno.h  |  2 +-
 arch/mips/include/uapi/asm/errno.h   |  2 +-
 arch/parisc/include/uapi/asm/errno.h |  2 +-
 arch/sparc/include/uapi/asm/errno.h  |  2 +-
 fs/namei.c                           | 14 +++++---------
 include/uapi/asm-generic/errno.h     |  2 +-
 6 files changed, 10 insertions(+), 14 deletions(-)

diff --git a/arch/alpha/include/uapi/asm/errno.h b/arch/alpha/include/uapi/asm/errno.h
index 1a99f38813c7..0384f6998974 100644
--- a/arch/alpha/include/uapi/asm/errno.h
+++ b/arch/alpha/include/uapi/asm/errno.h
@@ -19,7 +19,7 @@
 #define	ENOPROTOOPT	42	/* Protocol not available */
 #define	EPROTONOSUPPORT	43	/* Protocol not supported */
 #define	ESOCKTNOSUPPORT	44	/* Socket type not supported */
-#define	EOPNOTSUPP	45	/* Operation not supported on transport endpoint */
+#define	EOPNOTSUPP	45	/* Operation not supported */
 #define	EPFNOSUPPORT	46	/* Protocol family not supported */
 #define	EAFNOSUPPORT	47	/* Address family not supported by protocol */
 #define	EADDRINUSE	48	/* Address already in use */
diff --git a/arch/mips/include/uapi/asm/errno.h b/arch/mips/include/uapi/asm/errno.h
index 1835a50b69ce..2b6813715d1e 100644
--- a/arch/mips/include/uapi/asm/errno.h
+++ b/arch/mips/include/uapi/asm/errno.h
@@ -75,7 +75,7 @@
 #define ENOPROTOOPT	99	/* Protocol not available */
 #define EPROTONOSUPPORT 120	/* Protocol not supported */
 #define ESOCKTNOSUPPORT 121	/* Socket type not supported */
-#define EOPNOTSUPP	122	/* Operation not supported on transport endpoint */
+#define EOPNOTSUPP	122	/* Operation not supported */
 #define EPFNOSUPPORT	123	/* Protocol family not supported */
 #define EAFNOSUPPORT	124	/* Address family not supported by protocol */
 #define EADDRINUSE	125	/* Address already in use */
diff --git a/arch/parisc/include/uapi/asm/errno.h b/arch/parisc/include/uapi/asm/errno.h
index 93194fbb0a80..74bd65d79145 100644
--- a/arch/parisc/include/uapi/asm/errno.h
+++ b/arch/parisc/include/uapi/asm/errno.h
@@ -84,7 +84,7 @@
 #define	ENOPROTOOPT	220	/* Protocol not available */
 #define	EPROTONOSUPPORT	221	/* Protocol not supported */
 #define	ESOCKTNOSUPPORT	222	/* Socket type not supported */
-#define	EOPNOTSUPP	223	/* Operation not supported on transport endpoint */
+#define	EOPNOTSUPP	223	/* Operation not supported */
 #define	EPFNOSUPPORT	224	/* Protocol family not supported */
 #define	EAFNOSUPPORT	225	/* Address family not supported by protocol */
 #define	EADDRINUSE	226	/* Address already in use */
diff --git a/arch/sparc/include/uapi/asm/errno.h b/arch/sparc/include/uapi/asm/errno.h
index 71940ec9130b..699f648e1a35 100644
--- a/arch/sparc/include/uapi/asm/errno.h
+++ b/arch/sparc/include/uapi/asm/errno.h
@@ -16,7 +16,7 @@
 #define	ENOPROTOOPT	42	/* Protocol not available */
 #define	EPROTONOSUPPORT	43	/* Protocol not supported */
 #define	ESOCKTNOSUPPORT	44	/* Socket type not supported */
-#define	EOPNOTSUPP	45	/* Op not supported on transport endpoint */
+#define	EOPNOTSUPP	45	/* Operation not supported */
 #define	EPFNOSUPPORT	46	/* Protocol family not supported */
 #define	EAFNOSUPPORT	47	/* Address family not supported by protocol */
 #define	EADDRINUSE	48	/* Address already in use */
diff --git a/fs/namei.c b/fs/namei.c
index e7d3d28f3fd7..f21cd1028a8a 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -4254,7 +4254,7 @@ 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);
@@ -4650,13 +4650,9 @@ static struct dentry *lookup_open(struct nameidata *nd, struct file *file,
 		goto out_dput;
 	}
 
-	/* mimic operation missing errnos of vfs_mkdir/vfs_create */
-	if (create_dir && !dir_inode->i_op->mkdir) {
-		error = -EPERM;
-		goto out_dput;
-	}
-	if (!create_dir && !dir_inode->i_op->create) {
-		error = -EACCES;
+	if ((create_dir && !dir_inode->i_op->mkdir)
+		|| (!create_dir && !dir_inode->i_op->create)) {
+		error = -EOPNOTSUPP;
 		goto out_dput;
 	}
 
@@ -5518,7 +5514,7 @@ struct dentry *vfs_mkdir(struct mnt_idmap *idmap, struct inode *dir,
 	if (error)
 		goto err;
 
-	error = -EPERM;
+	error = -EOPNOTSUPP;
 	if (!dir->i_op->mkdir)
 		goto err;
 
diff --git a/include/uapi/asm-generic/errno.h b/include/uapi/asm-generic/errno.h
index bd78e69e0a43..c84ebf89c8b6 100644
--- a/include/uapi/asm-generic/errno.h
+++ b/include/uapi/asm-generic/errno.h
@@ -76,7 +76,7 @@
 #define	ENOPROTOOPT	92	/* Protocol not available */
 #define	EPROTONOSUPPORT	93	/* Protocol not supported */
 #define	ESOCKTNOSUPPORT	94	/* Socket type not supported */
-#define	EOPNOTSUPP	95	/* Operation not supported on transport endpoint */
+#define	EOPNOTSUPP	95	/* Operation not supported */
 #define	EPFNOSUPPORT	96	/* Protocol family not supported */
 #define	EAFNOSUPPORT	97	/* Address family not supported by protocol */
 #define	EADDRINUSE	98	/* Address already in use */
-- 
2.56.0

[PATCH v7 12/12] 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  |   2 +-
 .../filesystems/open_o_creat_o_dir.c          | 346 ++++++++++++++++++
 .../testing/selftests/filesystems/wrappers.h  |  11 +
 4 files changed, 359 insertions(+), 1 deletion(-)
 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 62f7b1c46649..e0ae02a5d2d1 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 93e2cc9123b0..a8b48cb6afd6 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 ustat_test rw_hint_test
+TEST_GEN_PROGS := open_o_creat_o_dir devpts_pts file_stressor anon_inode_test kernfs_test fclog ustat_test rw_hint_test
 TEST_GEN_PROGS += idmapped_tmpfile
 TEST_GEN_PROGS_EXTENDED := dnotify_test
 
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..93c0f4dea266
--- /dev/null
+++ b/tools/testing/selftests/filesystems/open_o_creat_o_dir.c
@@ -0,0 +1,346 @@
+// SPDX-License-Identifier: GPL-2.0
+#include <sys/stat.h>
+#include <sys/vfs.h>
+#include <errno.h>
+#include <limits.h>
+#include <fcntl.h>
+#include <stdio.h>
+#include <string.h>
+#include <unistd.h>
+#include <linux/magic.h>
+
+#include "kselftest_harness.h"
+#include "wrappers.h"
+
+#define openat_o_mkdir_checked_flags(dfd, pathname, flags) ({	\
+	struct stat __st;						\
+	int __fd = openat_o_mkdir(dfd, pathname, flags, S_IRWXU);	\
+	ASSERT_GE(__fd, 0);						\
+	ASSERT_EQ(fstat(__fd, &__st), 0);				\
+	EXPECT_TRUE(S_ISDIR(__st.st_mode));				\
+	__fd;								\
+})
+
+#define openat_o_mkdir_checked(dfd, pathname) \
+	openat_o_mkdir_checked_flags(dfd, pathname, O_RDONLY)
+
+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 = openat_o_mkdir_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 = openat_o_mkdir_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(openat_o_mkdir(self->dfd, "nonexistent/child", O_RDONLY, S_IRWXU), -1);
+	EXPECT_EQ(errno, ENOENT);
+}
+
+/* An invalid dfd must fail with EBADF. */
+TEST_F(open_o_creat_o_dir, ebadf)
+{
+	EXPECT_EQ(openat_o_mkdir(FD_INVALID, "badfdir", O_RDONLY, S_IRWXU), -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_RDONLY, S_IRWXU);
+	ASSERT_GE(file_fd, 0);
+
+	EXPECT_EQ(openat_o_mkdir(file_fd, "subdir", O_RDONLY, S_IRWXU), -1);
+	EXPECT_EQ(errno, ENOTDIR);
+
+	EXPECT_EQ(close(file_fd), 0);
+	EXPECT_EQ(unlinkat(self->dfd, "file", 0), 0);
+}
+
+/*
+ * O_CREAT|O_DIRECTORY without O_EXCL must fail with EINVAL and must not
+ * create anything at the path.
+ */
+TEST_F(open_o_creat_o_dir, requires_o_excl)
+{
+	struct stat st;
+
+	EXPECT_EQ(openat(self->dfd, "noexcl", O_CREAT | O_DIRECTORY | O_RDONLY,
+			 S_IRWXU), -1);
+	EXPECT_EQ(errno, EINVAL);
+
+	EXPECT_EQ(fstatat(self->dfd, "noexcl", &st, 0), -1);
+	EXPECT_EQ(errno, ENOENT);
+}
+
+/*
+ * Once the directory exists, repeating O_CREAT|O_DIRECTORY|O_EXCL must
+ * fail with EEXIST.
+ */
+TEST_F(open_o_creat_o_dir, existing_dir_eexist)
+{
+	int excldir_fd;
+
+	excldir_fd = openat_o_mkdir_checked(self->dfd, "excldir");
+
+	EXPECT_EQ(openat_o_mkdir(self->dfd, "excldir", O_RDONLY, S_IRWXU), -1);
+	EXPECT_EQ(errno, EEXIST);
+
+	EXPECT_EQ(close(excldir_fd), 0);
+	EXPECT_EQ(unlinkat(self->dfd, "excldir", AT_REMOVEDIR), 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(openat_o_mkdir(self->dfd, "rdwrdir", O_RDWR, S_IRWXU), -1);
+	EXPECT_EQ(errno, ENOTDIR);
+}
+
+/*
+ * openat(O_CREAT|O_DIRECTORY) with a trailing slash should work.
+ */
+TEST_F(open_o_creat_o_dir, trailing_slash)
+{
+	int fd = openat_o_mkdir_checked(self->dfd, "newdir/");
+	EXPECT_EQ(close(fd), 0);
+	EXPECT_EQ(unlinkat(self->dfd, "newdir", AT_REMOVEDIR), 0);
+}
+
+/*
+ * 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_RDONLY, S_IRWXU);
+	EXPECT_EQ(fd, -1);
+	EXPECT_EQ(errno, EISDIR);
+
+	EXPECT_EQ(fstatat(self->dfd, "trailing", &st, 0), -1);
+	EXPECT_EQ(errno, ENOENT);
+}
+
+/*
+ * 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 = openat_o_mkdir_checked(self->dfd, "parent");
+	child_fd = openat_o_mkdir_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);
+}
+
+/*
+ * O_EXCL means a trailing symlink is never followed, so O_CREAT|O_DIRECTORY
+ * must refuse to create through a dangling one, and must not create
+ * anything at the symlink target.
+ */
+TEST_F(open_o_creat_o_dir, dangling_symlink_eexist)
+{
+	struct stat st;
+
+	ASSERT_EQ(symlinkat("danglink_target", self->dfd, "danglink"), 0);
+
+	EXPECT_EQ(openat_o_mkdir(self->dfd, "danglink", O_RDONLY, S_IRWXU), -1);
+	EXPECT_EQ(errno, EEXIST);
+
+	/* Nothing must have been created at the target. */
+	EXPECT_EQ(fstatat(self->dfd, "danglink_target", &st, 0), -1);
+	EXPECT_EQ(errno, ENOENT);
+
+	EXPECT_EQ(unlinkat(self->dfd, "danglink", 0), 0);
+}
+
+/*
+ * A trailing symlink is an existing entry, even if it resolves to a
+ * directory, so O_CREAT|O_DIRECTORY must fail with EEXIST.
+ */
+TEST_F(open_o_creat_o_dir, symlink_to_dir_eexist)
+{
+	ASSERT_EQ(mkdirat(self->dfd, "realdir", 0700), 0);
+	ASSERT_EQ(symlinkat("realdir", self->dfd, "dirlink"), 0);
+
+	EXPECT_EQ(openat_o_mkdir(self->dfd, "dirlink", O_RDONLY, S_IRWXU), -1);
+	EXPECT_EQ(errno, EEXIST);
+
+	EXPECT_EQ(unlinkat(self->dfd, "dirlink", 0), 0);
+	EXPECT_EQ(unlinkat(self->dfd, "realdir", AT_REMOVEDIR), 0);
+}
+
+/*
+ * O_TMPFILE is encoded as __O_TMPFILE|O_DIRECTORY.  Now that O_CREAT is no
+ * longer rejected alongside O_DIRECTORY, O_TMPFILE|O_CREAT must still be
+ * rejected explicitly so that it cannot create a persistent file on kernels
+ * that predate O_TMPFILE.
+ */
+TEST_F(open_o_creat_o_dir, tmpfile_with_o_creat_einval)
+{
+	EXPECT_EQ(openat(self->dfd, ".", O_TMPFILE | O_CREAT | O_RDWR, S_IRWXU),
+		  -1);
+	EXPECT_EQ(errno, EINVAL);
+}
+
+/*
+ * Open the first of @paths that is a mounted filesystem of type @magic.
+ * Returns -1 if there is none.
+ */
+static int open_fs_root(const char *const *paths, long magic)
+{
+	for (; *paths; paths++) {
+		struct statfs sfs;
+		int fd;
+
+		fd = open(*paths, O_DIRECTORY | O_RDONLY);
+		if (fd < 0)
+			continue;
+		if (!fstatfs(fd, &sfs) && sfs.f_type == magic)
+			return fd;
+		close(fd);
+	}
+	return -1;
+}
+
+/*
+ * kernfs: O_CREAT|O_DIRECTORY on cgroup2 must create a cgroup and return a
+ * fd to that very cgroup.
+ */
+TEST(kernfs_cgroup2)
+{
+	static const char *const paths[] = {
+		"/sys/fs/cgroup", "/sys/fs/cgroup/unified", NULL
+	};
+	struct stat st_fd, st_path;
+	int root_fd, fd, procs_fd;
+	char name[64];
+
+	if (geteuid() != 0)
+		SKIP(return, "needs root to create a cgroup");
+	root_fd = open_fs_root(paths, CGROUP2_SUPER_MAGIC);
+	if (root_fd < 0)
+		SKIP(return, "cgroup2 is not mounted");
+
+	snprintf(name, sizeof(name), "open_o_creat_o_dir.%d", getpid());
+
+	fd = openat_o_mkdir_checked(root_fd, name);
+
+	/* The fd refers to the cgroup that is now visible at the name. */
+	ASSERT_EQ(fstat(fd, &st_fd), 0);
+	ASSERT_EQ(fstatat(root_fd, name, &st_path, 0), 0);
+	EXPECT_EQ(st_fd.st_ino, st_path.st_ino);
+	EXPECT_EQ(st_fd.st_dev, st_path.st_dev);
+
+	/* It is a populated cgroup, not just an empty directory. */
+	procs_fd = openat(fd, "cgroup.procs", O_RDONLY);
+	EXPECT_GE(procs_fd, 0);
+	if (procs_fd >= 0)
+		EXPECT_EQ(close(procs_fd), 0);
+
+	EXPECT_EQ(close(fd), 0);
+	EXPECT_EQ(unlinkat(root_fd, name, AT_REMOVEDIR), 0);
+	EXPECT_EQ(close(root_fd), 0);
+}
+
+/*
+ * tracefs sets SB_I_NO_ATOMIC_MKDIR: O_CREAT|O_DIRECTORY is refused with
+ * EOPNOTSUPP before any lookup, whether or not the name exists, and
+ * creates nothing.
+ */
+TEST(tracefs_eopnotsupp)
+{
+	static const char *const paths[] = {
+		"/sys/kernel/tracing", "/sys/kernel/debug/tracing", NULL
+	};
+	int tracing_fd, inst_fd;
+	char name[64];
+	struct stat st;
+
+	if (geteuid() != 0)
+		SKIP(return, "needs root to access tracefs");
+	tracing_fd = open_fs_root(paths, TRACEFS_MAGIC);
+	if (tracing_fd < 0)
+		SKIP(return, "tracefs is not mounted");
+	inst_fd = openat(tracing_fd, "instances", O_DIRECTORY | O_RDONLY);
+	if (inst_fd < 0) {
+		close(tracing_fd);
+		SKIP(return, "no tracefs instances directory");
+	}
+
+	snprintf(name, sizeof(name), "open_o_creat_o_dir.%d", getpid());
+
+	/* Nonexistent name, where mkdir(2) would create an instance */
+	EXPECT_EQ(openat_o_mkdir(inst_fd, name, O_RDONLY, S_IRWXU), -1);
+	EXPECT_EQ(errno, EOPNOTSUPP);
+	EXPECT_EQ(fstatat(inst_fd, name, &st, 0), -1);
+	EXPECT_EQ(errno, ENOENT);
+
+	/* Existing name */
+	EXPECT_EQ(openat_o_mkdir(tracing_fd, "instances", O_RDONLY, S_IRWXU), -1);
+	EXPECT_EQ(errno, EOPNOTSUPP);
+
+	EXPECT_EQ(close(inst_fd), 0);
+	EXPECT_EQ(close(tracing_fd), 0);
+}
+
+TEST_HARNESS_MAIN
diff --git a/tools/testing/selftests/filesystems/wrappers.h b/tools/testing/selftests/filesystems/wrappers.h
index 420ae4f908cf..bc58bdb80063 100644
--- a/tools/testing/selftests/filesystems/wrappers.h
+++ b/tools/testing/selftests/filesystems/wrappers.h
@@ -13,6 +13,10 @@
 #define STATX_MNT_ID_UNIQUE 0x00004000U /* Want/got extended stx_mount_id */
 #endif
 
+#ifndef FD_INVALID
+#define FD_INVALID -10009
+#endif
+
 static inline int sys_fsopen(const char *fsname, unsigned int flags)
 {
 	return syscall(__NR_fsopen, fsname, flags);
@@ -105,4 +109,11 @@ static inline int sys_open_tree(int dfd, const char *filename, unsigned int flag
 	return syscall(__NR_open_tree, dfd, filename, flags);
 }
 
+static inline int openat_o_mkdir(int dfd, const char *pathname,
+				 unsigned int flags, mode_t mode)
+{
+	return syscall(__NR_openat, dfd, pathname,
+		       flags | O_DIRECTORY | O_CREAT | O_EXCL, mode);
+}
+
 #endif
-- 
2.56.0

Re: [PATCH v7 07/12] kernfs: hold kernfs_iattr_rwsem when initializing a new inode

[Greg Kroah-Hartman]

On Fri, Oct 16, 2026 at 04:38:01PM +0200, Jori Koolstra wrote:
> kernfs_refresh_inode() copies the uid, gid, timestamps and nlink of a
> kernfs_node into its VFS inode. Since commit 9caf69614225 ("kernfs:
> Introduce separate rwsem to protect inode attributes.") these are
> protected by kernfs_iattr_rwsem.
> 
> kernfs_init_inode() calls kernfs_refresh_inode(), but was not converted.
> Its callers in turn hold kernfs_rwsem (kernfs_iop_lookup(),
> kernfs_fill_super()) or no lock at all (__kernfs_fh_to_dentry()),
> neither of which excludes a concurrent
> kernfs_setattr()/kernfs_iop_setattr().
> 
> Take kernfs_iattr_rwsem for reading around the refresh in
> kernfs_init_inode(). This nests inside kernfs_rwsem, which matches
> kernfs_link_sibling() and kernfs_unlink_sibling().
> 
> Also, kernfs_get_inode() then does not need to hold kernfs_rwsem, so
> drop it from kernfs_fill_super().

Oh, nice, this fixes the issue of kernfs not keeping the inode like we
talked about at LPC?  Can't argue with this tiny patch, nice work!

Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>