[PATCH 0/9] fs: harden anon inodes

[PATCH 0/9] fs: harden anon inodes

[Christian Brauner]

* Anonymous inodes currently don't come with a proper mode causing
  issues in the kernel when we want to add useful VFS debug assert. Fix
  that by giving them a proper mode and masking it off when we report it
  to userspace which relies on them not having any mode.

* Anonymous inodes currently allow to change inode attributes because
  the VFS falls back to simple_setattr() if i_op->setattr isn't
  implemented. This means the ownership and mode for every single user
  of anon_inode_inode can be changed. Block that as it's either useless
  or actively harmful. If specific ownership is needed the respective
  subsystem should allocate anonymous inodes from their own private
  superblock.

* Port pidfs to the new anon_inode_{g,s}etattr() helpers.

* Add proper tests for anonymous inode behavior.

The anonymous inode specific fixes should ideally be backported to all
LTS kernels.

Signed-off-by: Christian Brauner <brauner@kernel.org>
---
Christian Brauner (9):
      anon_inode: use a proper mode internally
      pidfs: use anon_inode_getattr()
      anon_inode: explicitly block ->setattr()
      pidfs: use anon_inode_setattr()
      anon_inode: raise SB_I_NODEV and SB_I_NOEXEC
      selftests/filesystems: add first test for anonymous inodes
      selftests/filesystems: add second test for anonymous inodes
      selftests/filesystems: add third test for anonymous inodes
      selftests/filesystems: add fourth test for anonymous inodes

 fs/anon_inodes.c                                   | 45 ++++++++++++++
 fs/internal.h                                      |  5 ++
 fs/libfs.c                                         |  2 +-
 fs/pidfs.c                                         | 26 +-------
 tools/testing/selftests/filesystems/.gitignore     |  1 +
 tools/testing/selftests/filesystems/Makefile       |  2 +-
 .../selftests/filesystems/anon_inode_test.c        | 69 ++++++++++++++++++++++
 7 files changed, 124 insertions(+), 26 deletions(-)
---
base-commit: 0af2f6be1b4281385b618cb86ad946eded089ac8
change-id: 20250407-work-anon_inode-e22bb1a74992

[PATCH 1/9] anon_inode: use a proper mode internally

[Christian Brauner]

This allows the VFS to not trip over anonymous inodes and we can add
asserts based on the mode into the vfs. When we report it to userspace
we can simply hide the mode to avoid regressions. I've audited all
direct callers of alloc_anon_inode() and only secretmen overrides i_mode
and i_op inode operations but it already uses a regular file.

Fixes: af153bb63a336 ("vfs: catch invalid modes in may_open()")
Cc: <stable@vger.kernel.org> # all LTS kernels
Reported-by: syzbot+5d8e79d323a13aa0b248@syzkaller.appspotmail.com
Closes: https://lore.kernel.org/all/67ed3fb3.050a0220.14623d.0009.GAE@google.com
Signed-off-by: Christian Brauner <brauner@kernel.org>
---
 fs/anon_inodes.c | 36 ++++++++++++++++++++++++++++++++++++
 fs/internal.h    |  3 +++
 fs/libfs.c       |  2 +-
 3 files changed, 40 insertions(+), 1 deletion(-)

diff --git a/fs/anon_inodes.c b/fs/anon_inodes.c
index 583ac81669c2..42e4b9c34f89 100644
--- a/fs/anon_inodes.c
+++ b/fs/anon_inodes.c
@@ -24,9 +24,43 @@
 
 #include <linux/uaccess.h>
 
+#include "internal.h"
+
 static struct vfsmount *anon_inode_mnt __ro_after_init;
 static struct inode *anon_inode_inode __ro_after_init;
 
+/*
+ * User space expects anonymous inodes to have no file type in st_mode.
+ *
+ * In particular, 'lsof' has this legacy logic:
+ *
+ *	type = s->st_mode & S_IFMT;
+ *	switch (type) {
+ *	  ...
+ *	case 0:
+ *		if (!strcmp(p, "anon_inode"))
+ *			Lf->ntype = Ntype = N_ANON_INODE;
+ *
+ * to detect our old anon_inode logic.
+ *
+ * Rather than mess with our internal sane inode data, just fix it
+ * up here in getattr() by masking off the format bits.
+ */
+int anon_inode_getattr(struct mnt_idmap *idmap, const struct path *path,
+		       struct kstat *stat, u32 request_mask,
+		       unsigned int query_flags)
+{
+	struct inode *inode = d_inode(path->dentry);
+
+	generic_fillattr(&nop_mnt_idmap, request_mask, inode, stat);
+	stat->mode &= ~S_IFMT;
+	return 0;
+}
+
+static const struct inode_operations anon_inode_operations = {
+	.getattr = anon_inode_getattr,
+};
+
 /*
  * anon_inodefs_dname() is called from d_path().
  */
@@ -66,6 +100,7 @@ static struct inode *anon_inode_make_secure_inode(
 	if (IS_ERR(inode))
 		return inode;
 	inode->i_flags &= ~S_PRIVATE;
+	inode->i_op = &anon_inode_operations;
 	error =	security_inode_init_security_anon(inode, &QSTR(name),
 						  context_inode);
 	if (error) {
@@ -313,6 +348,7 @@ static int __init anon_inode_init(void)
 	anon_inode_inode = alloc_anon_inode(anon_inode_mnt->mnt_sb);
 	if (IS_ERR(anon_inode_inode))
 		panic("anon_inode_init() inode allocation failed (%ld)\n", PTR_ERR(anon_inode_inode));
+	anon_inode_inode->i_op = &anon_inode_operations;
 
 	return 0;
 }
diff --git a/fs/internal.h b/fs/internal.h
index b9b3e29a73fd..717dc9eb6185 100644
--- a/fs/internal.h
+++ b/fs/internal.h
@@ -343,3 +343,6 @@ static inline bool path_mounted(const struct path *path)
 void file_f_owner_release(struct file *file);
 bool file_seek_cur_needs_f_lock(struct file *file);
 int statmount_mnt_idmap(struct mnt_idmap *idmap, struct seq_file *seq, bool uid_map);
+int anon_inode_getattr(struct mnt_idmap *idmap, const struct path *path,
+		       struct kstat *stat, u32 request_mask,
+		       unsigned int query_flags);
diff --git a/fs/libfs.c b/fs/libfs.c
index 6393d7c49ee6..0ad3336f5b49 100644
--- a/fs/libfs.c
+++ b/fs/libfs.c
@@ -1647,7 +1647,7 @@ struct inode *alloc_anon_inode(struct super_block *s)
 	 * that it already _is_ on the dirty list.
 	 */
 	inode->i_state = I_DIRTY;
-	inode->i_mode = S_IRUSR | S_IWUSR;
+	inode->i_mode = S_IFREG | S_IRUSR | S_IWUSR;
 	inode->i_uid = current_fsuid();
 	inode->i_gid = current_fsgid();
 	inode->i_flags |= S_PRIVATE;

-- 
2.47.2

[PATCH 2/9] pidfs: use anon_inode_getattr()

[Christian Brauner]

So far pidfs did use it's own version. Just use the generic version. We
use our own wrappers because we're going to be implementing our own
retrieval properties soon.

Signed-off-by: Christian Brauner <brauner@kernel.org>
---
 fs/pidfs.c | 24 +-----------------------
 1 file changed, 1 insertion(+), 23 deletions(-)

diff --git a/fs/pidfs.c b/fs/pidfs.c
index d64a4cbeb0da..809c3393b6a3 100644
--- a/fs/pidfs.c
+++ b/fs/pidfs.c
@@ -572,33 +572,11 @@ static int pidfs_setattr(struct mnt_idmap *idmap, struct dentry *dentry,
 	return -EOPNOTSUPP;
 }
 
-
-/*
- * User space expects pidfs inodes to have no file type in st_mode.
- *
- * In particular, 'lsof' has this legacy logic:
- *
- *	type = s->st_mode & S_IFMT;
- *	switch (type) {
- *	  ...
- *	case 0:
- *		if (!strcmp(p, "anon_inode"))
- *			Lf->ntype = Ntype = N_ANON_INODE;
- *
- * to detect our old anon_inode logic.
- *
- * Rather than mess with our internal sane inode data, just fix it
- * up here in getattr() by masking off the format bits.
- */
 static int pidfs_getattr(struct mnt_idmap *idmap, const struct path *path,
 			 struct kstat *stat, u32 request_mask,
 			 unsigned int query_flags)
 {
-	struct inode *inode = d_inode(path->dentry);
-
-	generic_fillattr(&nop_mnt_idmap, request_mask, inode, stat);
-	stat->mode &= ~S_IFMT;
-	return 0;
+	return anon_inode_getattr(idmap, path, stat, request_mask, query_flags);
 }
 
 static const struct inode_operations pidfs_inode_operations = {

-- 
2.47.2

[PATCH 3/9] anon_inode: explicitly block ->setattr()

[Christian Brauner]

It is currently possible to change the mode and owner of the single
anonymous inode in the kernel:

int main(int argc, char *argv[])
{
        int ret, sfd;
        sigset_t mask;
        struct signalfd_siginfo fdsi;

        sigemptyset(&mask);
        sigaddset(&mask, SIGINT);
        sigaddset(&mask, SIGQUIT);

        ret = sigprocmask(SIG_BLOCK, &mask, NULL);
        if (ret < 0)
                _exit(1);

        sfd = signalfd(-1, &mask, 0);
        if (sfd < 0)
                _exit(2);

        ret = fchown(sfd, 5555, 5555);
        if (ret < 0)
                _exit(3);

        ret = fchmod(sfd, 0777);
        if (ret < 0)
                _exit(3);

        _exit(4);
}

This is a bug. It's not really a meaningful one because anonymous inodes
don't really figure into path lookup and they cannot be reopened via
/proc/<pid>/fd/<nr> and can't be used for lookup itself. So they can
only ever serve as direct references.

But it is still completely bogus to allow the mode and ownership or any
of the properties of the anonymous inode to be changed. Block this!

Cc: <stable@vger.kernel.org> # all LTS kernels
Signed-off-by: Christian Brauner <brauner@kernel.org>
---
 fs/anon_inodes.c | 7 +++++++
 fs/internal.h    | 2 ++
 2 files changed, 9 insertions(+)

diff --git a/fs/anon_inodes.c b/fs/anon_inodes.c
index 42e4b9c34f89..cb51a90bece0 100644
--- a/fs/anon_inodes.c
+++ b/fs/anon_inodes.c
@@ -57,8 +57,15 @@ int anon_inode_getattr(struct mnt_idmap *idmap, const struct path *path,
 	return 0;
 }
 
+int anon_inode_setattr(struct mnt_idmap *idmap, struct dentry *dentry,
+		       struct iattr *attr)
+{
+	return -EOPNOTSUPP;
+}
+
 static const struct inode_operations anon_inode_operations = {
 	.getattr = anon_inode_getattr,
+	.setattr = anon_inode_setattr,
 };
 
 /*
diff --git a/fs/internal.h b/fs/internal.h
index 717dc9eb6185..f545400ce607 100644
--- a/fs/internal.h
+++ b/fs/internal.h
@@ -346,3 +346,5 @@ int statmount_mnt_idmap(struct mnt_idmap *idmap, struct seq_file *seq, bool uid_
 int anon_inode_getattr(struct mnt_idmap *idmap, const struct path *path,
 		       struct kstat *stat, u32 request_mask,
 		       unsigned int query_flags);
+int anon_inode_setattr(struct mnt_idmap *idmap, struct dentry *dentry,
+		       struct iattr *attr);

-- 
2.47.2

[PATCH 4/9] pidfs: use anon_inode_setattr()

[Christian Brauner]

So far pidfs did use it's own version. Just use the generic version.
We use our own wrappers because we're going to be implementing
properties soon.

Signed-off-by: Christian Brauner <brauner@kernel.org>
---
 fs/pidfs.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/pidfs.c b/fs/pidfs.c
index 809c3393b6a3..10b4ee454cca 100644
--- a/fs/pidfs.c
+++ b/fs/pidfs.c
@@ -569,7 +569,7 @@ static struct vfsmount *pidfs_mnt __ro_after_init;
 static int pidfs_setattr(struct mnt_idmap *idmap, struct dentry *dentry,
 			 struct iattr *attr)
 {
-	return -EOPNOTSUPP;
+	return anon_inode_setattr(idmap, dentry, attr);
 }
 
 static int pidfs_getattr(struct mnt_idmap *idmap, const struct path *path,

-- 
2.47.2

[PATCH 5/9] anon_inode: raise SB_I_NODEV and SB_I_NOEXEC

[Christian Brauner]

It isn't possible to execute anoymous inodes because they cannot be
opened in any way after they have been created. This includes execution:

execveat(fd_anon_inode, "", NULL, NULL, AT_EMPTY_PATH)

Anonymous inodes have inode->f_op set to no_open_fops which sets
no_open() which returns ENXIO. That means any call to do_dentry_open()
which is the endpoint of the do_open_execat() will fail. There's no
chance to execute an anonymous inode. Unless a given subsystem overrides
it ofc.

Howerver, we should still harden this and raise SB_I_NODEV and
SB_I_NOEXEC on the superblock itself so that no one gets any creative
ideas.

Cc: <stable@vger.kernel.org> # all LTS kernels
Signed-off-by: Christian Brauner <brauner@kernel.org>
---
 fs/anon_inodes.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/fs/anon_inodes.c b/fs/anon_inodes.c
index cb51a90bece0..e51e7d88980a 100644
--- a/fs/anon_inodes.c
+++ b/fs/anon_inodes.c
@@ -86,6 +86,8 @@ static int anon_inodefs_init_fs_context(struct fs_context *fc)
 	struct pseudo_fs_context *ctx = init_pseudo(fc, ANON_INODE_FS_MAGIC);
 	if (!ctx)
 		return -ENOMEM;
+	fc->s_iflags |= SB_I_NOEXEC;
+	fc->s_iflags |= SB_I_NODEV;
 	ctx->dops = &anon_inodefs_dentry_operations;
 	return 0;
 }

-- 
2.47.2

[PATCH 6/9] selftests/filesystems: add first test for anonymous inodes

[Christian Brauner]

Test that anonymous inodes cannot be chown()ed.

Signed-off-by: Christian Brauner <brauner@kernel.org>
---
 tools/testing/selftests/filesystems/.gitignore     |  1 +
 tools/testing/selftests/filesystems/Makefile       |  2 +-
 .../selftests/filesystems/anon_inode_test.c        | 26 ++++++++++++++++++++++
 3 files changed, 28 insertions(+), 1 deletion(-)

diff --git a/tools/testing/selftests/filesystems/.gitignore b/tools/testing/selftests/filesystems/.gitignore
index 828b66a10c63..7afa58e2bb20 100644
--- a/tools/testing/selftests/filesystems/.gitignore
+++ b/tools/testing/selftests/filesystems/.gitignore
@@ -2,3 +2,4 @@
 dnotify_test
 devpts_pts
 file_stressor
+anon_inode_test
diff --git a/tools/testing/selftests/filesystems/Makefile b/tools/testing/selftests/filesystems/Makefile
index 66305fc34c60..b02326193fee 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
+TEST_GEN_PROGS := devpts_pts file_stressor anon_inode_test
 TEST_GEN_PROGS_EXTENDED := dnotify_test
 
 include ../lib.mk
diff --git a/tools/testing/selftests/filesystems/anon_inode_test.c b/tools/testing/selftests/filesystems/anon_inode_test.c
new file mode 100644
index 000000000000..f2cae8f1ccae
--- /dev/null
+++ b/tools/testing/selftests/filesystems/anon_inode_test.c
@@ -0,0 +1,26 @@
+// SPDX-License-Identifier: GPL-2.0
+#define _GNU_SOURCE
+#define __SANE_USERSPACE_TYPES__
+
+#include <fcntl.h>
+#include <stdio.h>
+#include <sys/stat.h>
+
+#include "../kselftest_harness.h"
+#include "overlayfs/wrappers.h"
+
+TEST(anon_inode_no_chown)
+{
+	int fd_context;
+
+	fd_context = sys_fsopen("tmpfs", 0);
+	ASSERT_GE(fd_context, 0);
+
+	ASSERT_LT(fchown(fd_context, 1234, 5678), 0);
+	ASSERT_EQ(errno, EOPNOTSUPP);
+
+	EXPECT_EQ(close(fd_context), 0);
+}
+
+TEST_HARNESS_MAIN
+

-- 
2.47.2

[PATCH 7/9] selftests/filesystems: add second test for anonymous inodes

[Christian Brauner]

Test that anonymous inodes cannot be chmod()ed.

Signed-off-by: Christian Brauner <brauner@kernel.org>
---
 tools/testing/selftests/filesystems/anon_inode_test.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/tools/testing/selftests/filesystems/anon_inode_test.c b/tools/testing/selftests/filesystems/anon_inode_test.c
index f2cae8f1ccae..7c4d0a225363 100644
--- a/tools/testing/selftests/filesystems/anon_inode_test.c
+++ b/tools/testing/selftests/filesystems/anon_inode_test.c
@@ -22,5 +22,18 @@ TEST(anon_inode_no_chown)
 	EXPECT_EQ(close(fd_context), 0);
 }
 
+TEST(anon_inode_no_chmod)
+{
+	int fd_context;
+
+	fd_context = sys_fsopen("tmpfs", 0);
+	ASSERT_GE(fd_context, 0);
+
+	ASSERT_LT(fchmod(fd_context, 0777), 0);
+	ASSERT_EQ(errno, EOPNOTSUPP);
+
+	EXPECT_EQ(close(fd_context), 0);
+}
+
 TEST_HARNESS_MAIN
 

-- 
2.47.2

[PATCH 8/9] selftests/filesystems: add third test for anonymous inodes

[Christian Brauner]

Test that anonymous inodes cannot be exec()ed.

Signed-off-by: Christian Brauner <brauner@kernel.org>
---
 tools/testing/selftests/filesystems/anon_inode_test.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/tools/testing/selftests/filesystems/anon_inode_test.c b/tools/testing/selftests/filesystems/anon_inode_test.c
index 7c4d0a225363..486496252ddd 100644
--- a/tools/testing/selftests/filesystems/anon_inode_test.c
+++ b/tools/testing/selftests/filesystems/anon_inode_test.c
@@ -35,5 +35,18 @@ TEST(anon_inode_no_chmod)
 	EXPECT_EQ(close(fd_context), 0);
 }
 
+TEST(anon_inode_no_exec)
+{
+	int fd_context;
+
+	fd_context = sys_fsopen("tmpfs", 0);
+	ASSERT_GE(fd_context, 0);
+
+	ASSERT_LT(execveat(fd_context, "", NULL, NULL, AT_EMPTY_PATH), 0);
+	ASSERT_EQ(errno, EACCES);
+
+	EXPECT_EQ(close(fd_context), 0);
+}
+
 TEST_HARNESS_MAIN
 

-- 
2.47.2

[PATCH 9/9] selftests/filesystems: add fourth test for anonymous inodes

[Christian Brauner]

Test that anonymous inodes cannot be open()ed.

Signed-off-by: Christian Brauner <brauner@kernel.org>
---
 tools/testing/selftests/filesystems/anon_inode_test.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/tools/testing/selftests/filesystems/anon_inode_test.c b/tools/testing/selftests/filesystems/anon_inode_test.c
index 486496252ddd..e8e0ef1460d2 100644
--- a/tools/testing/selftests/filesystems/anon_inode_test.c
+++ b/tools/testing/selftests/filesystems/anon_inode_test.c
@@ -48,5 +48,22 @@ TEST(anon_inode_no_exec)
 	EXPECT_EQ(close(fd_context), 0);
 }
 
+TEST(anon_inode_no_open)
+{
+	int fd_context;
+
+	fd_context = sys_fsopen("tmpfs", 0);
+	ASSERT_GE(fd_context, 0);
+
+	ASSERT_GE(dup2(fd_context, 500), 0);
+	ASSERT_EQ(close(fd_context), 0);
+	fd_context = 500;
+
+	ASSERT_LT(open("/proc/self/fd/500", 0), 0);
+	ASSERT_EQ(errno, ENXIO);
+
+	EXPECT_EQ(close(fd_context), 0);
+}
+
 TEST_HARNESS_MAIN
 

-- 
2.47.2

Re: [PATCH 0/9] fs: harden anon inodes

[Mateusz Guzik]

On Mon, Apr 7, 2025 at 11:54 AM Christian Brauner <brauner@kernel.org> wrote:
>
> * Anonymous inodes currently don't come with a proper mode causing
>   issues in the kernel when we want to add useful VFS debug assert. Fix
>   that by giving them a proper mode and masking it off when we report it
>   to userspace which relies on them not having any mode.
>
> * Anonymous inodes currently allow to change inode attributes because
>   the VFS falls back to simple_setattr() if i_op->setattr isn't
>   implemented. This means the ownership and mode for every single user
>   of anon_inode_inode can be changed. Block that as it's either useless
>   or actively harmful. If specific ownership is needed the respective
>   subsystem should allocate anonymous inodes from their own private
>   superblock.
>
> * Port pidfs to the new anon_inode_{g,s}etattr() helpers.
>
> * Add proper tests for anonymous inode behavior.
>
> The anonymous inode specific fixes should ideally be backported to all
> LTS kernels.
>
> Signed-off-by: Christian Brauner <brauner@kernel.org>
> ---
> Christian Brauner (9):
>       anon_inode: use a proper mode internally
>       pidfs: use anon_inode_getattr()
>       anon_inode: explicitly block ->setattr()
>       pidfs: use anon_inode_setattr()
>       anon_inode: raise SB_I_NODEV and SB_I_NOEXEC
>       selftests/filesystems: add first test for anonymous inodes
>       selftests/filesystems: add second test for anonymous inodes
>       selftests/filesystems: add third test for anonymous inodes
>       selftests/filesystems: add fourth test for anonymous inodes
>

I have two nits, past that LGTM

1. I would add a comment explaining why S_IFREG in alloc_anon_inode()
2. commit messages for selftests could spell out what's being added
instead of being counted, it's all one-liners

for example:
selftests/filesystems: validate that anonymous inodes cannot be chown()ed

-- 
Mateusz Guzik <mjguzik gmail.com>

Re: [PATCH 1/9] anon_inode: use a proper mode internally

[Jeff Layton]

On Mon, 2025-04-07 at 11:54 +0200, Christian Brauner wrote:
> This allows the VFS to not trip over anonymous inodes and we can add
> asserts based on the mode into the vfs. When we report it to userspace
> we can simply hide the mode to avoid regressions. I've audited all
> direct callers of alloc_anon_inode() and only secretmen overrides i_mode
> and i_op inode operations but it already uses a regular file.
> 
> Fixes: af153bb63a336 ("vfs: catch invalid modes in may_open()")
> Cc: <stable@vger.kernel.org> # all LTS kernels
> Reported-by: syzbot+5d8e79d323a13aa0b248@syzkaller.appspotmail.com
> Closes: https://lore.kernel.org/all/67ed3fb3.050a0220.14623d.0009.GAE@google.com
> Signed-off-by: Christian Brauner <brauner@kernel.org>
> ---
>  fs/anon_inodes.c | 36 ++++++++++++++++++++++++++++++++++++
>  fs/internal.h    |  3 +++
>  fs/libfs.c       |  2 +-
>  3 files changed, 40 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/anon_inodes.c b/fs/anon_inodes.c
> index 583ac81669c2..42e4b9c34f89 100644
> --- a/fs/anon_inodes.c
> +++ b/fs/anon_inodes.c
> @@ -24,9 +24,43 @@
>  
>  #include <linux/uaccess.h>
>  
> +#include "internal.h"
> +
>  static struct vfsmount *anon_inode_mnt __ro_after_init;
>  static struct inode *anon_inode_inode __ro_after_init;
>  
> +/*
> + * User space expects anonymous inodes to have no file type in st_mode.

Weird. Does anything actually depend on this?

ISTM that they should report a file type.

> + *
> + * In particular, 'lsof' has this legacy logic:
> + *
> + *	type = s->st_mode & S_IFMT;
> + *	switch (type) {
> + *	  ...
> + *	case 0:
> + *		if (!strcmp(p, "anon_inode"))
> + *			Lf->ntype = Ntype = N_ANON_INODE;
> + *
> + * to detect our old anon_inode logic.
> + *
> + * Rather than mess with our internal sane inode data, just fix it
> + * up here in getattr() by masking off the format bits.
> + */
> +int anon_inode_getattr(struct mnt_idmap *idmap, const struct path *path,
> +		       struct kstat *stat, u32 request_mask,
> +		       unsigned int query_flags)
> +{
> +	struct inode *inode = d_inode(path->dentry);
> +
> +	generic_fillattr(&nop_mnt_idmap, request_mask, inode, stat);
> +	stat->mode &= ~S_IFMT;
> +	return 0;
> +}
> +
> +static const struct inode_operations anon_inode_operations = {
> +	.getattr = anon_inode_getattr,
> +};
> +
>  /*
>   * anon_inodefs_dname() is called from d_path().
>   */
> @@ -66,6 +100,7 @@ static struct inode *anon_inode_make_secure_inode(
>  	if (IS_ERR(inode))
>  		return inode;
>  	inode->i_flags &= ~S_PRIVATE;
> +	inode->i_op = &anon_inode_operations;
>  	error =	security_inode_init_security_anon(inode, &QSTR(name),
>  						  context_inode);
>  	if (error) {
> @@ -313,6 +348,7 @@ static int __init anon_inode_init(void)
>  	anon_inode_inode = alloc_anon_inode(anon_inode_mnt->mnt_sb);
>  	if (IS_ERR(anon_inode_inode))
>  		panic("anon_inode_init() inode allocation failed (%ld)\n", PTR_ERR(anon_inode_inode));
> +	anon_inode_inode->i_op = &anon_inode_operations;
>  
>  	return 0;
>  }
> diff --git a/fs/internal.h b/fs/internal.h
> index b9b3e29a73fd..717dc9eb6185 100644
> --- a/fs/internal.h
> +++ b/fs/internal.h
> @@ -343,3 +343,6 @@ static inline bool path_mounted(const struct path *path)
>  void file_f_owner_release(struct file *file);
>  bool file_seek_cur_needs_f_lock(struct file *file);
>  int statmount_mnt_idmap(struct mnt_idmap *idmap, struct seq_file *seq, bool uid_map);
> +int anon_inode_getattr(struct mnt_idmap *idmap, const struct path *path,
> +		       struct kstat *stat, u32 request_mask,
> +		       unsigned int query_flags);
> diff --git a/fs/libfs.c b/fs/libfs.c
> index 6393d7c49ee6..0ad3336f5b49 100644
> --- a/fs/libfs.c
> +++ b/fs/libfs.c
> @@ -1647,7 +1647,7 @@ struct inode *alloc_anon_inode(struct super_block *s)
>  	 * that it already _is_ on the dirty list.
>  	 */
>  	inode->i_state = I_DIRTY;
> -	inode->i_mode = S_IRUSR | S_IWUSR;
> +	inode->i_mode = S_IFREG | S_IRUSR | S_IWUSR;
>  	inode->i_uid = current_fsuid();
>  	inode->i_gid = current_fsgid();
>  	inode->i_flags |= S_PRIVATE;
> 

-- 
Jeff Layton <jlayton@kernel.org>

Re: [PATCH 0/9] fs: harden anon inodes

[Jeff Layton]

On Mon, 2025-04-07 at 11:54 +0200, Christian Brauner wrote:
> * Anonymous inodes currently don't come with a proper mode causing
>   issues in the kernel when we want to add useful VFS debug assert. Fix
>   that by giving them a proper mode and masking it off when we report it
>   to userspace which relies on them not having any mode.
> 
> * Anonymous inodes currently allow to change inode attributes because
>   the VFS falls back to simple_setattr() if i_op->setattr isn't
>   implemented. This means the ownership and mode for every single user
>   of anon_inode_inode can be changed. Block that as it's either useless
>   or actively harmful. If specific ownership is needed the respective
>   subsystem should allocate anonymous inodes from their own private
>   superblock.
> 
> * Port pidfs to the new anon_inode_{g,s}etattr() helpers.
> 
> * Add proper tests for anonymous inode behavior.
> 
> The anonymous inode specific fixes should ideally be backported to all
> LTS kernels.
> 
> Signed-off-by: Christian Brauner <brauner@kernel.org>
> ---
> Christian Brauner (9):
>       anon_inode: use a proper mode internally
>       pidfs: use anon_inode_getattr()
>       anon_inode: explicitly block ->setattr()
>       pidfs: use anon_inode_setattr()
>       anon_inode: raise SB_I_NODEV and SB_I_NOEXEC
>       selftests/filesystems: add first test for anonymous inodes
>       selftests/filesystems: add second test for anonymous inodes
>       selftests/filesystems: add third test for anonymous inodes
>       selftests/filesystems: add fourth test for anonymous inodes
> 
>  fs/anon_inodes.c                                   | 45 ++++++++++++++
>  fs/internal.h                                      |  5 ++
>  fs/libfs.c                                         |  2 +-
>  fs/pidfs.c                                         | 26 +-------
>  tools/testing/selftests/filesystems/.gitignore     |  1 +
>  tools/testing/selftests/filesystems/Makefile       |  2 +-
>  .../selftests/filesystems/anon_inode_test.c        | 69 ++++++++++++++++++++++
>  7 files changed, 124 insertions(+), 26 deletions(-)
> ---
> base-commit: 0af2f6be1b4281385b618cb86ad946eded089ac8
> change-id: 20250407-work-anon_inode-e22bb1a74992
> 

This all looks like good changes to make. I'm still a little curious
about what might be dependent on not seeing a file type in st_mode, but
if we haven't traditionally reported one, it's probably safest to
continue without doing so.

Reviewed-by: Jeff Layton <jlayton@kernel.org>

Re: [PATCH 0/9] fs: harden anon inodes

[Christian Brauner]

On Mon, Apr 07, 2025 at 12:19:45PM +0200, Mateusz Guzik wrote:
> On Mon, Apr 7, 2025 at 11:54 AM Christian Brauner <brauner@kernel.org> wrote:
> >
> > * Anonymous inodes currently don't come with a proper mode causing
> >   issues in the kernel when we want to add useful VFS debug assert. Fix
> >   that by giving them a proper mode and masking it off when we report it
> >   to userspace which relies on them not having any mode.
> >
> > * Anonymous inodes currently allow to change inode attributes because
> >   the VFS falls back to simple_setattr() if i_op->setattr isn't
> >   implemented. This means the ownership and mode for every single user
> >   of anon_inode_inode can be changed. Block that as it's either useless
> >   or actively harmful. If specific ownership is needed the respective
> >   subsystem should allocate anonymous inodes from their own private
> >   superblock.
> >
> > * Port pidfs to the new anon_inode_{g,s}etattr() helpers.
> >
> > * Add proper tests for anonymous inode behavior.
> >
> > The anonymous inode specific fixes should ideally be backported to all
> > LTS kernels.
> >
> > Signed-off-by: Christian Brauner <brauner@kernel.org>
> > ---
> > Christian Brauner (9):
> >       anon_inode: use a proper mode internally
> >       pidfs: use anon_inode_getattr()
> >       anon_inode: explicitly block ->setattr()
> >       pidfs: use anon_inode_setattr()
> >       anon_inode: raise SB_I_NODEV and SB_I_NOEXEC
> >       selftests/filesystems: add first test for anonymous inodes
> >       selftests/filesystems: add second test for anonymous inodes
> >       selftests/filesystems: add third test for anonymous inodes
> >       selftests/filesystems: add fourth test for anonymous inodes
> >
> 
> I have two nits, past that LGTM
> 
> 1. I would add a comment explaining why S_IFREG in alloc_anon_inode()

        /*
         * Historically anonymous inodes didn't have a type at all and
         * userspace has come to rely on this. Internally they're just
         * regular files but S_IFREG is masked off when reporting
         * information to userspace.
         */

> 2. commit messages for selftests could spell out what's being added
> instead of being counted, it's all one-liners

I've replaced the count with chown(), chmod(), exec(), and open().

Thanks!

Re: [PATCH 1/9] anon_inode: use a proper mode internally

[Christian Brauner]

On Mon, Apr 07, 2025 at 08:19:25AM -0400, Jeff Layton wrote:
> On Mon, 2025-04-07 at 11:54 +0200, Christian Brauner wrote:
> > This allows the VFS to not trip over anonymous inodes and we can add
> > asserts based on the mode into the vfs. When we report it to userspace
> > we can simply hide the mode to avoid regressions. I've audited all
> > direct callers of alloc_anon_inode() and only secretmen overrides i_mode
> > and i_op inode operations but it already uses a regular file.
> > 
> > Fixes: af153bb63a336 ("vfs: catch invalid modes in may_open()")
> > Cc: <stable@vger.kernel.org> # all LTS kernels
> > Reported-by: syzbot+5d8e79d323a13aa0b248@syzkaller.appspotmail.com
> > Closes: https://lore.kernel.org/all/67ed3fb3.050a0220.14623d.0009.GAE@google.com
> > Signed-off-by: Christian Brauner <brauner@kernel.org>
> > ---
> >  fs/anon_inodes.c | 36 ++++++++++++++++++++++++++++++++++++
> >  fs/internal.h    |  3 +++
> >  fs/libfs.c       |  2 +-
> >  3 files changed, 40 insertions(+), 1 deletion(-)
> > 
> > diff --git a/fs/anon_inodes.c b/fs/anon_inodes.c
> > index 583ac81669c2..42e4b9c34f89 100644
> > --- a/fs/anon_inodes.c
> > +++ b/fs/anon_inodes.c
> > @@ -24,9 +24,43 @@
> >  
> >  #include <linux/uaccess.h>
> >  
> > +#include "internal.h"
> > +
> >  static struct vfsmount *anon_inode_mnt __ro_after_init;
> >  static struct inode *anon_inode_inode __ro_after_init;
> >  
> > +/*
> > + * User space expects anonymous inodes to have no file type in st_mode.
> 
> Weird. Does anything actually depend on this?

Yeah, lsof failed and started complaining about this. They're checking
the mode to recognize anonymous inodes. And tbf, it works to generically
recognizer proper anonymous inodes because they came come from very
different superblocks (dmabuf, drm, vfio, anon_inode_mnt etc.).

> 
> ISTM that they should report a file type.

I agree but that ship has probably sailed.

Re: [PATCH 1/9] anon_inode: use a proper mode internally

[Jan Kara]

On Mon 07-04-25 11:54:15, Christian Brauner wrote:
> This allows the VFS to not trip over anonymous inodes and we can add
> asserts based on the mode into the vfs. When we report it to userspace
> we can simply hide the mode to avoid regressions. I've audited all
> direct callers of alloc_anon_inode() and only secretmen overrides i_mode
> and i_op inode operations but it already uses a regular file.
> 
> Fixes: af153bb63a336 ("vfs: catch invalid modes in may_open()")
> Cc: <stable@vger.kernel.org> # all LTS kernels
> Reported-by: syzbot+5d8e79d323a13aa0b248@syzkaller.appspotmail.com
> Closes: https://lore.kernel.org/all/67ed3fb3.050a0220.14623d.0009.GAE@google.com
> Signed-off-by: Christian Brauner <brauner@kernel.org>

Looks good. Feel free to add:

Reviewed-by: Jan Kara <jack@suse.cz>

								Honza

> ---
>  fs/anon_inodes.c | 36 ++++++++++++++++++++++++++++++++++++
>  fs/internal.h    |  3 +++
>  fs/libfs.c       |  2 +-
>  3 files changed, 40 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/anon_inodes.c b/fs/anon_inodes.c
> index 583ac81669c2..42e4b9c34f89 100644
> --- a/fs/anon_inodes.c
> +++ b/fs/anon_inodes.c
> @@ -24,9 +24,43 @@
>  
>  #include <linux/uaccess.h>
>  
> +#include "internal.h"
> +
>  static struct vfsmount *anon_inode_mnt __ro_after_init;
>  static struct inode *anon_inode_inode __ro_after_init;
>  
> +/*
> + * User space expects anonymous inodes to have no file type in st_mode.
> + *
> + * In particular, 'lsof' has this legacy logic:
> + *
> + *	type = s->st_mode & S_IFMT;
> + *	switch (type) {
> + *	  ...
> + *	case 0:
> + *		if (!strcmp(p, "anon_inode"))
> + *			Lf->ntype = Ntype = N_ANON_INODE;
> + *
> + * to detect our old anon_inode logic.
> + *
> + * Rather than mess with our internal sane inode data, just fix it
> + * up here in getattr() by masking off the format bits.
> + */
> +int anon_inode_getattr(struct mnt_idmap *idmap, const struct path *path,
> +		       struct kstat *stat, u32 request_mask,
> +		       unsigned int query_flags)
> +{
> +	struct inode *inode = d_inode(path->dentry);
> +
> +	generic_fillattr(&nop_mnt_idmap, request_mask, inode, stat);
> +	stat->mode &= ~S_IFMT;
> +	return 0;
> +}
> +
> +static const struct inode_operations anon_inode_operations = {
> +	.getattr = anon_inode_getattr,
> +};
> +
>  /*
>   * anon_inodefs_dname() is called from d_path().
>   */
> @@ -66,6 +100,7 @@ static struct inode *anon_inode_make_secure_inode(
>  	if (IS_ERR(inode))
>  		return inode;
>  	inode->i_flags &= ~S_PRIVATE;
> +	inode->i_op = &anon_inode_operations;
>  	error =	security_inode_init_security_anon(inode, &QSTR(name),
>  						  context_inode);
>  	if (error) {
> @@ -313,6 +348,7 @@ static int __init anon_inode_init(void)
>  	anon_inode_inode = alloc_anon_inode(anon_inode_mnt->mnt_sb);
>  	if (IS_ERR(anon_inode_inode))
>  		panic("anon_inode_init() inode allocation failed (%ld)\n", PTR_ERR(anon_inode_inode));
> +	anon_inode_inode->i_op = &anon_inode_operations;
>  
>  	return 0;
>  }
> diff --git a/fs/internal.h b/fs/internal.h
> index b9b3e29a73fd..717dc9eb6185 100644
> --- a/fs/internal.h
> +++ b/fs/internal.h
> @@ -343,3 +343,6 @@ static inline bool path_mounted(const struct path *path)
>  void file_f_owner_release(struct file *file);
>  bool file_seek_cur_needs_f_lock(struct file *file);
>  int statmount_mnt_idmap(struct mnt_idmap *idmap, struct seq_file *seq, bool uid_map);
> +int anon_inode_getattr(struct mnt_idmap *idmap, const struct path *path,
> +		       struct kstat *stat, u32 request_mask,
> +		       unsigned int query_flags);
> diff --git a/fs/libfs.c b/fs/libfs.c
> index 6393d7c49ee6..0ad3336f5b49 100644
> --- a/fs/libfs.c
> +++ b/fs/libfs.c
> @@ -1647,7 +1647,7 @@ struct inode *alloc_anon_inode(struct super_block *s)
>  	 * that it already _is_ on the dirty list.
>  	 */
>  	inode->i_state = I_DIRTY;
> -	inode->i_mode = S_IRUSR | S_IWUSR;
> +	inode->i_mode = S_IFREG | S_IRUSR | S_IWUSR;
>  	inode->i_uid = current_fsuid();
>  	inode->i_gid = current_fsgid();
>  	inode->i_flags |= S_PRIVATE;
> 
> -- 
> 2.47.2
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

Re: [PATCH 2/9] pidfs: use anon_inode_getattr()

[Jan Kara]

On Mon 07-04-25 11:54:16, Christian Brauner wrote:
> So far pidfs did use it's own version. Just use the generic version. We
> use our own wrappers because we're going to be implementing our own
> retrieval properties soon.
> 
> Signed-off-by: Christian Brauner <brauner@kernel.org>

Looks good. Feel free to add:

Reviewed-by: Jan Kara <jack@suse.cz>

								Honza

> ---
>  fs/pidfs.c | 24 +-----------------------
>  1 file changed, 1 insertion(+), 23 deletions(-)
> 
> diff --git a/fs/pidfs.c b/fs/pidfs.c
> index d64a4cbeb0da..809c3393b6a3 100644
> --- a/fs/pidfs.c
> +++ b/fs/pidfs.c
> @@ -572,33 +572,11 @@ static int pidfs_setattr(struct mnt_idmap *idmap, struct dentry *dentry,
>  	return -EOPNOTSUPP;
>  }
>  
> -
> -/*
> - * User space expects pidfs inodes to have no file type in st_mode.
> - *
> - * In particular, 'lsof' has this legacy logic:
> - *
> - *	type = s->st_mode & S_IFMT;
> - *	switch (type) {
> - *	  ...
> - *	case 0:
> - *		if (!strcmp(p, "anon_inode"))
> - *			Lf->ntype = Ntype = N_ANON_INODE;
> - *
> - * to detect our old anon_inode logic.
> - *
> - * Rather than mess with our internal sane inode data, just fix it
> - * up here in getattr() by masking off the format bits.
> - */
>  static int pidfs_getattr(struct mnt_idmap *idmap, const struct path *path,
>  			 struct kstat *stat, u32 request_mask,
>  			 unsigned int query_flags)
>  {
> -	struct inode *inode = d_inode(path->dentry);
> -
> -	generic_fillattr(&nop_mnt_idmap, request_mask, inode, stat);
> -	stat->mode &= ~S_IFMT;
> -	return 0;
> +	return anon_inode_getattr(idmap, path, stat, request_mask, query_flags);
>  }
>  
>  static const struct inode_operations pidfs_inode_operations = {
> 
> -- 
> 2.47.2
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

Re: [PATCH 3/9] anon_inode: explicitly block ->setattr()

[Jan Kara]

On Mon 07-04-25 11:54:17, Christian Brauner wrote:
> It is currently possible to change the mode and owner of the single
> anonymous inode in the kernel:
> 
> int main(int argc, char *argv[])
> {
>         int ret, sfd;
>         sigset_t mask;
>         struct signalfd_siginfo fdsi;
> 
>         sigemptyset(&mask);
>         sigaddset(&mask, SIGINT);
>         sigaddset(&mask, SIGQUIT);
> 
>         ret = sigprocmask(SIG_BLOCK, &mask, NULL);
>         if (ret < 0)
>                 _exit(1);
> 
>         sfd = signalfd(-1, &mask, 0);
>         if (sfd < 0)
>                 _exit(2);
> 
>         ret = fchown(sfd, 5555, 5555);
>         if (ret < 0)
>                 _exit(3);
> 
>         ret = fchmod(sfd, 0777);
>         if (ret < 0)
>                 _exit(3);
> 
>         _exit(4);
> }
> 
> This is a bug. It's not really a meaningful one because anonymous inodes
> don't really figure into path lookup and they cannot be reopened via
> /proc/<pid>/fd/<nr> and can't be used for lookup itself. So they can
> only ever serve as direct references.
> 
> But it is still completely bogus to allow the mode and ownership or any
> of the properties of the anonymous inode to be changed. Block this!
> 
> Cc: <stable@vger.kernel.org> # all LTS kernels
> Signed-off-by: Christian Brauner <brauner@kernel.org>

Definitely. Feel free to add:

Reviewed-by: Jan Kara <jack@suse.cz>

								Honza

> ---
>  fs/anon_inodes.c | 7 +++++++
>  fs/internal.h    | 2 ++
>  2 files changed, 9 insertions(+)
> 
> diff --git a/fs/anon_inodes.c b/fs/anon_inodes.c
> index 42e4b9c34f89..cb51a90bece0 100644
> --- a/fs/anon_inodes.c
> +++ b/fs/anon_inodes.c
> @@ -57,8 +57,15 @@ int anon_inode_getattr(struct mnt_idmap *idmap, const struct path *path,
>  	return 0;
>  }
>  
> +int anon_inode_setattr(struct mnt_idmap *idmap, struct dentry *dentry,
> +		       struct iattr *attr)
> +{
> +	return -EOPNOTSUPP;
> +}
> +
>  static const struct inode_operations anon_inode_operations = {
>  	.getattr = anon_inode_getattr,
> +	.setattr = anon_inode_setattr,
>  };
>  
>  /*
> diff --git a/fs/internal.h b/fs/internal.h
> index 717dc9eb6185..f545400ce607 100644
> --- a/fs/internal.h
> +++ b/fs/internal.h
> @@ -346,3 +346,5 @@ int statmount_mnt_idmap(struct mnt_idmap *idmap, struct seq_file *seq, bool uid_
>  int anon_inode_getattr(struct mnt_idmap *idmap, const struct path *path,
>  		       struct kstat *stat, u32 request_mask,
>  		       unsigned int query_flags);
> +int anon_inode_setattr(struct mnt_idmap *idmap, struct dentry *dentry,
> +		       struct iattr *attr);
> 
> -- 
> 2.47.2
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

Re: [PATCH 4/9] pidfs: use anon_inode_setattr()

[Jan Kara]

On Mon 07-04-25 11:54:18, Christian Brauner wrote:
> So far pidfs did use it's own version. Just use the generic version.
> We use our own wrappers because we're going to be implementing
> properties soon.
> 
> Signed-off-by: Christian Brauner <brauner@kernel.org>

OK. Feel free to add:

Reviewed-by: Jan Kara <jack@suse.cz>

								Honza

> ---
>  fs/pidfs.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/fs/pidfs.c b/fs/pidfs.c
> index 809c3393b6a3..10b4ee454cca 100644
> --- a/fs/pidfs.c
> +++ b/fs/pidfs.c
> @@ -569,7 +569,7 @@ static struct vfsmount *pidfs_mnt __ro_after_init;
>  static int pidfs_setattr(struct mnt_idmap *idmap, struct dentry *dentry,
>  			 struct iattr *attr)
>  {
> -	return -EOPNOTSUPP;
> +	return anon_inode_setattr(idmap, dentry, attr);
>  }
>  
>  static int pidfs_getattr(struct mnt_idmap *idmap, const struct path *path,
> 
> -- 
> 2.47.2
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

Re: [PATCH 5/9] anon_inode: raise SB_I_NODEV and SB_I_NOEXEC

[Jan Kara]

On Mon 07-04-25 11:54:19, Christian Brauner wrote:
> It isn't possible to execute anoymous inodes because they cannot be
				^^ anonymous

> opened in any way after they have been created. This includes execution:
> 
> execveat(fd_anon_inode, "", NULL, NULL, AT_EMPTY_PATH)
> 
> Anonymous inodes have inode->f_op set to no_open_fops which sets
> no_open() which returns ENXIO. That means any call to do_dentry_open()
> which is the endpoint of the do_open_execat() will fail. There's no
> chance to execute an anonymous inode. Unless a given subsystem overrides
> it ofc.
> 
> Howerver, we should still harden this and raise SB_I_NODEV and
  ^^^ However

> SB_I_NOEXEC on the superblock itself so that no one gets any creative
> ideas.

;)

Feel free to add:

Reviewed-by: Jan Kara <jack@suse.cz>

								Honza

> 
> Cc: <stable@vger.kernel.org> # all LTS kernels
> Signed-off-by: Christian Brauner <brauner@kernel.org>
> ---
>  fs/anon_inodes.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/fs/anon_inodes.c b/fs/anon_inodes.c
> index cb51a90bece0..e51e7d88980a 100644
> --- a/fs/anon_inodes.c
> +++ b/fs/anon_inodes.c
> @@ -86,6 +86,8 @@ static int anon_inodefs_init_fs_context(struct fs_context *fc)
>  	struct pseudo_fs_context *ctx = init_pseudo(fc, ANON_INODE_FS_MAGIC);
>  	if (!ctx)
>  		return -ENOMEM;
> +	fc->s_iflags |= SB_I_NOEXEC;
> +	fc->s_iflags |= SB_I_NODEV;
>  	ctx->dops = &anon_inodefs_dentry_operations;
>  	return 0;
>  }
> 
> -- 
> 2.47.2
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

Re: [PATCH 6/9] selftests/filesystems: add first test for anonymous inodes

[Jan Kara]

On Mon 07-04-25 11:54:20, Christian Brauner wrote:
> Test that anonymous inodes cannot be chown()ed.
> 
> Signed-off-by: Christian Brauner <brauner@kernel.org>

Looks good. Feel free to add:

Reviewed-by: Jan Kara <jack@suse.cz>

								Honza

> ---
>  tools/testing/selftests/filesystems/.gitignore     |  1 +
>  tools/testing/selftests/filesystems/Makefile       |  2 +-
>  .../selftests/filesystems/anon_inode_test.c        | 26 ++++++++++++++++++++++
>  3 files changed, 28 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/testing/selftests/filesystems/.gitignore b/tools/testing/selftests/filesystems/.gitignore
> index 828b66a10c63..7afa58e2bb20 100644
> --- a/tools/testing/selftests/filesystems/.gitignore
> +++ b/tools/testing/selftests/filesystems/.gitignore
> @@ -2,3 +2,4 @@
>  dnotify_test
>  devpts_pts
>  file_stressor
> +anon_inode_test
> diff --git a/tools/testing/selftests/filesystems/Makefile b/tools/testing/selftests/filesystems/Makefile
> index 66305fc34c60..b02326193fee 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
> +TEST_GEN_PROGS := devpts_pts file_stressor anon_inode_test
>  TEST_GEN_PROGS_EXTENDED := dnotify_test
>  
>  include ../lib.mk
> diff --git a/tools/testing/selftests/filesystems/anon_inode_test.c b/tools/testing/selftests/filesystems/anon_inode_test.c
> new file mode 100644
> index 000000000000..f2cae8f1ccae
> --- /dev/null
> +++ b/tools/testing/selftests/filesystems/anon_inode_test.c
> @@ -0,0 +1,26 @@
> +// SPDX-License-Identifier: GPL-2.0
> +#define _GNU_SOURCE
> +#define __SANE_USERSPACE_TYPES__
> +
> +#include <fcntl.h>
> +#include <stdio.h>
> +#include <sys/stat.h>
> +
> +#include "../kselftest_harness.h"
> +#include "overlayfs/wrappers.h"
> +
> +TEST(anon_inode_no_chown)
> +{
> +	int fd_context;
> +
> +	fd_context = sys_fsopen("tmpfs", 0);
> +	ASSERT_GE(fd_context, 0);
> +
> +	ASSERT_LT(fchown(fd_context, 1234, 5678), 0);
> +	ASSERT_EQ(errno, EOPNOTSUPP);
> +
> +	EXPECT_EQ(close(fd_context), 0);
> +}
> +
> +TEST_HARNESS_MAIN
> +
> 
> -- 
> 2.47.2
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

Re: [PATCH 7/9] selftests/filesystems: add second test for anonymous inodes

[Jan Kara]

On Mon 07-04-25 11:54:21, Christian Brauner wrote:
> Test that anonymous inodes cannot be chmod()ed.
> 
> Signed-off-by: Christian Brauner <brauner@kernel.org>

Looks good. Feel free to add:

Reviewed-by: Jan Kara <jack@suse.cz>

								Honza

> ---
>  tools/testing/selftests/filesystems/anon_inode_test.c | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
> 
> diff --git a/tools/testing/selftests/filesystems/anon_inode_test.c b/tools/testing/selftests/filesystems/anon_inode_test.c
> index f2cae8f1ccae..7c4d0a225363 100644
> --- a/tools/testing/selftests/filesystems/anon_inode_test.c
> +++ b/tools/testing/selftests/filesystems/anon_inode_test.c
> @@ -22,5 +22,18 @@ TEST(anon_inode_no_chown)
>  	EXPECT_EQ(close(fd_context), 0);
>  }
>  
> +TEST(anon_inode_no_chmod)
> +{
> +	int fd_context;
> +
> +	fd_context = sys_fsopen("tmpfs", 0);
> +	ASSERT_GE(fd_context, 0);
> +
> +	ASSERT_LT(fchmod(fd_context, 0777), 0);
> +	ASSERT_EQ(errno, EOPNOTSUPP);
> +
> +	EXPECT_EQ(close(fd_context), 0);
> +}
> +
>  TEST_HARNESS_MAIN
>  
> 
> -- 
> 2.47.2
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

Re: [PATCH 8/9] selftests/filesystems: add third test for anonymous inodes

[Jan Kara]

On Mon 07-04-25 11:54:22, Christian Brauner wrote:
> Test that anonymous inodes cannot be exec()ed.
> 
> Signed-off-by: Christian Brauner <brauner@kernel.org>

Looks good. Feel free to add:

Reviewed-by: Jan Kara <jack@suse.cz>

								Honza

> ---
>  tools/testing/selftests/filesystems/anon_inode_test.c | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
> 
> diff --git a/tools/testing/selftests/filesystems/anon_inode_test.c b/tools/testing/selftests/filesystems/anon_inode_test.c
> index 7c4d0a225363..486496252ddd 100644
> --- a/tools/testing/selftests/filesystems/anon_inode_test.c
> +++ b/tools/testing/selftests/filesystems/anon_inode_test.c
> @@ -35,5 +35,18 @@ TEST(anon_inode_no_chmod)
>  	EXPECT_EQ(close(fd_context), 0);
>  }
>  
> +TEST(anon_inode_no_exec)
> +{
> +	int fd_context;
> +
> +	fd_context = sys_fsopen("tmpfs", 0);
> +	ASSERT_GE(fd_context, 0);
> +
> +	ASSERT_LT(execveat(fd_context, "", NULL, NULL, AT_EMPTY_PATH), 0);
> +	ASSERT_EQ(errno, EACCES);
> +
> +	EXPECT_EQ(close(fd_context), 0);
> +}
> +
>  TEST_HARNESS_MAIN
>  
> 
> -- 
> 2.47.2
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

Re: [PATCH 9/9] selftests/filesystems: add fourth test for anonymous inodes

[Jan Kara]

On Mon 07-04-25 11:54:23, Christian Brauner wrote:
> Test that anonymous inodes cannot be open()ed.
> 
> Signed-off-by: Christian Brauner <brauner@kernel.org>

Looks good. Feel free to add:

Reviewed-by: Jan Kara <jack@suse.cz>

								Honza

> ---
>  tools/testing/selftests/filesystems/anon_inode_test.c | 17 +++++++++++++++++
>  1 file changed, 17 insertions(+)
> 
> diff --git a/tools/testing/selftests/filesystems/anon_inode_test.c b/tools/testing/selftests/filesystems/anon_inode_test.c
> index 486496252ddd..e8e0ef1460d2 100644
> --- a/tools/testing/selftests/filesystems/anon_inode_test.c
> +++ b/tools/testing/selftests/filesystems/anon_inode_test.c
> @@ -48,5 +48,22 @@ TEST(anon_inode_no_exec)
>  	EXPECT_EQ(close(fd_context), 0);
>  }
>  
> +TEST(anon_inode_no_open)
> +{
> +	int fd_context;
> +
> +	fd_context = sys_fsopen("tmpfs", 0);
> +	ASSERT_GE(fd_context, 0);
> +
> +	ASSERT_GE(dup2(fd_context, 500), 0);
> +	ASSERT_EQ(close(fd_context), 0);
> +	fd_context = 500;
> +
> +	ASSERT_LT(open("/proc/self/fd/500", 0), 0);
> +	ASSERT_EQ(errno, ENXIO);
> +
> +	EXPECT_EQ(close(fd_context), 0);
> +}
> +
>  TEST_HARNESS_MAIN
>  
> 
> -- 
> 2.47.2
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

Re: [PATCH 5/9] anon_inode: raise SB_I_NODEV and SB_I_NOEXEC

[Christian Brauner]

On Mon, Apr 07, 2025 at 04:07:47PM +0200, Jan Kara wrote:
> On Mon 07-04-25 11:54:19, Christian Brauner wrote:
> > It isn't possible to execute anoymous inodes because they cannot be
> 				^^ anonymous
> 
> > opened in any way after they have been created. This includes execution:
> > 
> > execveat(fd_anon_inode, "", NULL, NULL, AT_EMPTY_PATH)
> > 
> > Anonymous inodes have inode->f_op set to no_open_fops which sets
> > no_open() which returns ENXIO. That means any call to do_dentry_open()
> > which is the endpoint of the do_open_execat() will fail. There's no
> > chance to execute an anonymous inode. Unless a given subsystem overrides
> > it ofc.
> > 
> > Howerver, we should still harden this and raise SB_I_NODEV and
>   ^^^ However
> 
> > SB_I_NOEXEC on the superblock itself so that no one gets any creative
> > ideas.
> 
> ;)

I've told our new AI overloards to sprinkle-in some typos so no one
realizes I've been mostly replaced by a bot.

Or I'm just generally tired so I fat-finger a lot more than usual. :D

> 
> Feel free to add:
> 
> Reviewed-by: Jan Kara <jack@suse.cz>
> 
> 								Honza
> 
> > 
> > Cc: <stable@vger.kernel.org> # all LTS kernels
> > Signed-off-by: Christian Brauner <brauner@kernel.org>
> > ---
> >  fs/anon_inodes.c | 2 ++
> >  1 file changed, 2 insertions(+)
> > 
> > diff --git a/fs/anon_inodes.c b/fs/anon_inodes.c
> > index cb51a90bece0..e51e7d88980a 100644
> > --- a/fs/anon_inodes.c
> > +++ b/fs/anon_inodes.c
> > @@ -86,6 +86,8 @@ static int anon_inodefs_init_fs_context(struct fs_context *fc)
> >  	struct pseudo_fs_context *ctx = init_pseudo(fc, ANON_INODE_FS_MAGIC);
> >  	if (!ctx)
> >  		return -ENOMEM;
> > +	fc->s_iflags |= SB_I_NOEXEC;
> > +	fc->s_iflags |= SB_I_NODEV;
> >  	ctx->dops = &anon_inodefs_dentry_operations;
> >  	return 0;
> >  }
> > 
> > -- 
> > 2.47.2
> > 
> -- 
> Jan Kara <jack@suse.com>
> SUSE Labs, CR

Re: [PATCH 1/9] anon_inode: use a proper mode internally

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 5562 bytes --]

On Mon, Apr 07, 2025 at 11:54:15AM +0200, Christian Brauner wrote:
> This allows the VFS to not trip over anonymous inodes and we can add
> asserts based on the mode into the vfs. When we report it to userspace
> we can simply hide the mode to avoid regressions. I've audited all
> direct callers of alloc_anon_inode() and only secretmen overrides i_mode
> and i_op inode operations but it already uses a regular file.

We've been seeing failures in LTP's readadead01 in -next on arm64
platforms:

 4601 07:43:36.192033  tst_test.c:1900: TINFO: LTP version: 20250130-1-g60fe84aaf
 4602 07:43:36.201811  tst_test.c:1904: TINFO: Tested kernel: 6.15.0-rc1-next-20250410 #1 SMP PREEMPT Thu Apr 10 06:18:38 UTC 2025 aarch64
 4603 07:43:36.208400  tst_kconfig.c:88: TINFO: Parsing kernel config '/proc/config.gz'
 4604 07:43:36.218393  tst_test.c:1722: TINFO: Overall timeout per run is 0h 01m 30s
 4605 07:43:36.223886  readahead01.c:36: TPASS: readahead() with fd = -1 : EBADF (9)
 4606 07:43:36.229370  readahead01.c:43: TPASS: readahead() with invalid fd : EBADF (9)
 4607 07:43:36.234998  readahead01.c:64: TPASS: readahead() on O_PATH file : EBADF (9)
 4608 07:43:36.240527  readahead01.c:64: TPASS: readahead() on directory : EINVAL (22)
 4609 07:43:36.246118  readahead01.c:64: TPASS: readahead() on /dev/zero : EINVAL (22)
 4610 07:43:36.251530  readahead01.c:64: TPASS: readahead() on pipe read end : EINVAL (22)
 4611 07:43:36.260007  readahead01.c:64: TPASS: readahead() on pipe write end : EBADF (9)
 4612 07:43:36.265581  readahead01.c:64: TPASS: readahead() on unix socket : EINVAL (22)
 4613 07:43:36.270928  readahead01.c:64: TPASS: readahead() on inet socket : EINVAL (22)
 4614 07:43:36.276754  readahead01.c:64: TFAIL: readahead() on epoll succeeded
 4615 07:43:36.279460  readahead01.c:64: TFAIL: readahead() on eventfd succeeded
 4616 07:43:36.285053  readahead01.c:64: TFAIL: readahead() on signalfd succeeded
 4617 07:43:36.290504  readahead01.c:64: TFAIL: readahead() on timerfd succeeded
 4618 07:43:36.296220  readahead01.c:64: TFAIL: readahead() on fanotify succeeded
 4619 07:43:36.301605  readahead01.c:64: TFAIL: readahead() on inotify succeeded
 4620 07:43:36.307327  tst_fd.c:170: TCONF: Skipping userfaultfd: ENOSYS (38)
 4621 07:43:36.312806  readahead01.c:64: TFAIL: readahead() on perf event succeeded
 4622 07:43:36.318534  readahead01.c:64: TFAIL: readahead() on io uring succeeded
 4623 07:43:36.321511  readahead01.c:64: TFAIL: readahead() on bpf map succeeded
 4624 07:43:36.325711  readahead01.c:64: TFAIL: readahead() on fsopen succeeded
 4625 07:43:36.331073  readahead01.c:64: TFAIL: readahead() on fspick succeeded
 4626 07:43:36.336608  readahead01.c:64: TPASS: readahead() on open_tree : EBADF (9)
 4627 07:43:36.336903  
 4628 07:43:36.339354  Summary:
 4629 07:43:36.339641  passed   10
 4630 07:43:36.342132  failed   11
 4631 07:43:36.342420  broken   0
 4632 07:43:36.342648  skipped  1
 4633 07:43:36.344768  warnings 0

which bisected down to this patch, which is cfd86ef7e8e7b9e01 in -next:

git bisect start
# status: waiting for both good and bad commits
# bad: [29e7bf01ed8033c9a14ed0dc990dfe2736dbcd18] Add linux-next specific files for 20250410
git bisect bad 29e7bf01ed8033c9a14ed0dc990dfe2736dbcd18
# status: waiting for good commit(s), bad commit known
# good: [1785a3a7b96a52fae13880a5ba880a5f473eacb1] Merge branch 'for-linux-next-fixes' of https://gitlab.freedesktop.org/drm/misc/kernel.git
git bisect good 1785a3a7b96a52fae13880a5ba880a5f473eacb1
# bad: [793874436825ebf3dfeeac34b75682c234cf61ef] Merge branch 'for-linux-next' of https://gitlab.freedesktop.org/drm/misc/kernel.git
git bisect bad 793874436825ebf3dfeeac34b75682c234cf61ef
# bad: [f8b5c1664191e453611f77d36ba21b09bc468a2d] Merge branch 'next' of git://git.kernel.org/pub/scm/linux/kernel/git/pci/pci.git
git bisect bad f8b5c1664191e453611f77d36ba21b09bc468a2d
# good: [100ac6e209fce471f3ff4d4e92f9d192fcfa7637] Merge branch 'for-next' of git://git.kernel.org/pub/scm/linux/kernel/git/qcom/linux.git
git bisect good 100ac6e209fce471f3ff4d4e92f9d192fcfa7637
# bad: [143ced925e31fe24e820866403276492f05efaa5] Merge branch 'vfs.all' of git://git.kernel.org/pub/scm/linux/kernel/git/vfs/vfs.git
git bisect bad 143ced925e31fe24e820866403276492f05efaa5
# good: [b087fb728fdda75e1d3e83aa542d3aa025ac6c4a] Merge branch 'nfsd-next' of git://git.kernel.org/pub/scm/linux/kernel/git/cel/linux
git bisect good b087fb728fdda75e1d3e83aa542d3aa025ac6c4a
# good: [7ee85aeee98e85f72a663672267180218d1510db] Merge branch 'vfs-6.16.super' into vfs.all
git bisect good 7ee85aeee98e85f72a663672267180218d1510db
# bad: [d57e6ea6671b1ef0fcb09ccc52952c8a6bfb83c8] Merge branch 'vfs-6.16.misc' into vfs.all
git bisect bad d57e6ea6671b1ef0fcb09ccc52952c8a6bfb83c8
# bad: [25a6cc9a630b4b1b783903b23a3a97c5bf16bf41] selftests/filesystems: add open() test for anonymous inodes
git bisect bad 25a6cc9a630b4b1b783903b23a3a97c5bf16bf41
# bad: [c83b9024966090fe0df92aab16975b8d00089e1f] pidfs: use anon_inode_setattr()
git bisect bad c83b9024966090fe0df92aab16975b8d00089e1f
# bad: [cfd86ef7e8e7b9e015707e46479a6b1de141eed0] anon_inode: use a proper mode internally
git bisect bad cfd86ef7e8e7b9e015707e46479a6b1de141eed0
# good: [418556fa576ebbd644c7258a97b33203956ea232] docs: initramfs: update compression and mtime descriptions
git bisect good 418556fa576ebbd644c7258a97b33203956ea232
# first bad commit: [cfd86ef7e8e7b9e015707e46479a6b1de141eed0] anon_inode: use a proper mode internally

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH 1/9] anon_inode: use a proper mode internally

[Christian Brauner]

On Fri, Apr 11, 2025 at 11:31:24AM +0100, Mark Brown wrote:
> On Mon, Apr 07, 2025 at 11:54:15AM +0200, Christian Brauner wrote:
> > This allows the VFS to not trip over anonymous inodes and we can add
> > asserts based on the mode into the vfs. When we report it to userspace
> > we can simply hide the mode to avoid regressions. I've audited all
> > direct callers of alloc_anon_inode() and only secretmen overrides i_mode
> > and i_op inode operations but it already uses a regular file.
> 
> We've been seeing failures in LTP's readadead01 in -next on arm64
> platforms:

This fscking readhead garbage is driving me insane.
Ok, readahead skipped anonymous inodes because it's checking whether it
is a regular file or not. We now make them regular files internally.
Should be fixed in -next tomorrow.

> 
>  4601 07:43:36.192033  tst_test.c:1900: TINFO: LTP version: 20250130-1-g60fe84aaf
>  4602 07:43:36.201811  tst_test.c:1904: TINFO: Tested kernel: 6.15.0-rc1-next-20250410 #1 SMP PREEMPT Thu Apr 10 06:18:38 UTC 2025 aarch64
>  4603 07:43:36.208400  tst_kconfig.c:88: TINFO: Parsing kernel config '/proc/config.gz'
>  4604 07:43:36.218393  tst_test.c:1722: TINFO: Overall timeout per run is 0h 01m 30s
>  4605 07:43:36.223886  readahead01.c:36: TPASS: readahead() with fd = -1 : EBADF (9)
>  4606 07:43:36.229370  readahead01.c:43: TPASS: readahead() with invalid fd : EBADF (9)
>  4607 07:43:36.234998  readahead01.c:64: TPASS: readahead() on O_PATH file : EBADF (9)
>  4608 07:43:36.240527  readahead01.c:64: TPASS: readahead() on directory : EINVAL (22)
>  4609 07:43:36.246118  readahead01.c:64: TPASS: readahead() on /dev/zero : EINVAL (22)
>  4610 07:43:36.251530  readahead01.c:64: TPASS: readahead() on pipe read end : EINVAL (22)
>  4611 07:43:36.260007  readahead01.c:64: TPASS: readahead() on pipe write end : EBADF (9)
>  4612 07:43:36.265581  readahead01.c:64: TPASS: readahead() on unix socket : EINVAL (22)
>  4613 07:43:36.270928  readahead01.c:64: TPASS: readahead() on inet socket : EINVAL (22)
>  4614 07:43:36.276754  readahead01.c:64: TFAIL: readahead() on epoll succeeded
>  4615 07:43:36.279460  readahead01.c:64: TFAIL: readahead() on eventfd succeeded
>  4616 07:43:36.285053  readahead01.c:64: TFAIL: readahead() on signalfd succeeded
>  4617 07:43:36.290504  readahead01.c:64: TFAIL: readahead() on timerfd succeeded
>  4618 07:43:36.296220  readahead01.c:64: TFAIL: readahead() on fanotify succeeded
>  4619 07:43:36.301605  readahead01.c:64: TFAIL: readahead() on inotify succeeded
>  4620 07:43:36.307327  tst_fd.c:170: TCONF: Skipping userfaultfd: ENOSYS (38)
>  4621 07:43:36.312806  readahead01.c:64: TFAIL: readahead() on perf event succeeded
>  4622 07:43:36.318534  readahead01.c:64: TFAIL: readahead() on io uring succeeded
>  4623 07:43:36.321511  readahead01.c:64: TFAIL: readahead() on bpf map succeeded
>  4624 07:43:36.325711  readahead01.c:64: TFAIL: readahead() on fsopen succeeded
>  4625 07:43:36.331073  readahead01.c:64: TFAIL: readahead() on fspick succeeded
>  4626 07:43:36.336608  readahead01.c:64: TPASS: readahead() on open_tree : EBADF (9)
>  4627 07:43:36.336903  
>  4628 07:43:36.339354  Summary:
>  4629 07:43:36.339641  passed   10
>  4630 07:43:36.342132  failed   11
>  4631 07:43:36.342420  broken   0
>  4632 07:43:36.342648  skipped  1
>  4633 07:43:36.344768  warnings 0
> 
> which bisected down to this patch, which is cfd86ef7e8e7b9e01 in -next:
> 
> git bisect start
> # status: waiting for both good and bad commits
> # bad: [29e7bf01ed8033c9a14ed0dc990dfe2736dbcd18] Add linux-next specific files for 20250410
> git bisect bad 29e7bf01ed8033c9a14ed0dc990dfe2736dbcd18
> # status: waiting for good commit(s), bad commit known
> # good: [1785a3a7b96a52fae13880a5ba880a5f473eacb1] Merge branch 'for-linux-next-fixes' of https://gitlab.freedesktop.org/drm/misc/kernel.git
> git bisect good 1785a3a7b96a52fae13880a5ba880a5f473eacb1
> # bad: [793874436825ebf3dfeeac34b75682c234cf61ef] Merge branch 'for-linux-next' of https://gitlab.freedesktop.org/drm/misc/kernel.git
> git bisect bad 793874436825ebf3dfeeac34b75682c234cf61ef
> # bad: [f8b5c1664191e453611f77d36ba21b09bc468a2d] Merge branch 'next' of git://git.kernel.org/pub/scm/linux/kernel/git/pci/pci.git
> git bisect bad f8b5c1664191e453611f77d36ba21b09bc468a2d
> # good: [100ac6e209fce471f3ff4d4e92f9d192fcfa7637] Merge branch 'for-next' of git://git.kernel.org/pub/scm/linux/kernel/git/qcom/linux.git
> git bisect good 100ac6e209fce471f3ff4d4e92f9d192fcfa7637
> # bad: [143ced925e31fe24e820866403276492f05efaa5] Merge branch 'vfs.all' of git://git.kernel.org/pub/scm/linux/kernel/git/vfs/vfs.git
> git bisect bad 143ced925e31fe24e820866403276492f05efaa5
> # good: [b087fb728fdda75e1d3e83aa542d3aa025ac6c4a] Merge branch 'nfsd-next' of git://git.kernel.org/pub/scm/linux/kernel/git/cel/linux
> git bisect good b087fb728fdda75e1d3e83aa542d3aa025ac6c4a
> # good: [7ee85aeee98e85f72a663672267180218d1510db] Merge branch 'vfs-6.16.super' into vfs.all
> git bisect good 7ee85aeee98e85f72a663672267180218d1510db
> # bad: [d57e6ea6671b1ef0fcb09ccc52952c8a6bfb83c8] Merge branch 'vfs-6.16.misc' into vfs.all
> git bisect bad d57e6ea6671b1ef0fcb09ccc52952c8a6bfb83c8
> # bad: [25a6cc9a630b4b1b783903b23a3a97c5bf16bf41] selftests/filesystems: add open() test for anonymous inodes
> git bisect bad 25a6cc9a630b4b1b783903b23a3a97c5bf16bf41
> # bad: [c83b9024966090fe0df92aab16975b8d00089e1f] pidfs: use anon_inode_setattr()
> git bisect bad c83b9024966090fe0df92aab16975b8d00089e1f
> # bad: [cfd86ef7e8e7b9e015707e46479a6b1de141eed0] anon_inode: use a proper mode internally
> git bisect bad cfd86ef7e8e7b9e015707e46479a6b1de141eed0
> # good: [418556fa576ebbd644c7258a97b33203956ea232] docs: initramfs: update compression and mtime descriptions
> git bisect good 418556fa576ebbd644c7258a97b33203956ea232
> # first bad commit: [cfd86ef7e8e7b9e015707e46479a6b1de141eed0] anon_inode: use a proper mode internally

Re: [PATCH 1/9] anon_inode: use a proper mode internally

[Christoph Hellwig]

On Fri, Apr 11, 2025 at 05:03:55PM +0200, Christian Brauner wrote:
> On Fri, Apr 11, 2025 at 11:31:24AM +0100, Mark Brown wrote:
> > On Mon, Apr 07, 2025 at 11:54:15AM +0200, Christian Brauner wrote:
> > > This allows the VFS to not trip over anonymous inodes and we can add
> > > asserts based on the mode into the vfs. When we report it to userspace
> > > we can simply hide the mode to avoid regressions. I've audited all
> > > direct callers of alloc_anon_inode() and only secretmen overrides i_mode
> > > and i_op inode operations but it already uses a regular file.
> > 
> > We've been seeing failures in LTP's readadead01 in -next on arm64
> > platforms:
> 
> This fscking readhead garbage is driving me insane.
> Ok, readahead skipped anonymous inodes because it's checking whether it
> is a regular file or not. We now make them regular files internally.
> Should be fixed in -next tomorrow.

Is this the readahead syscall?  Yeah that random check in the high level
code looks odd if that's what is being triggered here.

Re: [PATCH 1/9] anon_inode: use a proper mode internally

[Xilin Wu]

On 2025/4/7 17:54:15, Christian Brauner wrote:
> This allows the VFS to not trip over anonymous inodes and we can add
> asserts based on the mode into the vfs. When we report it to userspace
> we can simply hide the mode to avoid regressions. I've audited all
> direct callers of alloc_anon_inode() and only secretmen overrides i_mode
> and i_op inode operations but it already uses a regular file.
> 
> Fixes: af153bb63a336 ("vfs: catch invalid modes in may_open()")
> Cc: <stable@vger.kernel.org> # all LTS kernels
> Reported-by: syzbot+5d8e79d323a13aa0b248@syzkaller.appspotmail.com
> Closes: https://lore.kernel.org/all/67ed3fb3.050a0220.14623d.0009.GAE@google.com
> Signed-off-by: Christian Brauner <brauner@kernel.org>

Hi Christian and FSdevel list,

I'm reporting a regression introduced in the linux-next tree, which I've 
tracked down using `git bisect` to this commit.

Starting with `linux-next` tag `next-20250408` (up to the latest 
`next-20250417` I tested), attempting to start an SDDM Wayland session 
(using KDE Plasma's KWin) results in a black screen. The `kwin_wayland` 
and `sddm-helper-start-wayland` processes both enter a state consuming 
100% CPU on a single core indefinitely, preventing the login screen from 
appearing. The last known good kernel is `next-20250407`.

Using `strace` on the `kwin_wayland` process revealed it's stuck in a 
tight loop involving `ppoll` and `ioctl`:

```
ioctl(29, FIONREAD, [0])                = 0
ppoll([{fd=10, events=POLLIN}, {fd=37, events=POLLIN}, {fd=9, 
events=POLLIN}, {fd=5, events=POLLIN}, {fd=29, events=POLLIN}, {fd=17, 
events=POLLIN}, {fd=3, events=POLLIN}], 7, NULL, NULL, 8) = 1 ([{fd=29, 
revents=POLLIN}])
ioctl(29, FIONREAD, [0])                = 0
ppoll([{fd=10, events=POLLIN}, {fd=37, events=POLLIN}, {fd=9, 
events=POLLIN}, {fd=5, events=POLLIN}, {fd=29, events=POLLIN}, {fd=17, 
events=POLLIN}, {fd=3, events=POLLIN}], 7, NULL, NULL, 8) = 1 ([{fd=29, 
revents=POLLIN}])
# ... this repeats indefinitely at high frequency
```

Initially, I suspected a DRM issue, but checking the file descriptor 
confirmed it's an `inotify` instance:

```
$ sudo ls -l /proc/<kwin_pid>/fd/29
lr-x------ 1 sddm sddm 64 Apr 17 14:03 /proc/<kwin_pid>/fd/29 -> 
anon_inode:inotify
```

Reverting this commit resolves the issue, allowing SDDM and KWin Wayland 
to start normally.

Could you please take a look at this? Let me know if you need any 
further information or testing from my side.

-- 
Best regards,
Xilin Wu <sophon@radxa.com>

Re: [PATCH 1/9] anon_inode: use a proper mode internally

[Christian Brauner]

On Fri, Apr 18, 2025 at 10:15:01AM +0800, Xilin Wu wrote:
> On 2025/4/7 17:54:15, Christian Brauner wrote:
> > This allows the VFS to not trip over anonymous inodes and we can add
> > asserts based on the mode into the vfs. When we report it to userspace
> > we can simply hide the mode to avoid regressions. I've audited all
> > direct callers of alloc_anon_inode() and only secretmen overrides i_mode
> > and i_op inode operations but it already uses a regular file.
> > 
> > Fixes: af153bb63a336 ("vfs: catch invalid modes in may_open()")
> > Cc: <stable@vger.kernel.org> # all LTS kernels
> > Reported-by: syzbot+5d8e79d323a13aa0b248@syzkaller.appspotmail.com
> > Closes: https://lore.kernel.org/all/67ed3fb3.050a0220.14623d.0009.GAE@google.com
> > Signed-off-by: Christian Brauner <brauner@kernel.org>
> 
> Hi Christian and FSdevel list,
> 
> I'm reporting a regression introduced in the linux-next tree, which I've
> tracked down using `git bisect` to this commit.
> 
> Starting with `linux-next` tag `next-20250408` (up to the latest
> `next-20250417` I tested), attempting to start an SDDM Wayland session
> (using KDE Plasma's KWin) results in a black screen. The `kwin_wayland` and
> `sddm-helper-start-wayland` processes both enter a state consuming 100% CPU
> on a single core indefinitely, preventing the login screen from appearing.
> The last known good kernel is `next-20250407`.
> 
> Using `strace` on the `kwin_wayland` process revealed it's stuck in a tight
> loop involving `ppoll` and `ioctl`:
> 
> ```
> ioctl(29, FIONREAD, [0])                = 0
> ppoll([{fd=10, events=POLLIN}, {fd=37, events=POLLIN}, {fd=9,
> events=POLLIN}, {fd=5, events=POLLIN}, {fd=29, events=POLLIN}, {fd=17,
> events=POLLIN}, {fd=3, events=POLLIN}], 7, NULL, NULL, 8) = 1 ([{fd=29,
> revents=POLLIN}])
> ioctl(29, FIONREAD, [0])                = 0
> ppoll([{fd=10, events=POLLIN}, {fd=37, events=POLLIN}, {fd=9,
> events=POLLIN}, {fd=5, events=POLLIN}, {fd=29, events=POLLIN}, {fd=17,
> events=POLLIN}, {fd=3, events=POLLIN}], 7, NULL, NULL, 8) = 1 ([{fd=29,
> revents=POLLIN}])
> # ... this repeats indefinitely at high frequency
> ```
> 
> Initially, I suspected a DRM issue, but checking the file descriptor
> confirmed it's an `inotify` instance:
> 
> ```
> $ sudo ls -l /proc/<kwin_pid>/fd/29
> lr-x------ 1 sddm sddm 64 Apr 17 14:03 /proc/<kwin_pid>/fd/29 ->
> anon_inode:inotify
> ```
> 
> Reverting this commit resolves the issue, allowing SDDM and KWin Wayland to
> start normally.
> 
> Could you please take a look at this? Let me know if you need any further
> information or testing from my side.

I'll take a look latest Tuesday.

Re: [PATCH 1/9] anon_inode: use a proper mode internally

[Christian Brauner]

On Sun, Apr 20, 2025 at 12:54:29PM +0200, Christian Brauner wrote:
> On Fri, Apr 18, 2025 at 10:15:01AM +0800, Xilin Wu wrote:
> > On 2025/4/7 17:54:15, Christian Brauner wrote:
> > > This allows the VFS to not trip over anonymous inodes and we can add
> > > asserts based on the mode into the vfs. When we report it to userspace
> > > we can simply hide the mode to avoid regressions. I've audited all
> > > direct callers of alloc_anon_inode() and only secretmen overrides i_mode
> > > and i_op inode operations but it already uses a regular file.
> > > 
> > > Fixes: af153bb63a336 ("vfs: catch invalid modes in may_open()")
> > > Cc: <stable@vger.kernel.org> # all LTS kernels
> > > Reported-by: syzbot+5d8e79d323a13aa0b248@syzkaller.appspotmail.com
> > > Closes: https://lore.kernel.org/all/67ed3fb3.050a0220.14623d.0009.GAE@google.com
> > > Signed-off-by: Christian Brauner <brauner@kernel.org>
> > 
> > Hi Christian and FSdevel list,
> > 
> > I'm reporting a regression introduced in the linux-next tree, which I've
> > tracked down using `git bisect` to this commit.
> > 
> > Starting with `linux-next` tag `next-20250408` (up to the latest
> > `next-20250417` I tested), attempting to start an SDDM Wayland session
> > (using KDE Plasma's KWin) results in a black screen. The `kwin_wayland` and
> > `sddm-helper-start-wayland` processes both enter a state consuming 100% CPU
> > on a single core indefinitely, preventing the login screen from appearing.
> > The last known good kernel is `next-20250407`.
> > 
> > Using `strace` on the `kwin_wayland` process revealed it's stuck in a tight
> > loop involving `ppoll` and `ioctl`:
> > 
> > ```
> > ioctl(29, FIONREAD, [0])                = 0
> > ppoll([{fd=10, events=POLLIN}, {fd=37, events=POLLIN}, {fd=9,
> > events=POLLIN}, {fd=5, events=POLLIN}, {fd=29, events=POLLIN}, {fd=17,
> > events=POLLIN}, {fd=3, events=POLLIN}], 7, NULL, NULL, 8) = 1 ([{fd=29,
> > revents=POLLIN}])
> > ioctl(29, FIONREAD, [0])                = 0
> > ppoll([{fd=10, events=POLLIN}, {fd=37, events=POLLIN}, {fd=9,
> > events=POLLIN}, {fd=5, events=POLLIN}, {fd=29, events=POLLIN}, {fd=17,
> > events=POLLIN}, {fd=3, events=POLLIN}], 7, NULL, NULL, 8) = 1 ([{fd=29,
> > revents=POLLIN}])
> > # ... this repeats indefinitely at high frequency
> > ```
> > 
> > Initially, I suspected a DRM issue, but checking the file descriptor
> > confirmed it's an `inotify` instance:
> > 
> > ```
> > $ sudo ls -l /proc/<kwin_pid>/fd/29
> > lr-x------ 1 sddm sddm 64 Apr 17 14:03 /proc/<kwin_pid>/fd/29 ->
> > anon_inode:inotify
> > ```
> > 
> > Reverting this commit resolves the issue, allowing SDDM and KWin Wayland to
> > start normally.
> > 
> > Could you please take a look at this? Let me know if you need any further
> > information or testing from my side.
> 
> I'll take a look latest Tuesday.

Fixed. We need to ensure that we call the ioctl handler for anonymous
inodes and not the generic FIONREAD call.