linux-trace-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH bpf v4 0/2] bpf: fix bpf_d_path() helper prototype
@ 2025-12-04  7:46 Shuran Liu
  2025-12-04  7:46 ` [PATCH bpf v4 1/2] bpf: mark bpf_d_path() buffer as writeable Shuran Liu
  2025-12-04  7:46 ` [PATCH bpf v4 2/2] selftests/bpf: add regression test for bpf_d_path() Shuran Liu
  0 siblings, 2 replies; 6+ messages in thread
From: Shuran Liu @ 2025-12-04  7:46 UTC (permalink / raw)
  To: song, mattbobrowski, bpf
  Cc: ast, daniel, andrii, martin.lau, eddyz87, yonghong.song,
	john.fastabend, kpsingh, sdf, haoluo, jolsa, rostedt, mhiramat,
	mathieu.desnoyers, linux-kernel, linux-trace-kernel, dxu,
	linux-kselftest, shuah, electronlsr

Hi,

This series fixes a verifier issue with bpf_d_path() and adds a
regression test to cover its use within a hook function.

Patch 1 updates the bpf_d_path() helper prototype so that the second
argument is marked as MEM_WRITE. This makes it explicit to the verifier
that the helper writes into the provided buffer.

Patch 2 extends the existing d_path selftest to cover incorrect verifier
assumptions caused by an incorrect function prototype. The test program calls
bpf_d_path() and checks if the first character of the path is '/'.
It ensures the verifier does not assume the buffer remains unwritten.

Changelog
=========

v4:
  - Use the fallocate hook instead of an LSM hook to simplify the selftest,
    as suggested by Matt and Alexei.
  - Add a utility function in test_d_path.c to load the BPF program,
    improving code reuse.

v3:
  - Switch the pathname prefix loop to use bpf_for() instead of
    #pragma unroll, as suggested by Matt.
  - Remove /tmp/bpf_d_path_test in the test cleanup path.
  - Add the missing Reviewed-by tags.

v2:
  - Merge the new test into the existing d_path selftest rather than   
  creating new files.   
  - Add PID filtering in the LSM program to avoid nondeterministic failures   
  due to unrelated processes triggering bprm_check_security.   
  - Synchronize child execution using a pipe to ensure deterministic   
  updates to the PID. 

Thanks for your time and reviews.

Shuran Liu (2):
  bpf: mark bpf_d_path() buffer as writeable
  selftests/bpf: add regression test for bpf_d_path()

 kernel/trace/bpf_trace.c                      |  2 +-
 .../testing/selftests/bpf/prog_tests/d_path.c | 90 +++++++++++++++----
 .../testing/selftests/bpf/progs/test_d_path.c | 23 +++++
 3 files changed, 96 insertions(+), 19 deletions(-)

-- 
2.52.0


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

* [PATCH bpf v4 1/2] bpf: mark bpf_d_path() buffer as writeable
  2025-12-04  7:46 [PATCH bpf v4 0/2] bpf: fix bpf_d_path() helper prototype Shuran Liu
@ 2025-12-04  7:46 ` Shuran Liu
  2025-12-04  7:46 ` [PATCH bpf v4 2/2] selftests/bpf: add regression test for bpf_d_path() Shuran Liu
  1 sibling, 0 replies; 6+ messages in thread
From: Shuran Liu @ 2025-12-04  7:46 UTC (permalink / raw)
  To: song, mattbobrowski, bpf
  Cc: ast, daniel, andrii, martin.lau, eddyz87, yonghong.song,
	john.fastabend, kpsingh, sdf, haoluo, jolsa, rostedt, mhiramat,
	mathieu.desnoyers, linux-kernel, linux-trace-kernel, dxu,
	linux-kselftest, shuah, electronlsr, Zesen Liu, Peili Gao,
	Haoran Ni

Commit 37cce22dbd51 ("bpf: verifier: Refactor helper access type
tracking") started distinguishing read vs write accesses performed by
helpers.

The second argument of bpf_d_path() is a pointer to a buffer that the
helper fills with the resulting path. However, its prototype currently
uses ARG_PTR_TO_MEM without MEM_WRITE.

Before 37cce22dbd51, helper accesses were conservatively treated as
potential writes, so this mismatch did not cause issues. Since that
commit, the verifier may incorrectly assume that the buffer contents
are unchanged across the helper call and base its optimizations on this
wrong assumption. This can lead to misbehaviour in BPF programs that
read back the buffer, such as prefix comparisons on the returned path.

Fix this by marking the second argument of bpf_d_path() as
ARG_PTR_TO_MEM | MEM_WRITE so that the verifier correctly models the
write to the caller-provided buffer.

Fixes: 37cce22dbd51 ("bpf: verifier: Refactor helper access type tracking")
Co-developed-by: Zesen Liu <ftyg@live.com>
Signed-off-by: Zesen Liu <ftyg@live.com>
Co-developed-by: Peili Gao <gplhust955@gmail.com>
Signed-off-by: Peili Gao <gplhust955@gmail.com>
Co-developed-by: Haoran Ni <haoran.ni.cs@gmail.com>
Signed-off-by: Haoran Ni <haoran.ni.cs@gmail.com>
Signed-off-by: Shuran Liu <electronlsr@gmail.com>
Reviewed-by: Matt Bobrowski <mattbobrowski@google.com>
---
 kernel/trace/bpf_trace.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
index 4f87c16d915a..49e0bdaa7a1b 100644
--- a/kernel/trace/bpf_trace.c
+++ b/kernel/trace/bpf_trace.c
@@ -965,7 +965,7 @@ static const struct bpf_func_proto bpf_d_path_proto = {
 	.ret_type	= RET_INTEGER,
 	.arg1_type	= ARG_PTR_TO_BTF_ID,
 	.arg1_btf_id	= &bpf_d_path_btf_ids[0],
-	.arg2_type	= ARG_PTR_TO_MEM,
+	.arg2_type	= ARG_PTR_TO_MEM | MEM_WRITE,
 	.arg3_type	= ARG_CONST_SIZE_OR_ZERO,
 	.allowed	= bpf_d_path_allowed,
 };
-- 
2.52.0


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

* [PATCH bpf v4 2/2] selftests/bpf: add regression test for bpf_d_path()
  2025-12-04  7:46 [PATCH bpf v4 0/2] bpf: fix bpf_d_path() helper prototype Shuran Liu
  2025-12-04  7:46 ` [PATCH bpf v4 1/2] bpf: mark bpf_d_path() buffer as writeable Shuran Liu
@ 2025-12-04  7:46 ` Shuran Liu
  2025-12-04 12:38   ` Shuran Liu
  1 sibling, 1 reply; 6+ messages in thread
From: Shuran Liu @ 2025-12-04  7:46 UTC (permalink / raw)
  To: song, mattbobrowski, bpf
  Cc: ast, daniel, andrii, martin.lau, eddyz87, yonghong.song,
	john.fastabend, kpsingh, sdf, haoluo, jolsa, rostedt, mhiramat,
	mathieu.desnoyers, linux-kernel, linux-trace-kernel, dxu,
	linux-kselftest, shuah, electronlsr, Zesen Liu, Peili Gao,
	Haoran Ni

Add a regression test for bpf_d_path() to cover incorrect verifier
assumptions caused by an incorrect function prototype. The test
attaches to the fallocate hook, calls bpf_d_path() and verifies that
a simple prefix comparison on the returned pathname behaves correctly
after the fix in patch 1. It ensures the verifier does not assume
the buffer remains unwritten.

Co-developed-by: Zesen Liu <ftyg@live.com>
Signed-off-by: Zesen Liu <ftyg@live.com>
Co-developed-by: Peili Gao <gplhust955@gmail.com>
Signed-off-by: Peili Gao <gplhust955@gmail.com>
Co-developed-by: Haoran Ni <haoran.ni.cs@gmail.com>
Signed-off-by: Haoran Ni <haoran.ni.cs@gmail.com>
Signed-off-by: Shuran Liu <electronlsr@gmail.com>
---
 .../testing/selftests/bpf/prog_tests/d_path.c | 90 +++++++++++++++----
 .../testing/selftests/bpf/progs/test_d_path.c | 23 +++++
 2 files changed, 95 insertions(+), 18 deletions(-)

diff --git a/tools/testing/selftests/bpf/prog_tests/d_path.c b/tools/testing/selftests/bpf/prog_tests/d_path.c
index ccc768592e66..c725d5258e65 100644
--- a/tools/testing/selftests/bpf/prog_tests/d_path.c
+++ b/tools/testing/selftests/bpf/prog_tests/d_path.c
@@ -38,6 +38,14 @@ static int set_pathname(int fd, pid_t pid)
 	return readlink(buf, src.paths[src.cnt++], MAX_PATH_LEN);
 }
 
+static inline long syscall_close(int fd)
+{
+	return syscall(__NR_close_range,
+			(unsigned int)fd,
+			(unsigned int)fd,
+			0u);
+}
+
 static int trigger_fstat_events(pid_t pid)
 {
 	int sockfd = -1, procfd = -1, devfd = -1;
@@ -104,36 +112,47 @@ static int trigger_fstat_events(pid_t pid)
 	/* sys_close no longer triggers filp_close, but we can
 	 * call sys_close_range instead which still does
 	 */
-#define close(fd) syscall(__NR_close_range, fd, fd, 0)
-
-	close(pipefd[0]);
-	close(pipefd[1]);
-	close(sockfd);
-	close(procfd);
-	close(devfd);
-	close(localfd);
-	close(indicatorfd);
-
-#undef close
+	syscall_close(pipefd[0]);
+	syscall_close(pipefd[1]);
+	syscall_close(sockfd);
+	syscall_close(procfd);
+	syscall_close(devfd);
+	syscall_close(localfd);
+	syscall_close(indicatorfd);
 	return ret;
 }
 
+static void attach_and_load(struct test_d_path **skel)
+{
+	int err;
+
+	*skel = test_d_path__open_and_load();
+	if (CHECK(!*skel, "setup", "d_path skeleton failed\n"))
+		goto cleanup;
+
+	err = test_d_path__attach(*skel);
+	if (CHECK(err, "setup", "attach failed: %d\n", err))
+		goto cleanup;
+
+	(*skel)->bss->my_pid = getpid();
+	return;
+
+cleanup:
+	test_d_path__destroy(*skel);
+	*skel = NULL;
+}
+
 static void test_d_path_basic(void)
 {
 	struct test_d_path__bss *bss;
 	struct test_d_path *skel;
 	int err;
 
-	skel = test_d_path__open_and_load();
-	if (CHECK(!skel, "setup", "d_path skeleton failed\n"))
-		goto cleanup;
-
-	err = test_d_path__attach(skel);
-	if (CHECK(err, "setup", "attach failed: %d\n", err))
+	attach_and_load(&skel);
+	if (!skel)
 		goto cleanup;
 
 	bss = skel->bss;
-	bss->my_pid = getpid();
 
 	err = trigger_fstat_events(bss->my_pid);
 	if (err < 0)
@@ -195,6 +214,38 @@ static void test_d_path_check_types(void)
 	test_d_path_check_types__destroy(skel);
 }
 
+/* Check if the verifier correctly generates code for
+ * accessing the memory modified by d_path helper.
+ */
+static void test_d_path_mem_access(void)
+{
+	int localfd = -1;
+	struct test_d_path__bss *bss;
+	struct test_d_path *skel;
+
+	attach_and_load(&skel);
+	if (!skel)
+		goto cleanup;
+
+	bss = skel->bss;
+
+	localfd = open("/tmp/d_path_loadgen.txt", O_CREAT | O_RDWR, 0644);
+	if (CHECK(localfd < 0, "trigger", "open /tmp/d_path_loadgen.txt failed\n"))
+		goto cleanup;
+
+	if (CHECK(fallocate(localfd, 0, 0, 1024) < 0, "trigger", "fallocate failed\n"))
+		goto cleanup;
+	remove("/tmp/d_path_loadgen.txt");
+
+	if (CHECK(!bss->path_match_fallocate, "check",
+		  "failed to match actual opened path"))
+		goto cleanup;
+
+cleanup:
+	syscall_close(localfd);
+	test_d_path__destroy(skel);
+}
+
 void test_d_path(void)
 {
 	if (test__start_subtest("basic"))
@@ -205,4 +256,7 @@ void test_d_path(void)
 
 	if (test__start_subtest("check_alloc_mem"))
 		test_d_path_check_types();
+
+	if (test__start_subtest("check_mem_access"))
+		test_d_path_mem_access();
 }
diff --git a/tools/testing/selftests/bpf/progs/test_d_path.c b/tools/testing/selftests/bpf/progs/test_d_path.c
index 84e1f883f97b..2f9b4cb67931 100644
--- a/tools/testing/selftests/bpf/progs/test_d_path.c
+++ b/tools/testing/selftests/bpf/progs/test_d_path.c
@@ -17,6 +17,7 @@ int rets_close[MAX_FILES] = {};
 
 int called_stat = 0;
 int called_close = 0;
+int path_match_fallocate = 0;
 
 SEC("fentry/security_inode_getattr")
 int BPF_PROG(prog_stat, struct path *path, struct kstat *stat,
@@ -62,4 +63,26 @@ int BPF_PROG(prog_close, struct file *file, void *id)
 	return 0;
 }
 
+SEC("fentry/vfs_fallocate")
+int BPF_PROG(prog_fallocate, struct file *file, int mode, loff_t offset, loff_t len)
+{
+	pid_t pid = bpf_get_current_pid_tgid() >> 32;
+	int ret = 0;
+	char path_fallocate[MAX_PATH_LEN] = {};
+
+	if (pid != my_pid)
+		return 0;
+
+	ret = bpf_d_path(&file->f_path,
+			 path_fallocate, MAX_PATH_LEN);
+	if (ret < 0)
+		return 0;
+
+	if (path_fallocate[0] != '/')
+		return 0;
+
+	path_match_fallocate = 1;
+	return 0;
+}
+
 char _license[] SEC("license") = "GPL";
-- 
2.52.0


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

* Re: [PATCH bpf v4 2/2] selftests/bpf: add regression test for bpf_d_path()
  2025-12-04  7:46 ` [PATCH bpf v4 2/2] selftests/bpf: add regression test for bpf_d_path() Shuran Liu
@ 2025-12-04 12:38   ` Shuran Liu
  2025-12-04 23:47     ` Song Liu
  0 siblings, 1 reply; 6+ messages in thread
From: Shuran Liu @ 2025-12-04 12:38 UTC (permalink / raw)
  To: song, mattbobrowski, bpf
  Cc: ast, daniel, andrii, martin.lau, eddyz87, yonghong.song,
	john.fastabend, kpsingh, sdf, haoluo, jolsa, rostedt, mhiramat,
	mathieu.desnoyers, linux-kernel, linux-trace-kernel, dxu,
	linux-kselftest, shuah, electronlsr

Hi,

I looked into the CI failure and it’s caused by the test assuming
/tmp is on tmpfs, which is not true in the CI environment, so
fallocate() fails there. Since /dev/shm is mounted as tmpfs on that
setup, would it be acceptable to change the test to use a file under
/dev/shm instead of /tmp?

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

* Re: [PATCH bpf v4 2/2] selftests/bpf: add regression test for bpf_d_path()
  2025-12-04 12:38   ` Shuran Liu
@ 2025-12-04 23:47     ` Song Liu
  2025-12-06 14:06       ` Shuran Liu
  0 siblings, 1 reply; 6+ messages in thread
From: Song Liu @ 2025-12-04 23:47 UTC (permalink / raw)
  To: Shuran Liu
  Cc: mattbobrowski, bpf, ast, daniel, andrii, martin.lau, eddyz87,
	yonghong.song, john.fastabend, kpsingh, sdf, haoluo, jolsa,
	rostedt, mhiramat, mathieu.desnoyers, linux-kernel,
	linux-trace-kernel, dxu, linux-kselftest, shuah

On Thu, Dec 4, 2025 at 4:39 AM Shuran Liu <electronlsr@gmail.com> wrote:
>
> Hi,
>
> I looked into the CI failure and it’s caused by the test assuming
> /tmp is on tmpfs, which is not true in the CI environment, so
> fallocate() fails there. Since /dev/shm is mounted as tmpfs on that
> setup, would it be acceptable to change the test to use a file under
> /dev/shm instead of /tmp?

You can use mkstemp. There are a few examples in prog_tests.

Thanks,
Song

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

* Re: [PATCH bpf v4 2/2] selftests/bpf: add regression test for bpf_d_path()
  2025-12-04 23:47     ` Song Liu
@ 2025-12-06 14:06       ` Shuran Liu
  0 siblings, 0 replies; 6+ messages in thread
From: Shuran Liu @ 2025-12-06 14:06 UTC (permalink / raw)
  To: song
  Cc: andrii, ast, bpf, daniel, dxu, eddyz87, electronlsr, haoluo,
	john.fastabend, jolsa, kpsingh, linux-kernel, linux-kselftest,
	linux-trace-kernel, martin.lau, mathieu.desnoyers, mattbobrowski,
	mhiramat, rostedt, sdf, shuah, yonghong.song

Hi Song,

I have done further testing and found that the initial version of
selftest (LSM version) could not reliably reproduce the issue on a
buggy kernel. I have now verified that the new fallocate test
correctly reproduces the bug.

I will be sending the v5 patch shortly. (This is our first patch
submission to the kernel, thank you for your patience :D )

Best regards,
Shuran Liu

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

end of thread, other threads:[~2025-12-06 14:06 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-12-04  7:46 [PATCH bpf v4 0/2] bpf: fix bpf_d_path() helper prototype Shuran Liu
2025-12-04  7:46 ` [PATCH bpf v4 1/2] bpf: mark bpf_d_path() buffer as writeable Shuran Liu
2025-12-04  7:46 ` [PATCH bpf v4 2/2] selftests/bpf: add regression test for bpf_d_path() Shuran Liu
2025-12-04 12:38   ` Shuran Liu
2025-12-04 23:47     ` Song Liu
2025-12-06 14:06       ` Shuran Liu

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).