[PATCH bpf-next 0/2] Allow storing referenced struct file kptrs in BPF maps

[PATCH bpf-next 0/2] Allow storing referenced struct file kptrs in BPF maps

[Justin Suess]

Hello,

This series adds a destructor for struct file, enabling it to be used as a
referenced kptr in BPF maps.

The destructor mirrors bpf_put_file() semantics and releases the reference
via fput(). This allows pointers returned from kfuncs such as
bpf_get_task_exe_file() to be safely stored and later reused with helpers
and kfuncs that operate on struct file.

This fills a gap compared to bpf_dynptr_from_file(), where the resulting
dynptr can be stored in a map, but cannot be passed to kfuncs expecting
a struct file *.

Use cases include caching file references across events and deferring
processing while keeping the underlying file alive.

Patch 1 adds the struct file kptr destructor and wires it into the BTF
kfunc sets.

Patch 2 adds selftests covering successful use and verifier rejection of
unreferenced pointers.

Justin Suess (2):
  bpf: Implement dtor for struct file BTF ID
  selftests/bpf: Add test for map-stored struct file kptrs

 kernel/bpf/helpers.c                          |  16 +-
 .../bpf/prog_tests/refcounted_kptr_file.c     | 246 ++++++++++++++++++
 .../bpf/progs/refcounted_kptr_file.c          | 152 +++++++++++
 .../bpf/progs/refcounted_kptr_file_fail.c     |  35 +++
 4 files changed, 448 insertions(+), 1 deletion(-)
 create mode 100644 tools/testing/selftests/bpf/prog_tests/refcounted_kptr_file.c
 create mode 100644 tools/testing/selftests/bpf/progs/refcounted_kptr_file.c
 create mode 100644 tools/testing/selftests/bpf/progs/refcounted_kptr_file_fail.c

-- 
2.53.0

[PATCH bpf-next 1/2] bpf: Implement dtor for struct file BTF ID

[Justin Suess]

Implement a destructor for struct file allowing it to be stored in a map
as a referenced kptr.

This dtor is analogous to bpf_put_file, and calls fput to release the
reference.

Signed-off-by: Justin Suess <utilityemal77@gmail.com>
---
 kernel/bpf/helpers.c | 16 +++++++++++++++-
 1 file changed, 15 insertions(+), 1 deletion(-)

diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
index baa12b24bb64..4739e2bf0f31 100644
--- a/kernel/bpf/helpers.c
+++ b/kernel/bpf/helpers.c
@@ -20,6 +20,7 @@
 #include <linux/proc_ns.h>
 #include <linux/sched/task.h>
 #include <linux/security.h>
+#include <linux/fs.h>
 #include <linux/btf_ids.h>
 #include <linux/bpf_mem_alloc.h>
 #include <linux/kasan.h>
@@ -2752,6 +2753,13 @@ __bpf_kfunc void bpf_task_release_dtor(void *p)
 }
 CFI_NOSEAL(bpf_task_release_dtor);
 
+__bpf_kfunc void bpf_file_release_dtor(void *file)
+{
+	fput((struct file *)file);
+}
+
+CFI_NOSEAL(bpf_file_release_dtor);
+
 #ifdef CONFIG_CGROUPS
 /**
  * bpf_cgroup_acquire - Acquire a reference to a cgroup. A cgroup acquired by
@@ -4763,6 +4771,8 @@ static const struct btf_kfunc_id_set generic_kfunc_set = {
 BTF_ID_LIST(generic_dtor_ids)
 BTF_ID(struct, task_struct)
 BTF_ID(func, bpf_task_release_dtor)
+BTF_ID(struct, file)
+BTF_ID(func, bpf_file_release_dtor)
 #ifdef CONFIG_CGROUPS
 BTF_ID(struct, cgroup)
 BTF_ID(func, bpf_cgroup_release_dtor)
@@ -4874,11 +4884,15 @@ static int __init kfunc_init(void)
 			.btf_id       = generic_dtor_ids[0],
 			.kfunc_btf_id = generic_dtor_ids[1]
 		},
-#ifdef CONFIG_CGROUPS
 		{
 			.btf_id       = generic_dtor_ids[2],
 			.kfunc_btf_id = generic_dtor_ids[3]
 		},
+#ifdef CONFIG_CGROUPS
+		{
+			.btf_id       = generic_dtor_ids[4],
+			.kfunc_btf_id = generic_dtor_ids[5]
+		},
 #endif
 	};
 
-- 
2.53.0

[PATCH bpf-next 2/2] selftests/bpf: Add test for map-stored struct file kptrs

[Justin Suess]

Add tests for map-stored struct file referenced kptrs.

Add a test storing a referenced kptr in a map. The referenced pointer is
acquired by get_task_exe_file in this case.

Add a test that verifies that the struct file reference remains valid
when the original file descriptor is closed, by checking the extented
attributes on it after completion. Trigger this validation manually by
hooking file_open and opening dev null.

Validation is done with xattrs.

Add a test verifying that attempting to insert unreferenced struct file
pointers into the map is rejected by the verifier.

Signed-off-by: Justin Suess <utilityemal77@gmail.com>
---
 .../bpf/prog_tests/refcounted_kptr_file.c     | 246 ++++++++++++++++++
 .../bpf/progs/refcounted_kptr_file.c          | 152 +++++++++++
 .../bpf/progs/refcounted_kptr_file_fail.c     |  35 +++
 3 files changed, 433 insertions(+)
 create mode 100644 tools/testing/selftests/bpf/prog_tests/refcounted_kptr_file.c
 create mode 100644 tools/testing/selftests/bpf/progs/refcounted_kptr_file.c
 create mode 100644 tools/testing/selftests/bpf/progs/refcounted_kptr_file_fail.c

diff --git a/tools/testing/selftests/bpf/prog_tests/refcounted_kptr_file.c b/tools/testing/selftests/bpf/prog_tests/refcounted_kptr_file.c
new file mode 100644
index 000000000000..173121377040
--- /dev/null
+++ b/tools/testing/selftests/bpf/prog_tests/refcounted_kptr_file.c
@@ -0,0 +1,246 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#define _GNU_SOURCE
+
+#include <errno.h>
+#include <test_progs.h>
+#include <fcntl.h>
+#include <limits.h>
+#include <stdbool.h>
+#include <stdlib.h>
+#include <string.h>
+#include <sys/stat.h>
+#include <sys/wait.h>
+#include <sys/xattr.h>
+#include <unistd.h>
+
+#include "refcounted_kptr_file_fail.skel.h"
+#include "refcounted_kptr_file.skel.h"
+
+static const char shell_path[] = "/bin/sh";
+static const char xattr_name[] = "user.kptr_ref";
+static const char xattr_value[] = "kptr-live";
+
+static void close_fd(int fd)
+{
+	if (fd >= 0)
+		close(fd);
+}
+
+static int write_full(int fd, const void *buf, size_t len)
+{
+	const char *pos = buf;
+
+	while (len) {
+		ssize_t written;
+
+		written = write(fd, pos, len);
+		if (written < 0) {
+			if (errno == EINTR)
+				continue;
+			return -errno;
+		}
+
+		pos += written;
+		len -= written;
+	}
+
+	return 0;
+}
+
+static int copy_file(const char *src_path, const char *dst_path, mode_t mode)
+{
+	char buf[4096];
+	int src_fd = -1, dst_fd = -1;
+	int err = 0;
+
+	src_fd = open(src_path, O_RDONLY | O_CLOEXEC);
+	if (src_fd < 0)
+		return -errno;
+
+	dst_fd = open(dst_path, O_WRONLY | O_CREAT | O_EXCL | O_CLOEXEC,
+		      mode & 0777);
+	if (dst_fd < 0) {
+		err = -errno;
+		goto out;
+	}
+
+	while (1) {
+		ssize_t bytes_read;
+
+		bytes_read = read(src_fd, buf, sizeof(buf));
+		if (bytes_read < 0) {
+			if (errno == EINTR)
+				continue;
+			err = -errno;
+			goto out;
+		}
+
+		if (!bytes_read)
+			break;
+
+		err = write_full(dst_fd, buf, bytes_read);
+		if (err) {
+			/* Remove partially written destination file so it
+			 * doesn't linger on disk.
+			 */
+			unlink(dst_path);
+			goto out;
+		}
+	}
+
+out:
+	close_fd(dst_fd);
+	close_fd(src_fd);
+	return err;
+}
+
+static bool prepare_tagged_shell(char *temp_dir, size_t temp_dir_sz,
+				 char *shell_copy, size_t shell_copy_sz)
+{
+	struct stat st;
+	int err;
+
+	if (!ASSERT_LT(snprintf(temp_dir, temp_dir_sz,
+				"./refcounted_kptr_file.XXXXXX"),
+		     (int)temp_dir_sz, "temp_dir_template"))
+		return false;
+
+	if (!ASSERT_OK_PTR(mkdtemp(temp_dir), "mkdtemp"))
+		return false;
+
+	if (!ASSERT_LT(snprintf(shell_copy, shell_copy_sz, "%s/sh", temp_dir),
+		       (int)shell_copy_sz, "shell_copy_path"))
+		goto err_rmdir;
+
+	if (!ASSERT_OK(stat(shell_path, &st), "stat_shell"))
+		goto err_rmdir;
+
+	err = copy_file(shell_path, shell_copy, st.st_mode);
+	if (!ASSERT_OK(err, "copy_shell"))
+		goto err_unlink;
+
+	err = setxattr(shell_copy, xattr_name, xattr_value, sizeof(xattr_value), 0);
+	if (err && errno == EOPNOTSUPP) {
+		printf("%s:SKIP:filesystem does not support user xattr (%d)\n",
+		       __func__, errno);
+		test__skip();
+		goto err_unlink;
+	}
+
+	if (!ASSERT_OK(err, "setxattr_shell"))
+		goto err_unlink;
+
+	return true;
+
+err_unlink:
+	unlink(shell_copy);
+err_rmdir:
+	rmdir(temp_dir);
+	return false;
+}
+
+static void run_refcounted_file_kptr_success(void)
+{
+	struct refcounted_kptr_file *skel;
+	char shell_copy[PATH_MAX] = {};
+	char temp_dir[PATH_MAX] = {};
+	int pipefd[2] = { -1, -1 };
+	int status;
+	pid_t child_pid;
+	int err, fd = -1;
+
+	skel = refcounted_kptr_file__open();
+	if (!ASSERT_OK_PTR(skel, "refcounted_kptr_file__open"))
+		return;
+
+	err = refcounted_kptr_file__load(skel);
+	if (!ASSERT_OK(err, "refcounted_kptr_file__load"))
+		goto out;
+
+	err = refcounted_kptr_file__attach(skel);
+	if (!ASSERT_OK(err, "refcounted_kptr_file__attach"))
+		goto out;
+
+	if (!prepare_tagged_shell(temp_dir, sizeof(temp_dir), shell_copy,
+				  sizeof(shell_copy)))
+		goto out;
+
+	if (!ASSERT_OK(pipe2(pipefd, O_CLOEXEC), "pipe2"))
+		goto out;
+
+	child_pid = fork();
+	if (!ASSERT_GT(child_pid, -1, "fork"))
+		goto out;
+
+	if (child_pid == 0) {
+		char sync;
+
+		close(pipefd[1]);
+		if (read(pipefd[0], &sync, 1) != 1)
+			_exit(127);
+		close(pipefd[0]);
+		execl(shell_copy, shell_copy, "-c", ": </dev/null", NULL);
+		_exit(127);
+	}
+
+	close(pipefd[0]);
+	pipefd[0] = -1;
+
+	/* Set the PID *before* unblocking the child so that the BPF program's
+	 * file_open hook can filter on it from the very first file opened during
+	 * exec.  If the write happened first, early file_open events during
+	 * exec could be missed.
+	 */
+	skel->bss->file_kptr_insert_pid = child_pid;
+	err = write_full(pipefd[1], "1", 1);
+	if (!ASSERT_OK(err, "start_child"))
+		goto out;
+	close(pipefd[1]);
+	pipefd[1] = -1;
+
+	if (!ASSERT_EQ(waitpid(child_pid, &status, 0), child_pid, "waitpid"))
+		goto out;
+	if (!ASSERT_TRUE(WIFEXITED(status), "child_exited"))
+		goto out;
+	if (!ASSERT_EQ(WEXITSTATUS(status), 0, "child_status"))
+		goto out;
+
+	skel->bss->file_kptr_verify_pid = getpid();
+	/* The child is gone at this point. Reopening an unrelated file triggers a
+	 * second file_open hook where the BPF program validates the stashed ref.
+	 * Our test op for the ref validity is reading the xattrs we set earlier.
+	 */
+	fd = open("/dev/null", O_RDONLY | O_CLOEXEC);
+	if (!ASSERT_GE(fd, 0, "open_dev_null"))
+		goto out;
+	close(fd);
+	fd = -1;
+
+	ASSERT_EQ(skel->bss->file_kptr_err, 0, "file_kptr_err");
+	ASSERT_EQ(skel->bss->file_kptr_inserted, 1, "file_kptr_inserted");
+	ASSERT_EQ(skel->bss->file_kptr_verified, 1, "file_kptr_verified");
+	ASSERT_EQ(skel->bss->file_kptr_xattr_ret, sizeof(xattr_value),
+		  "file_kptr_xattr_ret");
+	ASSERT_EQ(strcmp(skel->bss->file_kptr_value, xattr_value), 0,
+		  "file_kptr_value");
+
+out:
+	close_fd(fd);
+	close_fd(pipefd[0]);
+	close_fd(pipefd[1]);
+	if (shell_copy[0])
+		unlink(shell_copy);
+	if (temp_dir[0])
+		rmdir(temp_dir);
+	refcounted_kptr_file__destroy(skel);
+}
+
+void test_refcounted_kptr_file(void)
+{
+	if (test__start_subtest("holds_ref_after_close"))
+		run_refcounted_file_kptr_success();
+
+	if (test__start_subtest("reject_unref_ctx_file"))
+		RUN_TESTS(refcounted_kptr_file_fail);
+}
diff --git a/tools/testing/selftests/bpf/progs/refcounted_kptr_file.c b/tools/testing/selftests/bpf/progs/refcounted_kptr_file.c
new file mode 100644
index 000000000000..104fc85b2afa
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/refcounted_kptr_file.c
@@ -0,0 +1,152 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include <vmlinux.h>
+#include <bpf/bpf_helpers.h>
+#include <bpf/bpf_tracing.h>
+#include "bpf_kfuncs.h"
+#include "bpf_experimental.h"
+
+struct file_map_value {
+	struct file __kptr * file;
+};
+
+struct {
+	__uint(type, BPF_MAP_TYPE_ARRAY);
+	__type(key, int);
+	__type(value, struct file_map_value);
+	__uint(max_entries, 1);
+} stashed_files SEC(".maps");
+
+static const char xattr_name[] = "user.kptr_ref";
+static const char expected_value[] = "kptr-live";
+
+char file_kptr_probe_value[32];
+int file_kptr_insert_pid;
+int file_kptr_verify_pid;
+int file_kptr_inserted;
+int file_kptr_verified;
+int file_kptr_err;
+int file_kptr_xattr_ret;
+char file_kptr_value[32];
+
+SEC("lsm.s/file_open")
+int insert_file_kptr(struct file *ctx_file)
+{
+	struct bpf_dynptr value_ptr;
+	struct file_map_value *mapval;
+	struct task_struct *task;
+	struct file *file, *old;
+	int ret;
+	int zero = 0;
+
+	(void)ctx_file;
+
+	if ((__u32)(bpf_get_current_pid_tgid() >> 32) != (__u32)file_kptr_insert_pid)
+		return 0;
+
+	if (file_kptr_inserted)
+		return 0;
+
+	mapval = bpf_map_lookup_elem(&stashed_files, &zero);
+	if (!mapval) {
+		file_kptr_err = 1;
+		return 0;
+	}
+
+	task = bpf_get_current_task_btf();
+	file = bpf_get_task_exe_file(task);
+	if (!file) {
+		file_kptr_err = 2;
+		return 0;
+	}
+
+	/* Exec can open multiple files while the new image is being installed.
+	 * Only stash the child's final executable, which we identify by the test
+	 * xattr.
+	 */
+	ret = bpf_dynptr_from_mem(file_kptr_probe_value,
+				  sizeof(file_kptr_probe_value), 0,
+				  &value_ptr);
+	if (ret) {
+		file_kptr_err = 8;
+		bpf_put_file(file);
+		return 0;
+	}
+
+	ret = bpf_get_file_xattr(file, xattr_name, &value_ptr);
+	if (ret != sizeof(expected_value) ||
+	    bpf_strncmp(file_kptr_probe_value, sizeof(expected_value),
+			expected_value)) {
+		bpf_put_file(file);
+		return 0;
+	}
+
+	old = bpf_kptr_xchg(&mapval->file, file);
+	if (old)
+		bpf_put_file(old);
+
+	file_kptr_inserted = 1;
+	return 0;
+}
+
+SEC("lsm.s/file_open")
+int verify_file_kptr(struct file *ctx_file)
+{
+	struct bpf_dynptr value_ptr;
+	struct file_map_value *mapval;
+	struct file *file;
+	int zero = 0;
+	int ret;
+
+	(void)ctx_file;
+
+	if ((__u32)(bpf_get_current_pid_tgid() >> 32) != (__u32)file_kptr_verify_pid)
+		return 0;
+
+	if (!file_kptr_inserted || file_kptr_verified)
+		return 0;
+
+	mapval = bpf_map_lookup_elem(&stashed_files, &zero);
+	if (!mapval) {
+		file_kptr_err = 3;
+		return 0;
+	}
+
+	/* 
+	 * Pull the file out of the map to get a referenced pointer for the xattr
+	 * kfunc and to drop the map's last reference once verification completes.
+	 */
+	file = bpf_kptr_xchg(&mapval->file, NULL);
+	if (!file) {
+		file_kptr_err = 4;
+		return 0;
+	}
+
+	ret = bpf_dynptr_from_mem(file_kptr_value, sizeof(file_kptr_value), 0,
+				  &value_ptr);
+	if (ret) {
+		file_kptr_err = 5;
+		bpf_put_file(file);
+		return 0;
+	}
+
+	ret = bpf_get_file_xattr(file, xattr_name, &value_ptr);
+	file_kptr_xattr_ret = ret;
+	if (ret != sizeof(expected_value)) {
+		file_kptr_err = 6;
+		bpf_put_file(file);
+		return 0;
+	}
+
+	if (bpf_strncmp(file_kptr_value, sizeof(expected_value), expected_value)) {
+		file_kptr_err = 7;
+		bpf_put_file(file);
+		return 0;
+	}
+
+	file_kptr_verified = 1;
+	bpf_put_file(file);
+	return 0;
+}
+
+char _license[] SEC("license") = "GPL";
diff --git a/tools/testing/selftests/bpf/progs/refcounted_kptr_file_fail.c b/tools/testing/selftests/bpf/progs/refcounted_kptr_file_fail.c
new file mode 100644
index 000000000000..7adfa6645801
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/refcounted_kptr_file_fail.c
@@ -0,0 +1,35 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include <vmlinux.h>
+#include <bpf/bpf_helpers.h>
+#include <bpf/bpf_tracing.h>
+#include "bpf_misc.h"
+
+struct file_map_value {
+	struct file __kptr * file;
+};
+
+struct {
+	__uint(type, BPF_MAP_TYPE_ARRAY);
+	__type(key, int);
+	__type(value, struct file_map_value);
+	__uint(max_entries, 1);
+} stashed_files SEC(".maps");
+
+SEC("lsm.s/file_open")
+__failure __msg("R2 type=ctx expected=ptr_, trusted_ptr_, rcu_ptr_")
+int stash_unref_ctx_file(struct file *ctx_file)
+{
+	struct file_map_value *mapval;
+	int zero = 0;
+
+	mapval = bpf_map_lookup_elem(&stashed_files, &zero);
+	if (!mapval)
+		return 0;
+
+	/* ctx_file is just the hook argument, not an acquired file reference. */
+	bpf_kptr_xchg(&mapval->file, ctx_file);
+	return 0;
+}
+
+char _license[] SEC("license") = "GPL";
-- 
2.53.0

Re: [PATCH bpf-next 2/2] selftests/bpf: Add test for map-stored struct file kptrs

[bot+bpf-ci]

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

> selftests/bpf: Add test for map-stored struct file kptrs
>
> Add tests for map-stored struct file referenced kptrs.
>
> Add a test storing a referenced kptr in a map. The referenced pointer is
> acquired by get_task_exe_file in this case.
>
> Add a test that verifies that the struct file reference remains valid
> when the original file descriptor is closed, by checking the extented
> attributes on it after completion. Trigger this validation manually by
                                                               ^^^^^^^^^

There's a typo here: 'extented' should be 'extended'.

> hooking file_open and opening dev null.
>
> Validation is done with xattrs.
>
> Add a test verifying that attempting to insert unreferenced struct file
> pointers into the map is rejected by the verifier.
>
> Signed-off-by: Justin Suess <utilityemal77@gmail.com>

> diff --git a/tools/testing/selftests/bpf/prog_tests/refcounted_kptr_file.c b/tools/testing/selftests/bpf/prog_tests/refcounted_kptr_file.c
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/prog_tests/refcounted_kptr_file.c

[ ... ]

> @@ -0,0 +1,246 @@

[ ... ]

> +	while (1) {
> +		ssize_t bytes_read;
> +
> +		bytes_read = read(src_fd, buf, sizeof(buf));
> +		if (bytes_read < 0) {
> +			if (errno == EINTR)
> +				continue;
> +			err = -errno;
> +			goto out;
> +		}
> +
> +		if (!bytes_read)
> +			break;
> +
> +		err = write_full(dst_fd, buf, bytes_read);
> +		if (err) {
> +			/* Remove partially written destination file so it
> +			 * doesn't linger on disk.
> +			 */

Can this comment follow the BPF multi-line comment style? The opening /*
should be on its own line:

/*
 * Remove partially written destination file so it
 * doesn't linger on disk.
 */

> +			unlink(dst_path);
> +			goto out;
> +		}
> +	}

[ ... ]

> diff --git a/tools/testing/selftests/bpf/progs/refcounted_kptr_file.c b/tools/testing/selftests/bpf/progs/refcounted_kptr_file.c
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/progs/refcounted_kptr_file.c

[ ... ]

> @@ -0,0 +1,152 @@

[ ... ]

> +	task = bpf_get_current_task_btf();
> +	file = bpf_get_task_exe_file(task);
> +	if (!file) {
> +		file_kptr_err = 2;
> +		return 0;
> +	}
> +
> +	/* Exec can open multiple files while the new image is being installed.
> +	 * Only stash the child's final executable, which we identify by the test
> +	 * xattr.
> +	 */

Can this comment follow the BPF multi-line comment style? The opening /*
should be on its own line:

/*
 * Exec can open multiple files while the new image is being installed.
 * Only stash the child's final executable, which we identify by the test
 * xattr.
 */

> +	ret = bpf_dynptr_from_mem(file_kptr_probe_value,
> +				  sizeof(file_kptr_probe_value), 0,
> +				  &value_ptr);

[ ... ]


---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md

CI run summary: https://github.com/kernel-patches/bpf/actions/runs/24689427455

Re: [PATCH bpf-next 1/2] bpf: Implement dtor for struct file BTF ID

[Song Liu]

+ linux-fsdevel

On Mon, Apr 20, 2026 at 1:34 PM Justin Suess <utilityemal77@gmail.com> wrote:
>
> Implement a destructor for struct file allowing it to be stored in a map
> as a referenced kptr.
>
> This dtor is analogous to bpf_put_file, and calls fput to release the
> reference.
>
> Signed-off-by: Justin Suess <utilityemal77@gmail.com>

The code looks good to me.

Acked-by: Song Liu <song@kernel.org>

This allows bpf maps to hold references to files, just like a process
can hold references to files with file descriptors. With proper kfuncs,
this can unblock many use cases.

Re: [PATCH bpf-next 2/2] selftests/bpf: Add test for map-stored struct file kptrs

[Song Liu]

On Mon, Apr 20, 2026 at 1:34 PM Justin Suess <utilityemal77@gmail.com> wrote:
[...]
> +static void close_fd(int fd)
> +{
> +       if (fd >= 0)
> +               close(fd);
> +}

Do we really need close_fd()? close(-1) is allowed.

[...]

> diff --git a/tools/testing/selftests/bpf/progs/refcounted_kptr_file_fail.c b/tools/testing/selftests/bpf/progs/refcounted_kptr_file_fail.c
> new file mode 100644
> index 000000000000..7adfa6645801
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/progs/refcounted_kptr_file_fail.c
> @@ -0,0 +1,35 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +#include <vmlinux.h>
> +#include <bpf/bpf_helpers.h>
> +#include <bpf/bpf_tracing.h>
> +#include "bpf_misc.h"
> +
> +struct file_map_value {
> +       struct file __kptr * file;
> +};
> +
> +struct {
> +       __uint(type, BPF_MAP_TYPE_ARRAY);
> +       __type(key, int);
> +       __type(value, struct file_map_value);
> +       __uint(max_entries, 1);
> +} stashed_files SEC(".maps");
> +
> +SEC("lsm.s/file_open")
> +__failure __msg("R2 type=ctx expected=ptr_, trusted_ptr_, rcu_ptr_")
> +int stash_unref_ctx_file(struct file *ctx_file)
> +{
> +       struct file_map_value *mapval;
> +       int zero = 0;
> +
> +       mapval = bpf_map_lookup_elem(&stashed_files, &zero);
> +       if (!mapval)
> +               return 0;
> +
> +       /* ctx_file is just the hook argument, not an acquired file reference. */
> +       bpf_kptr_xchg(&mapval->file, ctx_file);
> +       return 0;
> +}

We can add more tests, just like task_kfunc_success.c and
task_kfunc_failure.c.

Thanks,
Song

Re: [PATCH bpf-next 1/2] bpf: Implement dtor for struct file BTF ID

[sashiko-bot]

Sashiko AI review found 1 potential issue(s):
- [High] bpf_file_release_dtor() directly calls fput(), which is unsafe in NMI context and can lead to a system deadlock.
--

commit b9990bf55467191e86e004a62cdb84e6dc71b794
Author: Justin Suess <utilityemal77@gmail.com>

bpf: Implement dtor for struct file BTF ID

This commit implements a destructor for struct file, allowing it to be stored
in a BPF map as a referenced kptr. The destructor calls fput() to release
the reference.

> diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
> index 2bb60200c2665..ee5ceb82e4f81 100644
> --- a/kernel/bpf/helpers.c
> +++ b/kernel/bpf/helpers.c
[ ... ]
> @@ -2752,6 +2753,13 @@ __bpf_kfunc void bpf_task_release_dtor(void *p)
>  }
>  CFI_NOSEAL(bpf_task_release_dtor);
>  
> +__bpf_kfunc void bpf_file_release_dtor(void *file)
> +{
> +	fput((struct file *)file);
> +}

Is it safe to call fput() here?

BPF programs can update or delete map elements from NMI context (for example,
in BPF_PROG_TYPE_PERF_EVENT programs), which will synchronously invoke this
destructor.

When fput() is called in an interrupt context, it adds the file to the
delayed_fput_list and calls schedule_delayed_work(). However,
schedule_delayed_work() is not NMI-safe, as it attempts to acquire spinlocks
such as the workqueue pool lock and timer_base->lock.

If the NMI interrupted a context that already held one of these locks, won't
this result in a hard deadlock?

Would it be better to defer releasing the struct file using an NMI-safe
mechanism, similar to how other BPF destructors handle this?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260420203306.3107246-1-utilityemal77@gmail.com?part=1

Re: [PATCH bpf-next 2/2] selftests/bpf: Add test for map-stored struct file kptrs

[sashiko-bot]

Sashiko AI review found 2 potential issue(s):
- [High] Wrapping `RUN_TESTS()` inside `test__start_subtest()` breaks subtest filtering and creates dummy subtests.
- [Low] Zombie process leak if an error occurs before `waitpid()`.
--

commit a0fbd925d86bff9e8b40a48dc8cff998dbbb50fc
Author: Justin Suess <utilityemal77@gmail.com>

selftests/bpf: Add test for map-stored struct file kptrs

This commit adds tests for map-stored referenced struct file pointers. It
verifies that the file reference remains valid after the original file
descriptor is closed, and checks that unreferenced struct file pointers are
rejected by the verifier.

> diff --git a/tools/testing/selftests/bpf/prog_tests/refcounted_kptr_file.c b/tools/testing/selftests/bpf/prog_tests/refcounted_kptr_file.c
> new file mode 100644
> index 0000000000000..173121377040c
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/prog_tests/refcounted_kptr_file.c
[ ... ]
> +	skel->bss->file_kptr_insert_pid = child_pid;
> +	err = write_full(pipefd[1], "1", 1);
> +	if (!ASSERT_OK(err, "start_child"))
> +		goto out;

If an error occurs here and the code jumps to the out label, does it leave
the child process as a zombie?

It looks like the cleanup path closes the pipe file descriptors which causes
the child to read EOF and exit, but waitpid() isn't called in the cleanup
path.

[ ... ]
> +void test_refcounted_kptr_file(void)
> +{
> +	if (test__start_subtest("holds_ref_after_close"))
> +		run_refcounted_file_kptr_success();
> +
> +	if (test__start_subtest("reject_unref_ctx_file"))
> +		RUN_TESTS(refcounted_kptr_file_fail);
> +}

Can wrapping RUN_TESTS() inside test__start_subtest() break test filtering?

RUN_TESTS() internally parses the skeleton and creates its own subtests for
each BPF program by calling test__start_subtest_with_desc().

Since the selftests framework doesn't support nested subtests, if a user
filters for the inner program name (like -t refcounted_kptr_file/stash_unref_ctx_file),
the outer test__start_subtest() will not match the filter and will skip
RUN_TESTS() completely.

Should RUN_TESTS() be called unconditionally at the top level of the test
function instead?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260420203306.3107246-1-utilityemal77@gmail.com?part=2

Re: [PATCH bpf-next 1/2] bpf: Implement dtor for struct file BTF ID

[Justin Suess]

On Tue, Apr 21, 2026 at 01:05:35AM +0000, sashiko-bot@kernel.org wrote:
> Sashiko AI review found 1 potential issue(s):
> - [High] bpf_file_release_dtor() directly calls fput(), which is unsafe in NMI context and can lead to a system deadlock.
> --
> 
> commit b9990bf55467191e86e004a62cdb84e6dc71b794
> Author: Justin Suess <utilityemal77@gmail.com>
> 
> bpf: Implement dtor for struct file BTF ID
> 
> This commit implements a destructor for struct file, allowing it to be stored
> in a BPF map as a referenced kptr. The destructor calls fput() to release
> the reference.
> 
> > diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
> > index 2bb60200c2665..ee5ceb82e4f81 100644
> > --- a/kernel/bpf/helpers.c
> > +++ b/kernel/bpf/helpers.c
> [ ... ]
> > @@ -2752,6 +2753,13 @@ __bpf_kfunc void bpf_task_release_dtor(void *p)
> >  }
> >  CFI_NOSEAL(bpf_task_release_dtor);
> >  
> > +__bpf_kfunc void bpf_file_release_dtor(void *file)
> > +{
> > +	fput((struct file *)file);
> > +}
> 
> Is it safe to call fput() here?
>
> BPF programs can update or delete map elements from NMI context (for example,
> in BPF_PROG_TYPE_PERF_EVENT programs), which will synchronously invoke this
> destructor.
> 
> When fput() is called in an interrupt context, it adds the file to the
> delayed_fput_list and calls schedule_delayed_work(). However,
> schedule_delayed_work() is not NMI-safe, as it attempts to acquire spinlocks
> such as the workqueue pool lock and timer_base->lock.
> 
> If the NMI interrupted a context that already held one of these locks, won't
> this result in a hard deadlock?
> 
> Would it be better to defer releasing the struct file using an NMI-safe
> mechanism, similar to how other BPF destructors handle this?
> 
BPF_PROG_TYPE_PERF_EVENT allows the kfunc bpf_put_file which also just
calls fput().

So if calling fput in BPF_PROG_TYPE_PERF_EVENT is safe for
bpf_put_file, why would it be unsafe in a dtor running in the same
context?

> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/20260420203306.3107246-1-utilityemal77@gmail.com?part=1

Re: [PATCH bpf-next 1/2] bpf: Implement dtor for struct file BTF ID

[Justin Suess]

On Mon, Apr 20, 2026 at 10:18:39PM -0400, Justin Suess wrote:
> On Tue, Apr 21, 2026 at 01:05:35AM +0000, sashiko-bot@kernel.org wrote:
> > Sashiko AI review found 1 potential issue(s):
> > - [High] bpf_file_release_dtor() directly calls fput(), which is unsafe in NMI context and can lead to a system deadlock.
> > --
> > 
> > commit b9990bf55467191e86e004a62cdb84e6dc71b794
> > Author: Justin Suess <utilityemal77@gmail.com>
> > 
> > bpf: Implement dtor for struct file BTF ID
> > 
> > This commit implements a destructor for struct file, allowing it to be stored
> > in a BPF map as a referenced kptr. The destructor calls fput() to release
> > the reference.
> > 
> > > diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
> > > index 2bb60200c2665..ee5ceb82e4f81 100644
> > > --- a/kernel/bpf/helpers.c
> > > +++ b/kernel/bpf/helpers.c
> > [ ... ]
> > > @@ -2752,6 +2753,13 @@ __bpf_kfunc void bpf_task_release_dtor(void *p)
> > >  }
> > >  CFI_NOSEAL(bpf_task_release_dtor);
> > >  
> > > +__bpf_kfunc void bpf_file_release_dtor(void *file)
> > > +{
> > > +	fput((struct file *)file);
> > > +}
> > 
> > Is it safe to call fput() here?
> >
> > BPF programs can update or delete map elements from NMI context (for example,
> > in BPF_PROG_TYPE_PERF_EVENT programs), which will synchronously invoke this
> > destructor.
> > 
> > When fput() is called in an interrupt context, it adds the file to the
> > delayed_fput_list and calls schedule_delayed_work(). However,
> > schedule_delayed_work() is not NMI-safe, as it attempts to acquire spinlocks
> > such as the workqueue pool lock and timer_base->lock.
> > 
> > If the NMI interrupted a context that already held one of these locks, won't
> > this result in a hard deadlock?
> > 
> > Would it be better to defer releasing the struct file using an NMI-safe
> > mechanism, similar to how other BPF destructors handle this?
> > 
> BPF_PROG_TYPE_PERF_EVENT allows the kfunc bpf_put_file which also just
> calls fput().
> 
> So if calling fput in BPF_PROG_TYPE_PERF_EVENT is safe for
> bpf_put_file, why would it be unsafe in a dtor running in the same
> context?
>
Disregard.

The AI was partially correct, but my course of investigation revealed a
real bug / reproducible deadlock in upstream.

In addition to my patch having an NMI unsafe dtor, the existing dtors
are also unsafe in NMI handlers.

I was able to make a reliable reproducer that deadlocks the kernel with
the task_struct dtor on upstream (without this patch).

I'll send the full report in a bit.

Justin
> > -- 
> > Sashiko AI review · https://sashiko.dev/#/patchset/20260420203306.3107246-1-utilityemal77@gmail.com?part=1

Re: [PATCH bpf-next 1/2] bpf: Implement dtor for struct file BTF ID

[Kumar Kartikeya Dwivedi]

On Tue, 21 Apr 2026 at 21:43, Justin Suess <utilityemal77@gmail.com> wrote:
>
> On Mon, Apr 20, 2026 at 10:18:39PM -0400, Justin Suess wrote:
> > On Tue, Apr 21, 2026 at 01:05:35AM +0000, sashiko-bot@kernel.org wrote:
> > > Sashiko AI review found 1 potential issue(s):
> > > - [High] bpf_file_release_dtor() directly calls fput(), which is unsafe in NMI context and can lead to a system deadlock.
> > > --
> > >
> > > commit b9990bf55467191e86e004a62cdb84e6dc71b794
> > > Author: Justin Suess <utilityemal77@gmail.com>
> > >
> > > bpf: Implement dtor for struct file BTF ID
> > >
> > > This commit implements a destructor for struct file, allowing it to be stored
> > > in a BPF map as a referenced kptr. The destructor calls fput() to release
> > > the reference.
> > >
> > > > diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
> > > > index 2bb60200c2665..ee5ceb82e4f81 100644
> > > > --- a/kernel/bpf/helpers.c
> > > > +++ b/kernel/bpf/helpers.c
> > > [ ... ]
> > > > @@ -2752,6 +2753,13 @@ __bpf_kfunc void bpf_task_release_dtor(void *p)
> > > >  }
> > > >  CFI_NOSEAL(bpf_task_release_dtor);
> > > >
> > > > +__bpf_kfunc void bpf_file_release_dtor(void *file)
> > > > +{
> > > > + fput((struct file *)file);
> > > > +}
> > >
> > > Is it safe to call fput() here?
> > >
> > > BPF programs can update or delete map elements from NMI context (for example,
> > > in BPF_PROG_TYPE_PERF_EVENT programs), which will synchronously invoke this
> > > destructor.
> > >
> > > When fput() is called in an interrupt context, it adds the file to the
> > > delayed_fput_list and calls schedule_delayed_work(). However,
> > > schedule_delayed_work() is not NMI-safe, as it attempts to acquire spinlocks
> > > such as the workqueue pool lock and timer_base->lock.
> > >
> > > If the NMI interrupted a context that already held one of these locks, won't
> > > this result in a hard deadlock?
> > >
> > > Would it be better to defer releasing the struct file using an NMI-safe
> > > mechanism, similar to how other BPF destructors handle this?
> > >
> > BPF_PROG_TYPE_PERF_EVENT allows the kfunc bpf_put_file which also just
> > calls fput().
> >
> > So if calling fput in BPF_PROG_TYPE_PERF_EVENT is safe for
> > bpf_put_file, why would it be unsafe in a dtor running in the same
> > context?
> >
> Disregard.
>
> The AI was partially correct, but my course of investigation revealed a
> real bug / reproducible deadlock in upstream.
>
> In addition to my patch having an NMI unsafe dtor, the existing dtors
> are also unsafe in NMI handlers.
>
> I was able to make a reliable reproducer that deadlocks the kernel with
> the task_struct dtor on upstream (without this patch).
>
> I'll send the full report in a bit.

This is an orthogonal issue, but what you're adding in this set makes
sense to me, but please respin after addressing the nits and some more
tests as Song suggested. Let's continue discussing the issue here in a
separate thread that you began.

pw-bot: cr

>
> Justin
> > > --
> > > Sashiko AI review · https://sashiko.dev/#/patchset/20260420203306.3107246-1-utilityemal77@gmail.com?part=1
>