linux-trace-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH bpf v3 0/2] bpf: fix bpf_d_path() helper prototype
@ 2025-12-02 14:19 Shuran Liu
  2025-12-02 14:19 ` [PATCH bpf v3 1/2] bpf: mark bpf_d_path() buffer as writeable Shuran Liu
  2025-12-02 14:19 ` [PATCH bpf v3 2/2] selftests/bpf: fix and consolidate d_path LSM regression test Shuran Liu
  0 siblings, 2 replies; 9+ messages in thread
From: Shuran Liu @ 2025-12-02 14:19 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 from an LSM program.

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 also cover the LSM
bprm_check_security hook. The LSM program calls bpf_d_path() on the
binary being executed and performs a simple prefix comparison on the
resulting pathname. To avoid nondeterminism, the program filters based
on an expected PID that is populated from userspace before the test
binary is executed, and the parent and child processes are synchronized
through a pipe so that the PID is set before exec. The test now uses
bpf_for() to express the small fixed-iteration loop in a
verifier-friendly way, and it removes the temporary /tmp/bpf_d_path_test
binary in the cleanup path.

Changelog
=========

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: fix and consolidate d_path LSM regression test

 kernel/trace/bpf_trace.c                      |  2 +-
 .../testing/selftests/bpf/prog_tests/d_path.c | 65 +++++++++++++++++++
 .../testing/selftests/bpf/progs/test_d_path.c | 33 ++++++++++
 3 files changed, 99 insertions(+), 1 deletion(-)

-- 
2.52.0


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

* [PATCH bpf v3 1/2] bpf: mark bpf_d_path() buffer as writeable
  2025-12-02 14:19 [PATCH bpf v3 0/2] bpf: fix bpf_d_path() helper prototype Shuran Liu
@ 2025-12-02 14:19 ` Shuran Liu
  2025-12-02 14:19 ` [PATCH bpf v3 2/2] selftests/bpf: fix and consolidate d_path LSM regression test Shuran Liu
  1 sibling, 0 replies; 9+ messages in thread
From: Shuran Liu @ 2025-12-02 14:19 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] 9+ messages in thread

* [PATCH bpf v3 2/2] selftests/bpf: fix and consolidate d_path LSM regression test
  2025-12-02 14:19 [PATCH bpf v3 0/2] bpf: fix bpf_d_path() helper prototype Shuran Liu
  2025-12-02 14:19 ` [PATCH bpf v3 1/2] bpf: mark bpf_d_path() buffer as writeable Shuran Liu
@ 2025-12-02 14:19 ` Shuran Liu
  2025-12-02 18:39   ` Song Liu
  2025-12-03  1:21   ` Alexei Starovoitov
  1 sibling, 2 replies; 9+ messages in thread
From: Shuran Liu @ 2025-12-02 14:19 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() when invoked from an LSM program.
The test attaches to the bprm_check_security hook, calls bpf_d_path() on
the binary being executed, and verifies that a simple prefix comparison on
the returned pathname behaves correctly after the fix in patch 1.

To avoid nondeterminism, the LSM program now filters based on the
expected PID, which is populated from userspace before the test binary is
executed. This prevents unrelated processes that also trigger the
bprm_check_security LSM hook from overwriting test results. Parent and
child processes are synchronized through a pipe to ensure the PID is set
before the child execs the test binary.

Per review feedback, the new LSM coverage is merged into the existing
d_path selftest rather than adding new prog_tests/ or progs/ files. The
loop that checks the pathname prefix now uses bpf_for(), which is a
verifier-friendly way to express a small, fixed-iteration loop, and the
temporary /tmp/bpf_d_path_test binary is removed in the test cleanup
path.

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>
---
 .../testing/selftests/bpf/prog_tests/d_path.c | 65 +++++++++++++++++++
 .../testing/selftests/bpf/progs/test_d_path.c | 33 ++++++++++
 2 files changed, 98 insertions(+)

diff --git a/tools/testing/selftests/bpf/prog_tests/d_path.c b/tools/testing/selftests/bpf/prog_tests/d_path.c
index ccc768592e66..202b44e6f482 100644
--- a/tools/testing/selftests/bpf/prog_tests/d_path.c
+++ b/tools/testing/selftests/bpf/prog_tests/d_path.c
@@ -195,6 +195,68 @@ static void test_d_path_check_types(void)
 	test_d_path_check_types__destroy(skel);
 }
 
+static void test_d_path_lsm(void)
+{
+	struct test_d_path *skel;
+	int err;
+	int pipefd[2];
+	pid_t pid;
+
+	skel = test_d_path__open_and_load();
+	if (!ASSERT_OK_PTR(skel, "d_path skeleton failed"))
+		return;
+
+	err = test_d_path__attach(skel);
+	if (!ASSERT_OK(err, "attach failed"))
+		goto cleanup;
+
+	/* Prepare the test binary */
+	system("cp /bin/true /tmp/bpf_d_path_test 2>/dev/null || :");
+
+	if (!ASSERT_OK(pipe(pipefd), "pipe failed"))
+		goto cleanup;
+
+	pid = fork();
+	if (!ASSERT_GE(pid, 0, "fork failed")) {
+		close(pipefd[0]);
+		close(pipefd[1]);
+		goto cleanup;
+	}
+
+	if (pid == 0) {
+		/* Child */
+		char buf;
+
+		close(pipefd[1]);
+		/* Wait for parent to set PID in BPF map */
+		if (read(pipefd[0], &buf, 1) != 1)
+			exit(1);
+		close(pipefd[0]);
+		execl("/tmp/bpf_d_path_test", "/tmp/bpf_d_path_test", NULL);
+		exit(1);
+	}
+
+	/* Parent */
+	close(pipefd[0]);
+
+	/* Update BPF map with child PID */
+	skel->bss->my_pid = pid;
+
+	/* Signal child to proceed */
+	write(pipefd[1], "G", 1);
+	close(pipefd[1]);
+
+	/* Wait for child */
+	waitpid(pid, NULL, 0);
+
+	ASSERT_EQ(skel->bss->called_lsm, 1, "lsm hook called");
+	ASSERT_EQ(skel->bss->lsm_match, 1, "lsm match");
+
+cleanup:
+	unlink("/tmp/bpf_d_path_test");
+	test_d_path__destroy(skel);
+}
+
 void test_d_path(void)
 {
 	if (test__start_subtest("basic"))
@@ -205,4 +267,7 @@ void test_d_path(void)
 
 	if (test__start_subtest("check_alloc_mem"))
 		test_d_path_check_types();
+
+	if (test__start_subtest("lsm"))
+		test_d_path_lsm();
 }
diff --git a/tools/testing/selftests/bpf/progs/test_d_path.c b/tools/testing/selftests/bpf/progs/test_d_path.c
index 84e1f883f97b..9ae36eabcd07 100644
--- a/tools/testing/selftests/bpf/progs/test_d_path.c
+++ b/tools/testing/selftests/bpf/progs/test_d_path.c
@@ -17,6 +17,8 @@ int rets_close[MAX_FILES] = {};
 
 int called_stat = 0;
 int called_close = 0;
+int called_lsm = 0;
+int lsm_match = 0;
 
 SEC("fentry/security_inode_getattr")
 int BPF_PROG(prog_stat, struct path *path, struct kstat *stat,
@@ -62,4 +64,35 @@ int BPF_PROG(prog_close, struct file *file, void *id)
 	return 0;
 }
 
+SEC("lsm/bprm_check_security")
+int BPF_PROG(prog_lsm, struct linux_binprm *bprm)
+{
+	pid_t pid = bpf_get_current_pid_tgid() >> 32;
+	char path[MAX_PATH_LEN] = {};
+	int ret;
+
+	if (pid != my_pid)
+		return 0;
+
+	called_lsm = 1;
+	ret = bpf_d_path(&bprm->file->f_path, path, MAX_PATH_LEN);
+	if (ret < 0)
+		return 0;
+
+	{
+		static const char target_dir[] = "/tmp/";
+		int i;
+
+		bpf_for(i, 0, sizeof(target_dir) - 1) {
+			if (path[i] != target_dir[i]) {
+				lsm_match = -1; /* mismatch */
+				return 0;
+			}
+		}
+	}
+
+	lsm_match = 1; /* prefix match */
+	return 0;
+}
+
 char _license[] SEC("license") = "GPL";
-- 
2.52.0


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

* Re: [PATCH bpf v3 2/2] selftests/bpf: fix and consolidate d_path LSM regression test
  2025-12-02 14:19 ` [PATCH bpf v3 2/2] selftests/bpf: fix and consolidate d_path LSM regression test Shuran Liu
@ 2025-12-02 18:39   ` Song Liu
  2025-12-04  4:34     ` Shuran Liu
  2025-12-03  1:21   ` Alexei Starovoitov
  1 sibling, 1 reply; 9+ messages in thread
From: Song Liu @ 2025-12-02 18:39 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, Zesen Liu,
	Peili Gao, Haoran Ni

On Tue, Dec 2, 2025 at 6:20 AM Shuran Liu <electronlsr@gmail.com> wrote:
>
> Add a regression test for bpf_d_path() when invoked from an LSM program.
> The test attaches to the bprm_check_security hook, calls bpf_d_path() on
> the binary being executed, and verifies that a simple prefix comparison on
> the returned pathname behaves correctly after the fix in patch 1.

I don't get why we add this selftest here. It doesn't appear to be related to
patch 1/2.

>
> To avoid nondeterminism, the LSM program now filters based on the
> expected PID, which is populated from userspace before the test binary is
> executed. This prevents unrelated processes that also trigger the
> bprm_check_security LSM hook from overwriting test results. Parent and
> child processes are synchronized through a pipe to ensure the PID is set
> before the child execs the test binary.

The paragraph above is not really necessary. Just curious, did some AI
write it?

>
> Per review feedback, the new LSM coverage is merged into the existing
> d_path selftest rather than adding new prog_tests/ or progs/ files. The
> loop that checks the pathname prefix now uses bpf_for(), which is a
> verifier-friendly way to express a small, fixed-iteration loop, and the
> temporary /tmp/bpf_d_path_test binary is removed in the test cleanup
> path.
>
> 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>
> ---
>  .../testing/selftests/bpf/prog_tests/d_path.c | 65 +++++++++++++++++++
>  .../testing/selftests/bpf/progs/test_d_path.c | 33 ++++++++++
>  2 files changed, 98 insertions(+)
>
> diff --git a/tools/testing/selftests/bpf/prog_tests/d_path.c b/tools/testing/selftests/bpf/prog_tests/d_path.c
> index ccc768592e66..202b44e6f482 100644
> --- a/tools/testing/selftests/bpf/prog_tests/d_path.c
> +++ b/tools/testing/selftests/bpf/prog_tests/d_path.c
> @@ -195,6 +195,68 @@ static void test_d_path_check_types(void)
>         test_d_path_check_types__destroy(skel);
>  }
>
> +static void test_d_path_lsm(void)
> +{
> +       struct test_d_path *skel;
> +       int err;
> +       int pipefd[2];
> +       pid_t pid;
> +
> +       skel = test_d_path__open_and_load();
> +       if (!ASSERT_OK_PTR(skel, "d_path skeleton failed"))
> +               return;
> +
> +       err = test_d_path__attach(skel);
> +       if (!ASSERT_OK(err, "attach failed"))
> +               goto cleanup;
> +
> +       /* Prepare the test binary */
> +       system("cp /bin/true /tmp/bpf_d_path_test 2>/dev/null || :");
> +
> +       if (!ASSERT_OK(pipe(pipefd), "pipe failed"))
> +               goto cleanup;
> +
> +       pid = fork();
> +       if (!ASSERT_GE(pid, 0, "fork failed")) {
> +               close(pipefd[0]);
> +               close(pipefd[1]);
> +               goto cleanup;
> +       }
> +
> +       if (pid == 0) {
> +               /* Child */
> +               char buf;
> +
> +               close(pipefd[1]);
> +               /* Wait for parent to set PID in BPF map */
> +               if (read(pipefd[0], &buf, 1) != 1)
> +                       exit(1);
> +               close(pipefd[0]);
> +               execl("/tmp/bpf_d_path_test", "/tmp/bpf_d_path_test", NULL);
> +               exit(1);
> +       }
> +
> +       /* Parent */
> +       close(pipefd[0]);
> +
> +       /* Update BPF map with child PID */
> +       skel->bss->my_pid = pid;
> +
> +       /* Signal child to proceed */
> +       write(pipefd[1], "G", 1);
> +       close(pipefd[1]);
> +
> +       /* Wait for child */
> +       waitpid(pid, NULL, 0);
> +
> +       ASSERT_EQ(skel->bss->called_lsm, 1, "lsm hook called");
> +       ASSERT_EQ(skel->bss->lsm_match, 1, "lsm match");
> +
> +cleanup:
> +       unlink("/tmp/bpf_d_path_test");
> +       test_d_path__destroy(skel);
> +}
> +
>  void test_d_path(void)
>  {
>         if (test__start_subtest("basic"))
> @@ -205,4 +267,7 @@ void test_d_path(void)
>
>         if (test__start_subtest("check_alloc_mem"))
>                 test_d_path_check_types();
> +
> +       if (test__start_subtest("lsm"))
> +               test_d_path_lsm();
>  }
> diff --git a/tools/testing/selftests/bpf/progs/test_d_path.c b/tools/testing/selftests/bpf/progs/test_d_path.c
> index 84e1f883f97b..9ae36eabcd07 100644
> --- a/tools/testing/selftests/bpf/progs/test_d_path.c
> +++ b/tools/testing/selftests/bpf/progs/test_d_path.c
> @@ -17,6 +17,8 @@ int rets_close[MAX_FILES] = {};
>
>  int called_stat = 0;
>  int called_close = 0;
> +int called_lsm = 0;
> +int lsm_match = 0;
>
>  SEC("fentry/security_inode_getattr")
>  int BPF_PROG(prog_stat, struct path *path, struct kstat *stat,
> @@ -62,4 +64,35 @@ int BPF_PROG(prog_close, struct file *file, void *id)
>         return 0;
>  }
>
> +SEC("lsm/bprm_check_security")
> +int BPF_PROG(prog_lsm, struct linux_binprm *bprm)
> +{
> +       pid_t pid = bpf_get_current_pid_tgid() >> 32;
> +       char path[MAX_PATH_LEN] = {};
> +       int ret;
> +
> +       if (pid != my_pid)
> +               return 0;
> +
> +       called_lsm = 1;
> +       ret = bpf_d_path(&bprm->file->f_path, path, MAX_PATH_LEN);
> +       if (ret < 0)
> +               return 0;
> +
> +       {

This {} block is not necessary.

> +               static const char target_dir[] = "/tmp/";
> +               int i;
> +
> +               bpf_for(i, 0, sizeof(target_dir) - 1) {
> +                       if (path[i] != target_dir[i]) {
> +                               lsm_match = -1; /* mismatch */
> +                               return 0;
> +                       }
> +               }
> +       }
> +
> +       lsm_match = 1; /* prefix match */
> +       return 0;
> +}
> +
>  char _license[] SEC("license") = "GPL";
> --
> 2.52.0
>

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

* Re: [PATCH bpf v3 2/2] selftests/bpf: fix and consolidate d_path LSM regression test
  2025-12-02 14:19 ` [PATCH bpf v3 2/2] selftests/bpf: fix and consolidate d_path LSM regression test Shuran Liu
  2025-12-02 18:39   ` Song Liu
@ 2025-12-03  1:21   ` Alexei Starovoitov
  2025-12-03 10:32     ` Matt Bobrowski
  1 sibling, 1 reply; 9+ messages in thread
From: Alexei Starovoitov @ 2025-12-03  1:21 UTC (permalink / raw)
  To: Shuran Liu
  Cc: Song Liu, Matt Bobrowski, bpf, Alexei Starovoitov,
	Daniel Borkmann, Andrii Nakryiko, Martin KaFai Lau, Eduard,
	Yonghong Song, John Fastabend, KP Singh, Stanislav Fomichev,
	Hao Luo, Jiri Olsa, Steven Rostedt, Masami Hiramatsu,
	Mathieu Desnoyers, LKML, linux-trace-kernel, Daniel Xu,
	open list:KERNEL SELFTEST FRAMEWORK, Shuah Khan, Zesen Liu,
	Peili Gao, Haoran Ni

On Tue, Dec 2, 2025 at 6:20 AM Shuran Liu <electronlsr@gmail.com> wrote:
>
> Add a regression test for bpf_d_path() when invoked from an LSM program.
> The test attaches to the bprm_check_security hook, calls bpf_d_path() on
> the binary being executed, and verifies that a simple prefix comparison on
> the returned pathname behaves correctly after the fix in patch 1.
>
> To avoid nondeterminism, the LSM program now filters based on the
> expected PID, which is populated from userspace before the test binary is
> executed. This prevents unrelated processes that also trigger the
> bprm_check_security LSM hook from overwriting test results. Parent and
> child processes are synchronized through a pipe to ensure the PID is set
> before the child execs the test binary.
>
> Per review feedback, the new LSM coverage is merged into the existing
> d_path selftest rather than adding new prog_tests/ or progs/ files. The
> loop that checks the pathname prefix now uses bpf_for(), which is a
> verifier-friendly way to express a small, fixed-iteration loop, and the
> temporary /tmp/bpf_d_path_test binary is removed in the test cleanup
> path.
>
> 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>
> ---
>  .../testing/selftests/bpf/prog_tests/d_path.c | 65 +++++++++++++++++++
>  .../testing/selftests/bpf/progs/test_d_path.c | 33 ++++++++++
>  2 files changed, 98 insertions(+)
>
> diff --git a/tools/testing/selftests/bpf/prog_tests/d_path.c b/tools/testing/selftests/bpf/prog_tests/d_path.c
> index ccc768592e66..202b44e6f482 100644
> --- a/tools/testing/selftests/bpf/prog_tests/d_path.c
> +++ b/tools/testing/selftests/bpf/prog_tests/d_path.c
> @@ -195,6 +195,68 @@ static void test_d_path_check_types(void)
>         test_d_path_check_types__destroy(skel);
>  }
>
> +static void test_d_path_lsm(void)
> +{
> +       struct test_d_path *skel;
> +       int err;
> +       int pipefd[2];
> +       pid_t pid;
> +
> +       skel = test_d_path__open_and_load();
> +       if (!ASSERT_OK_PTR(skel, "d_path skeleton failed"))
> +               return;
> +
> +       err = test_d_path__attach(skel);
> +       if (!ASSERT_OK(err, "attach failed"))
> +               goto cleanup;
> +
> +       /* Prepare the test binary */
> +       system("cp /bin/true /tmp/bpf_d_path_test 2>/dev/null || :");
> +
> +       if (!ASSERT_OK(pipe(pipefd), "pipe failed"))
> +               goto cleanup;
> +
> +       pid = fork();
> +       if (!ASSERT_GE(pid, 0, "fork failed")) {
> +               close(pipefd[0]);
> +               close(pipefd[1]);
> +               goto cleanup;
> +       }
> +
> +       if (pid == 0) {
> +               /* Child */
> +               char buf;
> +
> +               close(pipefd[1]);
> +               /* Wait for parent to set PID in BPF map */
> +               if (read(pipefd[0], &buf, 1) != 1)
> +                       exit(1);
> +               close(pipefd[0]);
> +               execl("/tmp/bpf_d_path_test", "/tmp/bpf_d_path_test", NULL);
> +               exit(1);
> +       }

No forks please. They often make selftest to be flaky.
Use simples possible way to test it.
Without forks and pipes.

pw-bot: cr

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

* Re: [PATCH bpf v3 2/2] selftests/bpf: fix and consolidate d_path LSM regression test
  2025-12-03  1:21   ` Alexei Starovoitov
@ 2025-12-03 10:32     ` Matt Bobrowski
  2025-12-04  4:39       ` Shuran Liu
  0 siblings, 1 reply; 9+ messages in thread
From: Matt Bobrowski @ 2025-12-03 10:32 UTC (permalink / raw)
  To: Alexei Starovoitov
  Cc: Shuran Liu, Song Liu, bpf, Alexei Starovoitov, Daniel Borkmann,
	Andrii Nakryiko, Martin KaFai Lau, Eduard, Yonghong Song,
	John Fastabend, KP Singh, Stanislav Fomichev, Hao Luo, Jiri Olsa,
	Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers, LKML,
	linux-trace-kernel, Daniel Xu,
	open list:KERNEL SELFTEST FRAMEWORK, Shuah Khan, Zesen Liu,
	Peili Gao, Haoran Ni

On Tue, Dec 02, 2025 at 05:21:59PM -0800, Alexei Starovoitov wrote:
> On Tue, Dec 2, 2025 at 6:20 AM Shuran Liu <electronlsr@gmail.com> wrote:
> >
> > Add a regression test for bpf_d_path() when invoked from an LSM program.
> > The test attaches to the bprm_check_security hook, calls bpf_d_path() on
> > the binary being executed, and verifies that a simple prefix comparison on
> > the returned pathname behaves correctly after the fix in patch 1.
> >
> > To avoid nondeterminism, the LSM program now filters based on the
> > expected PID, which is populated from userspace before the test binary is
> > executed. This prevents unrelated processes that also trigger the
> > bprm_check_security LSM hook from overwriting test results. Parent and
> > child processes are synchronized through a pipe to ensure the PID is set
> > before the child execs the test binary.
> >
> > Per review feedback, the new LSM coverage is merged into the existing
> > d_path selftest rather than adding new prog_tests/ or progs/ files. The
> > loop that checks the pathname prefix now uses bpf_for(), which is a
> > verifier-friendly way to express a small, fixed-iteration loop, and the
> > temporary /tmp/bpf_d_path_test binary is removed in the test cleanup
> > path.
> >
> > 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>
> > ---
> >  .../testing/selftests/bpf/prog_tests/d_path.c | 65 +++++++++++++++++++
> >  .../testing/selftests/bpf/progs/test_d_path.c | 33 ++++++++++
> >  2 files changed, 98 insertions(+)
> >
> > diff --git a/tools/testing/selftests/bpf/prog_tests/d_path.c b/tools/testing/selftests/bpf/prog_tests/d_path.c
> > index ccc768592e66..202b44e6f482 100644
> > --- a/tools/testing/selftests/bpf/prog_tests/d_path.c
> > +++ b/tools/testing/selftests/bpf/prog_tests/d_path.c
> > @@ -195,6 +195,68 @@ static void test_d_path_check_types(void)
> >         test_d_path_check_types__destroy(skel);
> >  }
> >
> > +static void test_d_path_lsm(void)
> > +{
> > +       struct test_d_path *skel;
> > +       int err;
> > +       int pipefd[2];
> > +       pid_t pid;
> > +
> > +       skel = test_d_path__open_and_load();
> > +       if (!ASSERT_OK_PTR(skel, "d_path skeleton failed"))
> > +               return;
> > +
> > +       err = test_d_path__attach(skel);
> > +       if (!ASSERT_OK(err, "attach failed"))
> > +               goto cleanup;
> > +
> > +       /* Prepare the test binary */
> > +       system("cp /bin/true /tmp/bpf_d_path_test 2>/dev/null || :");
> > +
> > +       if (!ASSERT_OK(pipe(pipefd), "pipe failed"))
> > +               goto cleanup;
> > +
> > +       pid = fork();
> > +       if (!ASSERT_GE(pid, 0, "fork failed")) {
> > +               close(pipefd[0]);
> > +               close(pipefd[1]);
> > +               goto cleanup;
> > +       }
> > +
> > +       if (pid == 0) {
> > +               /* Child */
> > +               char buf;
> > +
> > +               close(pipefd[1]);
> > +               /* Wait for parent to set PID in BPF map */
> > +               if (read(pipefd[0], &buf, 1) != 1)
> > +                       exit(1);
> > +               close(pipefd[0]);
> > +               execl("/tmp/bpf_d_path_test", "/tmp/bpf_d_path_test", NULL);
> > +               exit(1);
> > +       }
> 
> No forks please. They often make selftest to be flaky.
> Use simples possible way to test it.
> Without forks and pipes.

Yeah, I was also a little hesistant about letting this slide.

Shuran, change your BPF program such that you're attached to file_open
instead. That'll make testing from your test runnner far simpler.

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

* Re: [PATCH bpf v3 2/2] selftests/bpf: fix and consolidate d_path LSM regression test
  2025-12-02 18:39   ` Song Liu
@ 2025-12-04  4:34     ` Shuran Liu
  2025-12-04 21:41       ` Song Liu
  0 siblings, 1 reply; 9+ messages in thread
From: Shuran Liu @ 2025-12-04  4:34 UTC (permalink / raw)
  To: song
  Cc: andrii, ast, bpf, daniel, dxu, eddyz87, electronlsr, ftyg,
	gplhust955, haoluo, haoran.ni.cs, john.fastabend, jolsa, kpsingh,
	linux-kernel, linux-kselftest, linux-trace-kernel, martin.lau,
	mathieu.desnoyers, mattbobrowski, mhiramat, rostedt, sdf, shuah,
	yonghong.song

Hi Song,

Thanks for the review.

> I don't get why we add this selftest here. It doesn't appear to be related to
> patch 1/2.

The regression that patch 1/2 fixes was originally hit by an LSM program
calling bpf_d_path() from the bprm_check_security hook. The new subtest is a
minimal reproducer for that scenario: without patch 1/2 the string comparison
never matches due to verifier's faulty optimization, and with patch 1/2 it 
behaves correctly.

> The paragraph above is not really necessary. Just curious, did some AI
> write it?

The paragraph was indeed generated with the help of an AI assistant, and I didn’t 
trim it down enough. I’ll drop it and keep the changelog focused and brief in v4.

> This {} block is not necessary.

I’ll remove that extra block in v4.

Thanks again for the feedback.

Best regards,
Shuran Liu

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

* Re: [PATCH bpf v3 2/2] selftests/bpf: fix and consolidate d_path LSM regression test
  2025-12-03 10:32     ` Matt Bobrowski
@ 2025-12-04  4:39       ` Shuran Liu
  0 siblings, 0 replies; 9+ messages in thread
From: Shuran Liu @ 2025-12-04  4:39 UTC (permalink / raw)
  To: mattbobrowski, alexei.starovoitov
  Cc: andrii, ast, bpf, daniel, dxu, eddyz87, electronlsr, ftyg,
	gplhust955, haoluo, haoran.ni.cs, john.fastabend, jolsa, kpsingh,
	linux-kernel, linux-kselftest, linux-trace-kernel, martin.lau,
	mathieu.desnoyers, mhiramat, rostedt, sdf, shuah, song,
	yonghong.song

Hi Matt and Alexei,

Thanks for the feedback.

I have updated the test to use fallocate instead, which is in the allowlist of the d_path helper. I also minimized the test case while retaining the ability to reproduce the issue.

I will send the updated patch shortly. Thanks again for the review.

Best regards,
Shuran Liu

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

* Re: [PATCH bpf v3 2/2] selftests/bpf: fix and consolidate d_path LSM regression test
  2025-12-04  4:34     ` Shuran Liu
@ 2025-12-04 21:41       ` Song Liu
  0 siblings, 0 replies; 9+ messages in thread
From: Song Liu @ 2025-12-04 21:41 UTC (permalink / raw)
  To: Shuran Liu
  Cc: andrii, ast, bpf, daniel, dxu, eddyz87, ftyg, gplhust955, haoluo,
	haoran.ni.cs, john.fastabend, jolsa, kpsingh, linux-kernel,
	linux-kselftest, linux-trace-kernel, martin.lau,
	mathieu.desnoyers, mattbobrowski, mhiramat, rostedt, sdf, shuah,
	yonghong.song

On Wed, Dec 3, 2025 at 8:34 PM Shuran Liu <electronlsr@gmail.com> wrote:
>
> Hi Song,
>
> Thanks for the review.
>
> > I don't get why we add this selftest here. It doesn't appear to be related to
> > patch 1/2.
>
> The regression that patch 1/2 fixes was originally hit by an LSM program
> calling bpf_d_path() from the bprm_check_security hook. The new subtest is a
> minimal reproducer for that scenario: without patch 1/2 the string comparison
> never matches due to verifier's faulty optimization, and with patch 1/2 it
> behaves correctly.

I somehow didn't reproduce this in my local tests. Thanks for the explanation.

Song

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

end of thread, other threads:[~2025-12-04 21:41 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-12-02 14:19 [PATCH bpf v3 0/2] bpf: fix bpf_d_path() helper prototype Shuran Liu
2025-12-02 14:19 ` [PATCH bpf v3 1/2] bpf: mark bpf_d_path() buffer as writeable Shuran Liu
2025-12-02 14:19 ` [PATCH bpf v3 2/2] selftests/bpf: fix and consolidate d_path LSM regression test Shuran Liu
2025-12-02 18:39   ` Song Liu
2025-12-04  4:34     ` Shuran Liu
2025-12-04 21:41       ` Song Liu
2025-12-03  1:21   ` Alexei Starovoitov
2025-12-03 10:32     ` Matt Bobrowski
2025-12-04  4:39       ` 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).