Linux filesystem development
 help / color / mirror / Atom feed
* [PATCH 0/4] configfs: add a basic selftest
@ 2026-08-26 10:47 Breno Leitao
  2026-08-26 10:47 ` [PATCH 1/4] selftests/configfs: add tests for the userspace interface Breno Leitao
                   ` (3 more replies)
  0 siblings, 4 replies; 5+ messages in thread
From: Breno Leitao @ 2026-08-26 10:47 UTC (permalink / raw)
  To: Shuah Khan, Andreas Hindborg
  Cc: linux-kernel, linux-kselftest, gustavold, linux-fsdevel,
	Breno Leitao, kernel-team, vasilisalmpanis

configfs has had no selftest since day one, which makes maintaining it
harder than it needs to be.

The good news is that it is not hard to test: nearly all of it is
reachable from userspace through mkdir/rmdir/read/write and symlink(2).
This series leverages the sample configfs module to do exactly that.

The sample implements no ->allow_link(), so patch 2 adds a subsystem that
accepts symlinks and patches 3 and 4 build the symlink coverage on it.

Patch 4 races symlink(2) against rmdir(2) of the target and reproduces the
use-after-free syzbot reported, so it fails until these patch land:

  1507d5b57e40 ("configfs: pin the symlink target's dirent instead of chasing ->ci_dentry")
  2251d0ed97c2 ("configfs: unhash the dentry before dropping the item in rmdir")

Both are in configfs-next.  The first trips the s_count WARN_ON()
in configfs_get(), the second the KASAN use-after-free in
config_item_get().

Signed-off-by: Breno Leitao <leitao@debian.org>
---
Breno Leitao (4):
      selftests/configfs: add tests for the userspace interface
      samples/configfs: add a subsystem that accepts symlinks
      selftests/configfs: cover symlink and unlink
      selftests/configfs: race symlink against rmdir of the target

 MAINTAINERS                                        |   1 +
 samples/configfs/configfs_sample.c                 | 121 ++++++
 tools/testing/selftests/Makefile                   |   1 +
 .../selftests/filesystems/configfs/.gitignore      |   2 +
 .../selftests/filesystems/configfs/Makefile        |   8 +
 .../testing/selftests/filesystems/configfs/config  |   5 +
 .../selftests/filesystems/configfs/configfs_test.c | 481 +++++++++++++++++++++
 7 files changed, 619 insertions(+)
---
base-commit: a8406e6c0b793ce0788019683837c40855b55995
change-id: 20260826-config_selftest-9d84a5b1ec26

Best regards,
--  
Breno Leitao <leitao@debian.org>


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH 1/4] selftests/configfs: add tests for the userspace interface
  2026-08-26 10:47 [PATCH 0/4] configfs: add a basic selftest Breno Leitao
@ 2026-08-26 10:47 ` Breno Leitao
  2026-08-26 10:47 ` [PATCH 2/4] samples/configfs: add a subsystem that accepts symlinks Breno Leitao
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 5+ messages in thread
From: Breno Leitao @ 2026-08-26 10:47 UTC (permalink / raw)
  To: Shuah Khan, Andreas Hindborg
  Cc: linux-kernel, linux-kselftest, gustavold, linux-fsdevel,
	Breno Leitao, kernel-team, vasilisalmpanis

configfs has no selftest coverage.  Its userspace ABI is entirely
mkdir/rmdir/read/write on attribute files, and the error each of those
returns is part of the contract.

Add tests driving the subsystems samples/configfs registers, each in its
own mount namespace on a private configfs mount.

Signed-off-by: Breno Leitao <leitao@debian.org>
---
 MAINTAINERS                                        |   1 +
 tools/testing/selftests/Makefile                   |   1 +
 .../selftests/filesystems/configfs/.gitignore      |   2 +
 .../selftests/filesystems/configfs/Makefile        |   8 +
 .../testing/selftests/filesystems/configfs/config  |   5 +
 .../selftests/filesystems/configfs/configfs_test.c | 338 +++++++++++++++++++++
 6 files changed, 355 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index fe10d29b7657b..69aea9931b774 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -6604,6 +6604,7 @@ S:	Supported
 F:	fs/configfs/
 F:	include/linux/configfs.h
 F:	samples/configfs/
+F:	tools/testing/selftests/filesystems/configfs/
 
 CONFIGFS [RUST]
 M:	Andreas Hindborg <a.hindborg@kernel.org>
diff --git a/tools/testing/selftests/Makefile b/tools/testing/selftests/Makefile
index 2d960626750e3..b328a235f1295 100644
--- a/tools/testing/selftests/Makefile
+++ b/tools/testing/selftests/Makefile
@@ -34,6 +34,7 @@ TARGETS += exec
 TARGETS += fchmodat2
 TARGETS += filesystems
 TARGETS += filesystems/binderfs
+TARGETS += filesystems/configfs
 TARGETS += filesystems/epoll
 TARGETS += filesystems/failfs
 TARGETS += filesystems/fat
diff --git a/tools/testing/selftests/filesystems/configfs/.gitignore b/tools/testing/selftests/filesystems/configfs/.gitignore
new file mode 100644
index 0000000000000..accfb6bb48268
--- /dev/null
+++ b/tools/testing/selftests/filesystems/configfs/.gitignore
@@ -0,0 +1,2 @@
+# SPDX-License-Identifier: GPL-2.0-only
+configfs_test
diff --git a/tools/testing/selftests/filesystems/configfs/Makefile b/tools/testing/selftests/filesystems/configfs/Makefile
new file mode 100644
index 0000000000000..359296356c831
--- /dev/null
+++ b/tools/testing/selftests/filesystems/configfs/Makefile
@@ -0,0 +1,8 @@
+# SPDX-License-Identifier: GPL-2.0
+# Copyright (c) 2026 Meta Platforms, Inc. and affiliates
+# Copyright (c) 2026 Breno Leitao <leitao@debian.org>
+
+CFLAGS += -Wall -Werror
+TEST_GEN_PROGS := configfs_test
+
+include ../../lib.mk
diff --git a/tools/testing/selftests/filesystems/configfs/config b/tools/testing/selftests/filesystems/configfs/config
new file mode 100644
index 0000000000000..5ea17df535b3f
--- /dev/null
+++ b/tools/testing/selftests/filesystems/configfs/config
@@ -0,0 +1,5 @@
+CONFIG_CONFIGFS_FS=y
+CONFIG_MODULES=y
+CONFIG_MODULE_UNLOAD=y
+CONFIG_SAMPLES=y
+CONFIG_SAMPLE_CONFIGFS=m
diff --git a/tools/testing/selftests/filesystems/configfs/configfs_test.c b/tools/testing/selftests/filesystems/configfs/configfs_test.c
new file mode 100644
index 0000000000000..072d0dcf3f96b
--- /dev/null
+++ b/tools/testing/selftests/filesystems/configfs/configfs_test.c
@@ -0,0 +1,338 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Exercise the configfs userspace interface through the three subsystems
+ * registered by samples/configfs.
+ *
+ * Copyright (c) 2026 Meta Platforms, Inc. and affiliates
+ * Copyright (c) 2026 Breno Leitao <leitao@debian.org>
+ */
+#define _GNU_SOURCE
+
+#include <errno.h>
+#include <fcntl.h>
+#include <sched.h>
+#include <stdbool.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <sys/mount.h>
+#include <sys/stat.h>
+#include <sys/syscall.h>
+#include <sys/vfs.h>
+#include <unistd.h>
+
+#include "kselftest_harness.h"
+
+/* Private to fs/configfs/mount.c. */
+#define CONFIGFS_MAGIC		0x62656570
+
+#define SAMPLE_MODULE		"configfs_sample"
+
+#define CHILDLESS		"01-childless"
+#define SIMPLE			"02-simple-children"
+#define GROUPS			"03-group-children"
+
+#define ITEM_A			SIMPLE "/kselftest-a"
+#define ITEM_B			SIMPLE "/kselftest-b"
+#define GROUP			GROUPS "/kselftest-group"
+#define GROUP_ITEM		GROUP "/kselftest-a"
+
+/* Deepest first, so one pass empties the tree. */
+static const char * const test_dirs[] = {
+	GROUP_ITEM,
+	GROUP,
+	ITEM_A,
+	ITEM_B,
+};
+
+static void drop_test_dirs(void)
+{
+	size_t i;
+
+	for (i = 0; i < ARRAY_SIZE(test_dirs); i++)
+		rmdir(test_dirs[i]);
+}
+
+static ssize_t read_attr(const char *path, char *buf, size_t len)
+{
+	ssize_t ret;
+	int fd;
+
+	fd = open(path, O_RDONLY);
+	if (fd < 0)
+		return -1;
+
+	ret = read(fd, buf, len - 1);
+	close(fd);
+	if (ret < 0)
+		return -1;
+
+	buf[ret] = '\0';
+	return ret;
+}
+
+static ssize_t write_attr(const char *path, const char *val)
+{
+	ssize_t ret;
+	int fd, err;
+
+	fd = open(path, O_WRONLY);
+	if (fd < 0)
+		return -1;
+
+	ret = write(fd, val, strlen(val));
+	err = errno;
+	close(fd);
+	errno = err;
+
+	return ret;
+}
+
+FIXTURE(configfs) {
+	char mnt[sizeof(P_tmpdir "/configfs_XXXXXX")];
+	bool mounted;
+};
+
+FIXTURE_SETUP(configfs)
+{
+	char tmpl[] = P_tmpdir "/configfs_XXXXXX";
+
+	if (geteuid())
+		SKIP(return, "need root to load modules and mount configfs");
+
+	ASSERT_EQ(system("modprobe -q " SAMPLE_MODULE), 0)
+		TH_LOG(SAMPLE_MODULE " missing, is CONFIG_SAMPLE_CONFIGFS=m?");
+
+	ASSERT_EQ(unshare(CLONE_NEWNS), 0);
+	ASSERT_EQ(mount(NULL, "/", NULL, MS_REC | MS_PRIVATE, NULL), 0);
+
+	ASSERT_NE(mkdtemp(tmpl), NULL);
+	strcpy(self->mnt, tmpl);
+
+	ASSERT_EQ(mount("configfs", self->mnt, "configfs", 0, NULL), 0);
+	ASSERT_EQ(chdir(self->mnt), 0);
+	self->mounted = true;
+
+	/* configfs items outlive the mount, so a killed run leaves some. */
+	drop_test_dirs();
+}
+
+FIXTURE_TEARDOWN(configfs)
+{
+	if (self->mounted) {
+		drop_test_dirs();
+		EXPECT_EQ(chdir("/"), 0);
+		EXPECT_EQ(umount2(self->mnt, MNT_DETACH), 0);
+	}
+
+	if (self->mnt[0])
+		EXPECT_EQ(rmdir(self->mnt), 0);
+}
+
+TEST_F(configfs, mount_and_subsystems)
+{
+	const char * const subsys[] = { CHILDLESS, SIMPLE, GROUPS };
+	struct statfs sfs;
+	struct stat st;
+	size_t i;
+
+	ASSERT_EQ(statfs(".", &sfs), 0);
+	EXPECT_EQ(sfs.f_type, CONFIGFS_MAGIC);
+
+	for (i = 0; i < ARRAY_SIZE(subsys); i++) {
+		ASSERT_EQ(stat(subsys[i], &st), 0)
+			TH_LOG("%s is missing", subsys[i]);
+		EXPECT_TRUE(S_ISDIR(st.st_mode));
+	}
+}
+
+TEST_F(configfs, mkdir_at_root)
+{
+	/* The root has no ->mkdir(); only subsystems register there. */
+	ASSERT_EQ(mkdir("kselftest-root", 0755), -1);
+	EXPECT_EQ(errno, EPERM);
+}
+
+TEST_F(configfs, rmdir_subsystem)
+{
+	ASSERT_EQ(rmdir(CHILDLESS), -1);
+	EXPECT_EQ(errno, EPERM);
+}
+
+TEST_F(configfs, mkdir_without_group_ops)
+{
+	/* 01-childless has attributes but no ->make_item()/->make_group(). */
+	ASSERT_EQ(mkdir(CHILDLESS "/kselftest-a", 0755), -1);
+	EXPECT_EQ(errno, EPERM);
+}
+
+TEST_F(configfs, attr_store_and_show)
+{
+	char buf[64];
+
+	ASSERT_GT(write_attr(CHILDLESS "/storeme", "42"), 0);
+	ASSERT_GT(read_attr(CHILDLESS "/storeme", buf, sizeof(buf)), 0);
+	EXPECT_STREQ(buf, "42\n");
+}
+
+TEST_F(configfs, attr_store_rejects_garbage)
+{
+	ASSERT_EQ(write_attr(CHILDLESS "/storeme", "not-a-number"), -1);
+	EXPECT_EQ(errno, EINVAL);
+}
+
+TEST_F(configfs, attr_show_runs_on_every_open)
+{
+	char first[64], second[64];
+
+	/* 01-childless/showme increments the value it just returned. */
+	ASSERT_GT(read_attr(CHILDLESS "/showme", first, sizeof(first)), 0);
+	ASSERT_GT(read_attr(CHILDLESS "/showme", second, sizeof(second)), 0);
+	EXPECT_EQ(atoi(second), atoi(first) + 1);
+}
+
+TEST_F(configfs, attr_read_only)
+{
+	ASSERT_EQ(open(CHILDLESS "/description", O_WRONLY), -1);
+	EXPECT_EQ(errno, EACCES);
+}
+
+TEST_F(configfs, attr_unlink)
+{
+	/* ->unlink() only accepts the symlinks configfs itself created. */
+	ASSERT_EQ(unlink(CHILDLESS "/storeme"), -1);
+	EXPECT_EQ(errno, EPERM);
+}
+
+TEST_F(configfs, attr_read_length)
+{
+	char buf[8192];
+	struct stat st;
+	ssize_t n;
+	int fd;
+
+	fd = open(CHILDLESS "/description", O_RDONLY);
+	ASSERT_GE(fd, 0);
+
+	/* Attributes report a page, whatever ->show() ends up producing. */
+	ASSERT_EQ(fstat(fd, &st), 0);
+	EXPECT_EQ(st.st_size, sysconf(_SC_PAGESIZE));
+
+	n = read(fd, buf, sizeof(buf));
+	ASSERT_GT(n, 0);
+	EXPECT_LT(n, st.st_size);
+	EXPECT_EQ(read(fd, buf, sizeof(buf)), 0);
+
+	EXPECT_EQ(close(fd), 0);
+}
+
+TEST_F(configfs, attr_write_is_not_incremental)
+{
+	char buf[64];
+	int fd;
+
+	/*
+	 * Every write hands the whole buffer to ->store() and the file
+	 * position is ignored, so the second write replaces the first.
+	 */
+	fd = open(CHILDLESS "/storeme", O_WRONLY);
+	ASSERT_GE(fd, 0);
+	ASSERT_EQ(write(fd, "1", 1), 1);
+	ASSERT_EQ(write(fd, "2", 1), 1);
+	EXPECT_EQ(close(fd), 0);
+
+	ASSERT_GT(read_attr(CHILDLESS "/storeme", buf, sizeof(buf)), 0);
+	EXPECT_STREQ(buf, "2\n");
+}
+
+TEST_F(configfs, item_create_and_drop)
+{
+	struct stat st;
+
+	ASSERT_EQ(mkdir(ITEM_A, 0755), 0);
+	EXPECT_EQ(stat(ITEM_A "/storeme", &st), 0);
+
+	/* The item carries its own attributes, not the subsystem's. */
+	ASSERT_EQ(stat(ITEM_A "/description", &st), -1);
+	EXPECT_EQ(errno, ENOENT);
+
+	ASSERT_EQ(rmdir(ITEM_A), 0);
+	ASSERT_EQ(stat(ITEM_A, &st), -1);
+	EXPECT_EQ(errno, ENOENT);
+}
+
+TEST_F(configfs, item_create_twice)
+{
+	ASSERT_EQ(mkdir(ITEM_A, 0755), 0);
+	ASSERT_EQ(mkdir(ITEM_A, 0755), -1);
+	EXPECT_EQ(errno, EEXIST);
+}
+
+TEST_F(configfs, item_has_no_children)
+{
+	/* ->make_item() produces an item, so it cannot nest. */
+	ASSERT_EQ(mkdir(ITEM_A, 0755), 0);
+	ASSERT_EQ(mkdir(ITEM_A "/kselftest-b", 0755), -1);
+	EXPECT_EQ(errno, EPERM);
+}
+
+TEST_F(configfs, item_attrs_are_private)
+{
+	char buf[64];
+
+	ASSERT_EQ(mkdir(ITEM_A, 0755), 0);
+	ASSERT_EQ(mkdir(ITEM_B, 0755), 0);
+
+	ASSERT_GT(write_attr(ITEM_A "/storeme", "11"), 0);
+	ASSERT_GT(write_attr(ITEM_B "/storeme", "22"), 0);
+
+	ASSERT_GT(read_attr(ITEM_A "/storeme", buf, sizeof(buf)), 0);
+	EXPECT_STREQ(buf, "11\n");
+	ASSERT_GT(read_attr(ITEM_B "/storeme", buf, sizeof(buf)), 0);
+	EXPECT_STREQ(buf, "22\n");
+}
+
+TEST_F(configfs, group_create_and_drop)
+{
+	struct stat st;
+
+	/* 03-group-children hands out groups that take items of their own. */
+	ASSERT_EQ(mkdir(GROUP, 0755), 0);
+	EXPECT_EQ(stat(GROUP "/description", &st), 0);
+
+	ASSERT_EQ(mkdir(GROUP_ITEM, 0755), 0);
+	EXPECT_EQ(stat(GROUP_ITEM "/storeme", &st), 0);
+
+	ASSERT_EQ(rmdir(GROUP), -1);
+	EXPECT_EQ(errno, ENOTEMPTY);
+
+	ASSERT_EQ(rmdir(GROUP_ITEM), 0);
+	ASSERT_EQ(rmdir(GROUP), 0);
+}
+
+TEST_F(configfs, rename_item)
+{
+	ASSERT_EQ(mkdir(ITEM_A, 0755), 0);
+	ASSERT_EQ(rename(ITEM_A, ITEM_B), -1);
+	EXPECT_EQ(errno, EPERM);
+}
+
+TEST_F(configfs, symlink_without_allow_link)
+{
+	ASSERT_EQ(mkdir(ITEM_A, 0755), 0);
+	ASSERT_EQ(symlink(ITEM_A, SIMPLE "/kselftest-link"), -1);
+	EXPECT_EQ(errno, EPERM);
+}
+
+TEST_F(configfs, module_pinned_by_item)
+{
+	ASSERT_EQ(mkdir(ITEM_A, 0755), 0);
+
+	/* mkdir() pins both the subsystem's module and the new item's. */
+	ASSERT_EQ(syscall(__NR_delete_module, SAMPLE_MODULE, O_NONBLOCK), -1);
+	if (errno == ENOSYS)
+		SKIP(return, "kernel built without CONFIG_MODULE_UNLOAD");
+	EXPECT_EQ(errno, EWOULDBLOCK);
+}
+
+TEST_HARNESS_MAIN

-- 
2.53.0-Meta


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH 2/4] samples/configfs: add a subsystem that accepts symlinks
  2026-08-26 10:47 [PATCH 0/4] configfs: add a basic selftest Breno Leitao
  2026-08-26 10:47 ` [PATCH 1/4] selftests/configfs: add tests for the userspace interface Breno Leitao
@ 2026-08-26 10:47 ` Breno Leitao
  2026-08-26 10:47 ` [PATCH 3/4] selftests/configfs: cover symlink and unlink Breno Leitao
  2026-08-26 10:47 ` [PATCH 4/4] selftests/configfs: race symlink against rmdir of the target Breno Leitao
  3 siblings, 0 replies; 5+ messages in thread
From: Breno Leitao @ 2026-08-26 10:47 UTC (permalink / raw)
  To: Shuah Khan, Andreas Hindborg
  Cc: linux-kernel, linux-kselftest, gustavold, linux-fsdevel,
	Breno Leitao, kernel-team, vasilisalmpanis

The sample registers three subsystems and none implements ->allow_link(),
so the symlink(2) half of the configfs ABI has no small in-tree user to
test against.

Add 04-symlink-children, whose children take a link to any config_item and
export the number of links they hold.

Signed-off-by: Breno Leitao <leitao@debian.org>
---
 samples/configfs/configfs_sample.c | 121 +++++++++++++++++++++++++++++++++++++
 1 file changed, 121 insertions(+)

diff --git a/samples/configfs/configfs_sample.c b/samples/configfs/configfs_sample.c
index c1b108ec4ea02..08403e787aef6 100644
--- a/samples/configfs/configfs_sample.c
+++ b/samples/configfs/configfs_sample.c
@@ -313,6 +313,126 @@ static struct configfs_subsystem group_children_subsys = {
 
 /* ----------------------------------------------------------------- */
 
+/*
+ * 04-symlink-children
+ *
+ * This example has children that are valid sources for symlink(2).  A
+ * child accepts a link to any other config_item and reports how many
+ * links it currently holds, so ->allow_link() and ->drop_link() are
+ * observable from userspace.
+ */
+
+struct symlink_child {
+	struct config_item item;
+	int nlinks;
+};
+
+static inline struct symlink_child *to_symlink_child(struct config_item *item)
+{
+	return container_of(item, struct symlink_child, item);
+}
+
+static ssize_t symlink_child_nlinks_show(struct config_item *item, char *page)
+{
+	return sprintf(page, "%d\n", to_symlink_child(item)->nlinks);
+}
+
+CONFIGFS_ATTR_RO(symlink_child_, nlinks);
+
+static struct configfs_attribute *symlink_child_attrs[] = {
+	&symlink_child_attr_nlinks,
+	NULL,
+};
+
+/*
+ * The VFS holds the source item's directory locked across symlink(2) and
+ * unlink(2), so ->nlinks needs no lock of its own.
+ */
+static int symlink_child_allow_link(struct config_item *src,
+		struct config_item *target)
+{
+	to_symlink_child(src)->nlinks++;
+
+	return 0;
+}
+
+static void symlink_child_drop_link(struct config_item *src,
+		struct config_item *target)
+{
+	to_symlink_child(src)->nlinks--;
+}
+
+static void symlink_child_release(struct config_item *item)
+{
+	kfree(to_symlink_child(item));
+}
+
+static const struct configfs_item_operations symlink_child_item_ops = {
+	.release	= symlink_child_release,
+	.allow_link	= symlink_child_allow_link,
+	.drop_link	= symlink_child_drop_link,
+};
+
+static const struct config_item_type symlink_child_type = {
+	.ct_item_ops	= &symlink_child_item_ops,
+	.ct_attrs	= symlink_child_attrs,
+	.ct_owner	= THIS_MODULE,
+};
+
+static struct config_item *symlink_children_make_item(
+		struct config_group *group, const char *name)
+{
+	struct symlink_child *symlink_child;
+
+	symlink_child = kzalloc_obj(*symlink_child, GFP_KERNEL);
+	if (!symlink_child)
+		return ERR_PTR(-ENOMEM);
+
+	config_item_init_type_name(&symlink_child->item, name,
+				   &symlink_child_type);
+
+	return &symlink_child->item;
+}
+
+static ssize_t symlink_children_description_show(struct config_item *item,
+		char *page)
+{
+	return sprintf(page,
+"[04-symlink-children]\n"
+"\n"
+"This subsystem allows the creation of child config_items that\n"
+"symlink(2) can point at other config_items from.  Each child\n"
+"reports the number of links it holds.\n");
+}
+
+CONFIGFS_ATTR_RO(symlink_children_, description);
+
+static struct configfs_attribute *symlink_children_attrs[] = {
+	&symlink_children_attr_description,
+	NULL,
+};
+
+static const struct configfs_group_operations symlink_children_group_ops = {
+	.make_item	= symlink_children_make_item,
+};
+
+static const struct config_item_type symlink_children_type = {
+	.ct_group_ops	= &symlink_children_group_ops,
+	.ct_attrs	= symlink_children_attrs,
+	.ct_owner	= THIS_MODULE,
+};
+
+static struct configfs_subsystem symlink_children_subsys = {
+	.su_group = {
+		.cg_item = {
+			.ci_namebuf = "04-symlink-children",
+			.ci_type = &symlink_children_type,
+		},
+	},
+};
+
+/* ----------------------------------------------------------------- */
+
 /*
  * We're now done with our subsystem definitions.
  * For convenience in this module, here's a list of them all.  It
@@ -324,6 +444,7 @@ static struct configfs_subsystem *example_subsys[] = {
 	&childless_subsys.subsys,
 	&simple_children_subsys,
 	&group_children_subsys,
+	&symlink_children_subsys,
 	NULL,
 };
 

-- 
2.53.0-Meta


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH 3/4] selftests/configfs: cover symlink and unlink
  2026-08-26 10:47 [PATCH 0/4] configfs: add a basic selftest Breno Leitao
  2026-08-26 10:47 ` [PATCH 1/4] selftests/configfs: add tests for the userspace interface Breno Leitao
  2026-08-26 10:47 ` [PATCH 2/4] samples/configfs: add a subsystem that accepts symlinks Breno Leitao
@ 2026-08-26 10:47 ` Breno Leitao
  2026-08-26 10:47 ` [PATCH 4/4] selftests/configfs: race symlink against rmdir of the target Breno Leitao
  3 siblings, 0 replies; 5+ messages in thread
From: Breno Leitao @ 2026-08-26 10:47 UTC (permalink / raw)
  To: Shuah Khan, Andreas Hindborg
  Cc: linux-kernel, linux-kselftest, gustavold, linux-fsdevel,
	Breno Leitao, kernel-team, vasilisalmpanis

samples/configfs had nothing to link from, so symlink(2) went untested
beyond the EPERM a subsystem without ->allow_link() returns.

Cover creating and removing a link, the relative body configfs stores, the
count ->allow_link() and ->drop_link() keep, the busy errors both ends
return, and what get_target() rejects.

Signed-off-by: Breno Leitao <leitao@debian.org>
---
 .../selftests/filesystems/configfs/configfs_test.c | 85 +++++++++++++++++++++-
 1 file changed, 83 insertions(+), 2 deletions(-)

diff --git a/tools/testing/selftests/filesystems/configfs/configfs_test.c b/tools/testing/selftests/filesystems/configfs/configfs_test.c
index 072d0dcf3f96b..9b15e1fd69e5b 100644
--- a/tools/testing/selftests/filesystems/configfs/configfs_test.c
+++ b/tools/testing/selftests/filesystems/configfs/configfs_test.c
@@ -1,6 +1,6 @@
 // SPDX-License-Identifier: GPL-2.0
 /*
- * Exercise the configfs userspace interface through the three subsystems
+ * Exercise the configfs userspace interface through the subsystems
  * registered by samples/configfs.
  *
  * Copyright (c) 2026 Meta Platforms, Inc. and affiliates
@@ -10,6 +10,7 @@
 
 #include <errno.h>
 #include <fcntl.h>
+#include <limits.h>
 #include <sched.h>
 #include <stdbool.h>
 #include <stdio.h>
@@ -31,16 +32,24 @@
 #define CHILDLESS		"01-childless"
 #define SIMPLE			"02-simple-children"
 #define GROUPS			"03-group-children"
+#define SYMLINKS		"04-symlink-children"
 
 #define ITEM_A			SIMPLE "/kselftest-a"
 #define ITEM_B			SIMPLE "/kselftest-b"
 #define GROUP			GROUPS "/kselftest-group"
 #define GROUP_ITEM		GROUP "/kselftest-a"
+#define LINK_SRC		SYMLINKS "/kselftest-src"
+#define LINK			LINK_SRC "/kselftest-link"
+
+static const char * const test_links[] = {
+	LINK,
+};
 
 /* Deepest first, so one pass empties the tree. */
 static const char * const test_dirs[] = {
 	GROUP_ITEM,
 	GROUP,
+	LINK_SRC,
 	ITEM_A,
 	ITEM_B,
 };
@@ -49,6 +58,10 @@ static void drop_test_dirs(void)
 {
 	size_t i;
 
+	/* Links first: they hold both their source and their target. */
+	for (i = 0; i < ARRAY_SIZE(test_links); i++)
+		unlink(test_links[i]);
+
 	for (i = 0; i < ARRAY_SIZE(test_dirs); i++)
 		rmdir(test_dirs[i]);
 }
@@ -131,7 +144,7 @@ FIXTURE_TEARDOWN(configfs)
 
 TEST_F(configfs, mount_and_subsystems)
 {
-	const char * const subsys[] = { CHILDLESS, SIMPLE, GROUPS };
+	const char * const subsys[] = { CHILDLESS, SIMPLE, GROUPS, SYMLINKS };
 	struct statfs sfs;
 	struct stat st;
 	size_t i;
@@ -324,6 +337,74 @@ TEST_F(configfs, symlink_without_allow_link)
 	EXPECT_EQ(errno, EPERM);
 }
 
+TEST_F(configfs, symlink_and_unlink)
+{
+	char buf[PATH_MAX];
+	struct stat st;
+	ssize_t n;
+
+	ASSERT_EQ(mkdir(ITEM_A, 0755), 0);
+	ASSERT_EQ(mkdir(LINK_SRC, 0755), 0);
+
+	ASSERT_EQ(symlink(ITEM_A, LINK), 0);
+
+	/* configfs stores its own body, a path relative to the link. */
+	n = readlink(LINK, buf, sizeof(buf) - 1);
+	ASSERT_GT(n, 0);
+	buf[n] = '\0';
+	EXPECT_STREQ(buf, "../../" ITEM_A);
+	EXPECT_EQ(stat(LINK "/storeme", &st), 0);
+
+	/* ->allow_link() ran on the source, not on the target. */
+	ASSERT_GT(read_attr(LINK_SRC "/nlinks", buf, sizeof(buf)), 0);
+	EXPECT_STREQ(buf, "1\n");
+
+	ASSERT_EQ(unlink(LINK), 0);
+	ASSERT_GT(read_attr(LINK_SRC "/nlinks", buf, sizeof(buf)), 0);
+	EXPECT_STREQ(buf, "0\n");
+}
+
+TEST_F(configfs, symlink_pins_both_ends)
+{
+	ASSERT_EQ(mkdir(ITEM_A, 0755), 0);
+	ASSERT_EQ(mkdir(LINK_SRC, 0755), 0);
+	ASSERT_EQ(symlink(ITEM_A, LINK), 0);
+
+	/* A linked item cannot go away under the link. */
+	ASSERT_EQ(rmdir(ITEM_A), -1);
+	EXPECT_EQ(errno, EBUSY);
+
+	/* The link counts as a child of its source. */
+	ASSERT_EQ(rmdir(LINK_SRC), -1);
+	EXPECT_EQ(errno, ENOTEMPTY);
+
+	ASSERT_EQ(unlink(LINK), 0);
+	EXPECT_EQ(rmdir(ITEM_A), 0);
+}
+
+TEST_F(configfs, symlink_target_outside_configfs)
+{
+	ASSERT_EQ(mkdir(LINK_SRC, 0755), 0);
+	ASSERT_EQ(symlink("/", LINK), -1);
+	EXPECT_EQ(errno, EPERM);
+}
+
+TEST_F(configfs, symlink_target_missing)
+{
+	ASSERT_EQ(mkdir(LINK_SRC, 0755), 0);
+	ASSERT_EQ(symlink(SIMPLE "/kselftest-gone", LINK), -1);
+	EXPECT_EQ(errno, ENOENT);
+}
+
+TEST_F(configfs, symlink_target_is_an_attribute)
+{
+	ASSERT_EQ(mkdir(LINK_SRC, 0755), 0);
+
+	/* The target is resolved with LOOKUP_DIRECTORY. */
+	ASSERT_EQ(symlink(CHILDLESS "/storeme", LINK), -1);
+	EXPECT_EQ(errno, ENOTDIR);
+}
+
 TEST_F(configfs, module_pinned_by_item)
 {
 	ASSERT_EQ(mkdir(ITEM_A, 0755), 0);

-- 
2.53.0-Meta


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH 4/4] selftests/configfs: race symlink against rmdir of the target
  2026-08-26 10:47 [PATCH 0/4] configfs: add a basic selftest Breno Leitao
                   ` (2 preceding siblings ...)
  2026-08-26 10:47 ` [PATCH 3/4] selftests/configfs: cover symlink and unlink Breno Leitao
@ 2026-08-26 10:47 ` Breno Leitao
  3 siblings, 0 replies; 5+ messages in thread
From: Breno Leitao @ 2026-08-26 10:47 UTC (permalink / raw)
  To: Shuah Khan, Andreas Hindborg
  Cc: linux-kernel, linux-kselftest, gustavold, linux-fsdevel,
	Breno Leitao, kernel-team, vasilisalmpanis

configfs_rmdir() drops the last reference to an item while its dentry is
still hashed, so a symlink(2) resolving that target takes a reference on
freed memory.  syzbot reported it [1].

Race mkdir/rmdir of a target against symlink/unlink of a link to it,
watching /sys/kernel/warn_count.

Fails until the fix [2] lands.

Link: https://syzkaller.appspot.com/bug?extid=6b16e3d085833cbf3e25 [1]
Link: https://lore.kernel.org/all/20260730093435.195441-1-vasilisalmpanis@gmail.com/ [2]
Signed-off-by: Breno Leitao <leitao@debian.org>
---
 .../selftests/filesystems/configfs/Makefile        |  2 +-
 .../selftests/filesystems/configfs/configfs_test.c | 62 ++++++++++++++++++++++
 2 files changed, 63 insertions(+), 1 deletion(-)

diff --git a/tools/testing/selftests/filesystems/configfs/Makefile b/tools/testing/selftests/filesystems/configfs/Makefile
index 359296356c831..40a91ed788ed2 100644
--- a/tools/testing/selftests/filesystems/configfs/Makefile
+++ b/tools/testing/selftests/filesystems/configfs/Makefile
@@ -2,7 +2,7 @@
 # Copyright (c) 2026 Meta Platforms, Inc. and affiliates
 # Copyright (c) 2026 Breno Leitao <leitao@debian.org>
 
-CFLAGS += -Wall -Werror
+CFLAGS += -Wall -Werror -pthread
 TEST_GEN_PROGS := configfs_test
 
 include ../../lib.mk
diff --git a/tools/testing/selftests/filesystems/configfs/configfs_test.c b/tools/testing/selftests/filesystems/configfs/configfs_test.c
index 9b15e1fd69e5b..c6a1049e5852e 100644
--- a/tools/testing/selftests/filesystems/configfs/configfs_test.c
+++ b/tools/testing/selftests/filesystems/configfs/configfs_test.c
@@ -11,6 +11,7 @@
 #include <errno.h>
 #include <fcntl.h>
 #include <limits.h>
+#include <pthread.h>
 #include <sched.h>
 #include <stdbool.h>
 #include <stdio.h>
@@ -41,6 +42,8 @@
 #define LINK_SRC		SYMLINKS "/kselftest-src"
 #define LINK			LINK_SRC "/kselftest-link"
 
+#define RACE_ITERATIONS		20000
+
 static const char * const test_links[] = {
 	LINK,
 };
@@ -405,6 +408,65 @@ TEST_F(configfs, symlink_target_is_an_attribute)
 	EXPECT_EQ(errno, ENOTDIR);
 }
 
+static volatile int race_stop;
+
+static void *rmdir_target(void *arg)
+{
+	while (!race_stop) {
+		if (mkdir(ITEM_A, 0755) == 0 || errno == EEXIST)
+			rmdir(ITEM_A);
+	}
+
+	return NULL;
+}
+
+/* -1 if the kernel does not export a warning counter. */
+static long warn_count(void)
+{
+	char buf[32];
+
+	if (read_attr("/sys/kernel/warn_count", buf, sizeof(buf)) < 0)
+		return -1;
+
+	return strtol(buf, NULL, 10);
+}
+
+TEST_F(configfs, symlink_races_with_target_rmdir)
+{
+	pthread_t thread;
+	long warns;
+	int i;
+
+	warns = warn_count();
+	if (warns < 0)
+		SKIP(return, "no /sys/kernel/warn_count to watch");
+
+	ASSERT_EQ(mkdir(LINK_SRC, 0755), 0);
+	ASSERT_EQ(pthread_create(&thread, NULL, rmdir_target, NULL), 0);
+
+	/*
+	 * configfs_rmdir() drops the last reference to the item while its
+	 * dentry is still hashed, and get_target() takes a hashed dentry as
+	 * proof that the item behind it is alive.  The symlink then walks
+	 * ->ci_dentry into a released dirent, which configfs_get() warns
+	 * about.  KASAN sees the freed item itself.
+	 */
+	for (i = 0; i < RACE_ITERATIONS; i++) {
+		if (symlink(ITEM_A, LINK) == 0)
+			unlink(LINK);
+
+		/* Give up on the first splat rather than flood the log. */
+		if (!(i % 128) && warn_count() != warns)
+			break;
+	}
+
+	race_stop = 1;
+	ASSERT_EQ(pthread_join(thread, NULL), 0);
+
+	EXPECT_EQ(warn_count(), warns)
+		TH_LOG("kernel warned after %d iterations", i);
+}
+
 TEST_F(configfs, module_pinned_by_item)
 {
 	ASSERT_EQ(mkdir(ITEM_A, 0755), 0);

-- 
2.53.0-Meta


^ permalink raw reply related	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2026-08-26 10:47 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-26 10:47 [PATCH 0/4] configfs: add a basic selftest Breno Leitao
2026-08-26 10:47 ` [PATCH 1/4] selftests/configfs: add tests for the userspace interface Breno Leitao
2026-08-26 10:47 ` [PATCH 2/4] samples/configfs: add a subsystem that accepts symlinks Breno Leitao
2026-08-26 10:47 ` [PATCH 3/4] selftests/configfs: cover symlink and unlink Breno Leitao
2026-08-26 10:47 ` [PATCH 4/4] selftests/configfs: race symlink against rmdir of the target Breno Leitao

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox