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

[PATCH bpf-next v2 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.

v1: https://lore.kernel.org/bpf/20260420203306.3107246-1-utilityemal77@gmail.com/

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 and success cases.

Changes for v2:

- Fix comment formatting based on sashiko feedback.
- Fix typo in commit based on sashiko feedback.
- Remove close_fd helper based on Song Liu's feedback.
- Expand selftests to include success and failure verifier tests.

Note on NMI safety:

An existing bug in NMI safety after further investigation from this
sashiko report [1] was found and reported here, with reproducer. [2]

Basically it is possible to invoke map dtors from within an nmi context
by attaching to specific tracepoints. Because the existing dtors rely on
rcu and/or locks, you can cause deadlocks. The fput added by this patch
is unsafe in nmi context and affected by this bug.

However, the solution requires deeper investigation into verifier safety
in NMI contexts, and is outside the scope of this patch.

[1] : https://lore.kernel.org/bpf/20260421010536.17FB1C19425@smtp.kernel.org/
[2] : https://lore.kernel.org/bpf/20260421201035.1729473-1-utilityemal77@gmail.com/

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     | 249 ++++++++++++++++++
 .../bpf/progs/refcounted_kptr_file.c          | 158 +++++++++++
 .../bpf/progs/refcounted_kptr_file_fail.c     | 141 ++++++++++
 .../bpf/progs/refcounted_kptr_file_success.c  |  61 +++++
 5 files changed, 624 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
 create mode 100644 tools/testing/selftests/bpf/progs/refcounted_kptr_file_success.c

-- 
2.53.0

[PATCH bpf-next v2 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.

Acked-by: Song Liu <song@kernel.org>
Cc: Kumar Kartikeya Dwivedi <memxor@gmail.com>
Signed-off-by: Justin Suess <utilityemal77@gmail.com>
---

Changes in v2:
  - None

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

Add fail tests for reference counting with the struct file objects.

Cc: Song Liu <song@kernel.org>
Cc: Kumar Kartikeya Dwivedi <memxor@gmail.com>
Signed-off-by: Justin Suess <utilityemal77@gmail.com>
---

Changes in v2:
  - Add success verifier tests per Song Liu's suggestion.
  - Remove close_fd per Song Liu's suggestion.
  - Fix typos in commit per Sashiko.
  - Fix subtest ordering per Sashiko.
  - Fix comments per Sashiko.

 .../bpf/prog_tests/refcounted_kptr_file.c     | 249 ++++++++++++++++++
 .../bpf/progs/refcounted_kptr_file.c          | 158 +++++++++++
 .../bpf/progs/refcounted_kptr_file_fail.c     | 141 ++++++++++
 .../bpf/progs/refcounted_kptr_file_success.c  |  61 +++++
 4 files changed, 609 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
 create mode 100644 tools/testing/selftests/bpf/progs/refcounted_kptr_file_success.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..1f2586a7af93
--- /dev/null
+++ b/tools/testing/selftests/bpf/prog_tests/refcounted_kptr_file.c
@@ -0,0 +1,249 @@
+// 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"
+#include "refcounted_kptr_file_success.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 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) {
+			unlink(dst_path);
+			goto out;
+		}
+	}
+
+out:
+	err = close(src_fd) ?: err;
+	err = close(dst_fd) ?: err;
+	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 = -1;
+	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;
+	child_pid = -1;
+	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);
+	close(pipefd[0]);
+	close(pipefd[1]);
+	if (child_pid > 0)
+		(void)waitpid(child_pid, NULL, 0);
+	if (shell_copy[0])
+		unlink(shell_copy);
+	if (temp_dir[0])
+		rmdir(temp_dir);
+	refcounted_kptr_file__destroy(skel);
+}
+
+void test_refcounted_kptr_file_runtime(void)
+{
+	if (test__start_subtest("holds_ref_after_close"))
+		run_refcounted_file_kptr_success();
+}
+
+void test_refcounted_kptr_file(void)
+{
+	RUN_TESTS(refcounted_kptr_file_success);
+
+	RUN_TESTS(refcounted_kptr_file_fail);
+
+	RUN_TESTS(refcounted_kptr_file);
+}
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..510ea1107791
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/refcounted_kptr_file.c
@@ -0,0 +1,158 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include <vmlinux.h>
+#include <bpf/bpf_helpers.h>
+#include <bpf/bpf_tracing.h>
+#include "bpf_misc.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")
+__description("file_kptr: insert referenced file")
+__success
+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")
+__description("file_kptr: verify referenced file")
+__success
+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..67fea792960d
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/refcounted_kptr_file_fail.c
@@ -0,0 +1,141 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include <vmlinux.h>
+#include <bpf/bpf_helpers.h>
+#include <bpf/bpf_tracing.h>
+#include "bpf_misc.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");
+
+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;
+}
+
+SEC("lsm.s/file_open")
+__failure
+__msg("Unreleased reference")
+int stash_xchg_file_kptr_unreleased(struct file *ctx_file)
+{
+	struct file_map_value *mapval;
+	struct file *file = NULL;
+	int zero = 0;
+
+	(void)ctx_file;
+
+	mapval = bpf_map_lookup_elem(&stashed_files, &zero);
+	if (!mapval)
+		return 0;
+
+	file = bpf_get_task_exe_file(bpf_get_current_task_btf());
+	if (!file)
+		return 0;
+
+	file = bpf_kptr_xchg(&mapval->file, file);
+	if (file)
+		bpf_put_file(file);
+
+	file = bpf_kptr_xchg(&mapval->file, NULL);
+	if (!file)
+		return 0;
+
+	/* Retrieved kptr is never released. */
+	return 0;
+}
+
+SEC("lsm.s/file_open")
+__failure
+__msg("Possibly NULL pointer passed to trusted arg0")
+int stash_xchg_file_kptr_no_null_check(struct file *ctx_file)
+{
+	struct file_map_value *mapval;
+	struct file *file = NULL;
+	int zero = 0;
+
+	(void)ctx_file;
+
+	mapval = bpf_map_lookup_elem(&stashed_files, &zero);
+	if (!mapval)
+		return 0;
+
+	file = bpf_get_task_exe_file(bpf_get_current_task_btf());
+	if (!file)
+		return 0;
+
+	file = bpf_kptr_xchg(&mapval->file, file);
+	if (file)
+		bpf_put_file(file);
+
+	file = bpf_kptr_xchg(&mapval->file, NULL);
+	bpf_put_file(file);
+	return 0;
+}
+
+SEC("lsm.s/file_open")
+__failure
+__msg("R1 must be referenced or trusted")
+int put_map_owned_file(struct file *ctx_file)
+{
+	struct file_map_value *mapval;
+	struct file *file = NULL;
+	int zero = 0;
+
+	(void)ctx_file;
+
+	mapval = bpf_map_lookup_elem(&stashed_files, &zero);
+	if (!mapval)
+		return 0;
+
+	file = bpf_get_task_exe_file(bpf_get_current_task_btf());
+	if (!file)
+		return 0;
+
+	file = bpf_kptr_xchg(&mapval->file, file);
+	if (file)
+		bpf_put_file(file);
+
+	bpf_rcu_read_lock();
+	file = mapval->file;
+	if (!file) {
+		bpf_rcu_read_unlock();
+		return 0;
+	}
+
+	/* Can't release a kptr while it is still owned by the map. */
+	bpf_put_file(file);
+	bpf_rcu_read_unlock();
+	return 0;
+}
+
+SEC("lsm.s/file_open")
+__failure
+__msg("release kernel function bpf_put_file expects")
+int BPF_PROG(put_unref_ctx_file, struct file *file)
+{
+	/* Can't release the unacquired LSM hook argument. */
+	bpf_put_file(file);
+	return 0;
+}
+
+char _license[] SEC("license") = "GPL";
diff --git a/tools/testing/selftests/bpf/progs/refcounted_kptr_file_success.c b/tools/testing/selftests/bpf/progs/refcounted_kptr_file_success.c
new file mode 100644
index 000000000000..ece00c9e584f
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/refcounted_kptr_file_success.c
@@ -0,0 +1,61 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include <vmlinux.h>
+#include <bpf/bpf_helpers.h>
+#include <bpf/bpf_tracing.h>
+
+#include "bpf_misc.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");
+
+SEC("lsm.s/file_open")
+__description("file_kptr: xchg and reinsert")
+__success
+int xchg_reinsert_file_kptr(struct file *ctx_file)
+{
+	struct file_map_value *mapval;
+	struct file *file, *old;
+	int zero = 0;
+
+	(void)ctx_file;
+
+	mapval = bpf_map_lookup_elem(&stashed_files, &zero);
+	if (!mapval)
+		return 0;
+
+	file = bpf_get_task_exe_file(bpf_get_current_task_btf());
+	if (!file)
+		return 0;
+
+	old = bpf_kptr_xchg(&mapval->file, file);
+	if (old)
+		bpf_put_file(old);
+
+	file = bpf_kptr_xchg(&mapval->file, NULL);
+	if (!file)
+		return 0;
+
+	old = bpf_kptr_xchg(&mapval->file, file);
+	if (old) {
+		bpf_put_file(old);
+		return 0;
+	}
+
+	file = bpf_kptr_xchg(&mapval->file, NULL);
+	if (file)
+		bpf_put_file(file);
+
+	return 0;
+}
+
+char _license[] SEC("license") = "GPL";
-- 
2.53.0

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

[bot+bpf-ci]

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

> 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

[ ... ]

> @@ -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);
> +

Is bpf_file_release_dtor() safe to call from NMI context? BPF programs
can 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():

kernel/bpf/helpers.c:bpf_file_release_dtor() {
    fput((struct file *)file);
}

fs/file_table.c:fput() {
    ...
    if (!atomic_long_dec_and_test(&file->f_count))
        return;
    ...
    if (likely(!in_interrupt() && !(task->flags & PF_KTHREAD))) {
        ...
    } else {
        fput_many(file, 1);
    }
}

fs/file_table.c:fput_many() {
    ...
    if (llist_add_batch(&head, &tail, &delayed_fput_list))
        schedule_delayed_work(&delayed_fput_work, 1);
}

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?

This issue was raised by sashiko-bot in v1:
https://lore.kernel.org/bpf/20260421010536.17FB1C19425@smtp.kernel.org/

The author acknowledged this as a real bug with a reproducible deadlock,
but v2 still uses the same fput() call without any NMI-safe deferral
mechanism.

[ ... ]


---
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/24844511587

Re: [PATCH bpf-next v2 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 calls fput(), which is not NMI-safe and can cause a system deadlock when a struct file kptr is dropped from NMI context.
--

commit 37dfae0a149d4a18858208a4dfafb133d94b8e0d
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 map as a referenced kptr. The destructor relies on fput() to
release the file 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);

Can calling fput() here lead to a system deadlock if the destructor is
invoked from NMI context?

When a map element containing a struct file kptr is replaced or deleted, BPF
synchronously calls the destructor on the old pointer. BPF map updates can
be performed by programs executing in NMI context, such as perf event
programs or fentry programs attached to NMI handlers.

If the file's refcount reaches 0, fput() invokes __fput_deferred(). In NMI
context, __fput_deferred() skips task_work_add() and falls back to calling
schedule_delayed_work().

The schedule_delayed_work() function queues the work using add_timer_global()
or add_timer_on(), which acquire the timer_base raw spinlock. If the NMI
preempted code that was already holding this spinlock, the system will
deadlock.

Does dropping file references from BPF map destructors need to be deferred
using an NMI-safe mechanism, such as irq_work?

> +}
> +
> +CFI_NOSEAL(bpf_file_release_dtor);

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

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

[sashiko-bot]

Sashiko AI review found 1 potential issue(s):
- [Medium] In `copy_file`, closing an invalid file descriptor overwrites the original error code.
--

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

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

This commit adds tests for map-stored struct file referenced kptrs. It
validates that the struct file reference remains valid when 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 00000000000000..1f2586a7af9365
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/prog_tests/refcounted_kptr_file.c

[ ... ]

> +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;
> +	}

[ ... ]

> +out:
> +	err = close(src_fd) ?: err;
> +	err = close(dst_fd) ?: err;

If opening the destination file fails, dst_fd is left as -1, and the code jumps
here with err set to -errno. Would calling close(-1) here fail and return -1,
causing the ?: operator to overwrite the meaningful error code in err with -1?

Should the file descriptors be guarded with a >= 0 check before calling
close() to prevent masking the true cause of the failure?

> +	return err;
> +}

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