* [PATCH bpf v2 0/2] bpf: fix bpf_d_path() helper prototype
@ 2025-12-02 7:54 Shuran Liu
2025-12-02 7:54 ` [PATCH bpf v2 1/2] bpf: mark bpf_d_path() buffer as writeable Shuran Liu
2025-12-02 7:54 ` [PATCH bpf v2 2/2] selftests/bpf: fix and consolidate d_path LSM regression test Shuran Liu
0 siblings, 2 replies; 6+ messages in thread
From: Shuran Liu @ 2025-12-02 7:54 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, electronlsr
Hi,
this series fixes a verifier regression for bpf_d_path() introduced by
commit 37cce22dbd51 ("bpf: verifier: Refactor helper access type
tracking") and adds a small selftest to exercise the helper from an
LSM program.
Commit 37cce22dbd51 started distinguishing read vs write accesses
performed by helpers. bpf_d_path()'s buffer argument was left as
ARG_PTR_TO_MEM without MEM_WRITE, so the verifier could incorrectly
assume that the buffer contents are unchanged across the helper call
and base its optimizations on this wrong assumption.
In practice this showed up as a misbehaving LSM BPF program that calls
bpf_d_path() and then does a simple prefix comparison on the returned
path: the program would sometimes take the "mismatch" branch even
though both bytes being compared were actually equal.
Patch 1 fixes bpf_d_path()'s helper prototype by marking the buffer
argument as ARG_PTR_TO_MEM | MEM_WRITE, so that the verifier correctly
models the write to the caller-provided buffer.
Patch 2 adds a regression test that exercises bpf_d_path() from an LSM
program attached to bprm_check_security. The test verifies that pathname
prefix comparisons behave correctly with the fix applied.
Changes in 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,
Shuran Liu
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 | 64 +++++++++++++++++++
.../testing/selftests/bpf/progs/test_d_path.c | 33 ++++++++++
3 files changed, 98 insertions(+), 1 deletion(-)
--
2.52.0
^ permalink raw reply [flat|nested] 6+ messages in thread* [PATCH bpf v2 1/2] bpf: mark bpf_d_path() buffer as writeable 2025-12-02 7:54 [PATCH bpf v2 0/2] bpf: fix bpf_d_path() helper prototype Shuran Liu @ 2025-12-02 7:54 ` Shuran Liu 2025-12-02 8:19 ` Matt Bobrowski 2025-12-02 7:54 ` [PATCH bpf v2 2/2] selftests/bpf: fix and consolidate d_path LSM regression test Shuran Liu 1 sibling, 1 reply; 6+ messages in thread From: Shuran Liu @ 2025-12-02 7:54 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, 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> --- 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
* Re: [PATCH bpf v2 1/2] bpf: mark bpf_d_path() buffer as writeable 2025-12-02 7:54 ` [PATCH bpf v2 1/2] bpf: mark bpf_d_path() buffer as writeable Shuran Liu @ 2025-12-02 8:19 ` Matt Bobrowski 0 siblings, 0 replies; 6+ messages in thread From: Matt Bobrowski @ 2025-12-02 8:19 UTC (permalink / raw) To: Shuran Liu Cc: song, bpf, ast, daniel, andrii, martin.lau, eddyz87, yonghong.song, john.fastabend, kpsingh, sdf, haoluo, jolsa, rostedt, mhiramat, mathieu.desnoyers, linux-kernel, linux-trace-kernel, Zesen Liu, Peili Gao, Haoran Ni On Tue, Dec 02, 2025 at 03:54:40PM +0800, Shuran Liu wrote: > 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> You forgot to include my Reviewed-by trailer from the initial patch series (https://lore.kernel.org/bpf/aS3jARS7a-gh9UCa@google.com/), so here it is again. 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 [flat|nested] 6+ messages in thread
* [PATCH bpf v2 2/2] selftests/bpf: fix and consolidate d_path LSM regression test 2025-12-02 7:54 [PATCH bpf v2 0/2] bpf: fix bpf_d_path() helper prototype Shuran Liu 2025-12-02 7:54 ` [PATCH bpf v2 1/2] bpf: mark bpf_d_path() buffer as writeable Shuran Liu @ 2025-12-02 7:54 ` Shuran Liu 2025-12-02 8:59 ` Matt Bobrowski 1 sibling, 1 reply; 6+ messages in thread From: Shuran Liu @ 2025-12-02 7:54 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, 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 test is merged into the existing d_path selftest rather than adding new prog_tests/ or progs/ files. 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 | 64 +++++++++++++++++++ .../testing/selftests/bpf/progs/test_d_path.c | 33 ++++++++++ 2 files changed, 97 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..2909ca3bae0f 100644 --- a/tools/testing/selftests/bpf/prog_tests/d_path.c +++ b/tools/testing/selftests/bpf/prog_tests/d_path.c @@ -195,6 +195,67 @@ 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: + test_d_path__destroy(skel); +} + void test_d_path(void) { if (test__start_subtest("basic")) @@ -205,4 +266,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..7f65c282069a 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/"; + +#pragma unroll + for (int i = 0; i < sizeof(target_dir) - 1; i++) { + 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] 6+ messages in thread
* Re: [PATCH bpf v2 2/2] selftests/bpf: fix and consolidate d_path LSM regression test 2025-12-02 7:54 ` [PATCH bpf v2 2/2] selftests/bpf: fix and consolidate d_path LSM regression test Shuran Liu @ 2025-12-02 8:59 ` Matt Bobrowski [not found] ` <F1F96C9B-EAD1-4FD7-A053-EE072A5F4E53@gmail.com> 0 siblings, 1 reply; 6+ messages in thread From: Matt Bobrowski @ 2025-12-02 8:59 UTC (permalink / raw) To: Shuran Liu Cc: song, bpf, ast, daniel, andrii, martin.lau, eddyz87, yonghong.song, john.fastabend, kpsingh, sdf, haoluo, jolsa, rostedt, mhiramat, mathieu.desnoyers, linux-kernel, linux-trace-kernel, Zesen Liu, Peili Gao, Haoran Ni On Tue, Dec 02, 2025 at 03:54:41PM +0800, Shuran Liu 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 test is merged into the existing d_path > selftest rather than adding new prog_tests/ or progs/ files. > > 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> Feel free to add: Reviewed-by: Matt Bobrowski <mattbobrowski@google.com> > --- > .../testing/selftests/bpf/prog_tests/d_path.c | 64 +++++++++++++++++++ > .../testing/selftests/bpf/progs/test_d_path.c | 33 ++++++++++ > 2 files changed, 97 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..2909ca3bae0f 100644 > --- a/tools/testing/selftests/bpf/prog_tests/d_path.c > +++ b/tools/testing/selftests/bpf/prog_tests/d_path.c > @@ -195,6 +195,67 @@ 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 || :"); I'd much prefer if we also cleaned up after ourselves, but it's not that much of an issue I guess. > + 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: > + test_d_path__destroy(skel); > +} > + > void test_d_path(void) > { > if (test__start_subtest("basic")) > @@ -205,4 +266,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..7f65c282069a 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/"; > + > +#pragma unroll > + for (int i = 0; i < sizeof(target_dir) - 1; i++) { > + 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] 6+ messages in thread
[parent not found: <F1F96C9B-EAD1-4FD7-A053-EE072A5F4E53@gmail.com>]
* Re: [PATCH bpf v2 2/2] selftests/bpf: fix and consolidate d_path LSM regression test [not found] ` <F1F96C9B-EAD1-4FD7-A053-EE072A5F4E53@gmail.com> @ 2025-12-02 10:51 ` Matt Bobrowski 0 siblings, 0 replies; 6+ messages in thread From: Matt Bobrowski @ 2025-12-02 10:51 UTC (permalink / raw) To: Shuran Liu, g Cc: song, bpf, ast, daniel, andrii, martin.lau, eddyz87, yonghong.song, john.fastabend, kpsingh, sdf, haoluo, jolsa, rostedt, mhiramat, mathieu.desnoyers, linux-kernel, linux-trace-kernel, Zesen Liu, Peili Gao, Haoran Ni On Tue, Dec 02, 2025 at 05:30:15PM +0800, Shuran Liu wrote: > Hi Matt, > > Thanks a lot for the review and for re-sending your Reviewed-by tag. > > In the next version of the series I’ll add your > > Reviewed-by: Matt Bobrowski <mattbobrowski@google.com> > > to the patch that introduces the new selftest, and I’ll also make sure to > remove /tmp/bpf_d_path_test in the test cleanup path as you suggested. SGTM. > I also noticed that the CI is currently failing due to the `#pragma unroll` > around the loop in prog_lsm(). Would you prefer that I simply drop the pragma > in the next version, given that the loop bound is small and constant anyway, > or is there a better way you’d recommend to handle this? Yeah, I don't think the use of this directive is required here given the iteration count is tiny. Alternatively, perhaps you could switch over to using a more BPF verifier preferred alternative (i.e. bpf_for() or better yet and simpler bpf_repeat())? > > On Tue, Dec 02, 2025 at 00:59:45AM -0800, Matt Bobrowski wrote： > > > > On Tue, Dec 02, 2025 at 03:54:41PM +0800, Shuran Liu 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 test is merged into the existing d_path > >> selftest rather than adding new prog_tests/ or progs/ files. > >> > >> 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> > > > > Feel free to add: > > > > Reviewed-by: Matt Bobrowski <mattbobrowski@google.com <mailto:mattbobrowski@google.com>> > > > >> --- > >> .../testing/selftests/bpf/prog_tests/d_path.c | 64 +++++++++++++++++++ > >> .../testing/selftests/bpf/progs/test_d_path.c | 33 ++++++++++ > >> 2 files changed, 97 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..2909ca3bae0f 100644 > >> --- a/tools/testing/selftests/bpf/prog_tests/d_path.c > >> +++ b/tools/testing/selftests/bpf/prog_tests/d_path.c > >> @@ -195,6 +195,67 @@ 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 || :"); > > > > I'd much prefer if we also cleaned up after ourselves, but it's not > > that much of an issue I guess. > > > >> + 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: > >> + test_d_path__destroy(skel); > >> +} > >> + > >> void test_d_path(void) > >> { > >> if (test__start_subtest("basic")) > >> @@ -205,4 +266,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..7f65c282069a 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/"; > >> + > >> +#pragma unroll > >> + for (int i = 0; i < sizeof(target_dir) - 1; i++) { > >> + 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] 6+ messages in thread
end of thread, other threads:[~2025-12-02 10:51 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-12-02 7:54 [PATCH bpf v2 0/2] bpf: fix bpf_d_path() helper prototype Shuran Liu
2025-12-02 7:54 ` [PATCH bpf v2 1/2] bpf: mark bpf_d_path() buffer as writeable Shuran Liu
2025-12-02 8:19 ` Matt Bobrowski
2025-12-02 7:54 ` [PATCH bpf v2 2/2] selftests/bpf: fix and consolidate d_path LSM regression test Shuran Liu
2025-12-02 8:59 ` Matt Bobrowski
[not found] ` <F1F96C9B-EAD1-4FD7-A053-EE072A5F4E53@gmail.com>
2025-12-02 10:51 ` Matt Bobrowski
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).