* [PATCH bpf 1/2] bpf: mark bpf_d_path() buffer as writeable
2025-12-01 14:38 [PATCH bpf 0/2] bpf: fix bpf_d_path() helper prototype Shuran Liu
@ 2025-12-01 14:38 ` Shuran Liu
2025-12-01 18:48 ` Matt Bobrowski
2025-12-01 14:38 ` [PATCH bpf 2/2] selftests/bpf: add regression test for bpf_d_path() Shuran Liu
2025-12-01 19:22 ` [PATCH bpf 0/2] bpf: fix bpf_d_path() helper prototype Matt Bobrowski
2 siblings, 1 reply; 6+ messages in thread
From: Shuran Liu @ 2025-12-01 14: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, 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 1/2] bpf: mark bpf_d_path() buffer as writeable
2025-12-01 14:38 ` [PATCH bpf 1/2] bpf: mark bpf_d_path() buffer as writeable Shuran Liu
@ 2025-12-01 18:48 ` Matt Bobrowski
0 siblings, 0 replies; 6+ messages in thread
From: Matt Bobrowski @ 2025-12-01 18:48 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 Mon, Dec 01, 2025 at 10:38:12PM +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>
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 2/2] selftests/bpf: add regression test for bpf_d_path()
2025-12-01 14:38 [PATCH bpf 0/2] bpf: fix bpf_d_path() helper prototype Shuran Liu
2025-12-01 14:38 ` [PATCH bpf 1/2] bpf: mark bpf_d_path() buffer as writeable Shuran Liu
@ 2025-12-01 14:38 ` Shuran Liu
2025-12-01 19:16 ` Matt Bobrowski
2025-12-01 19:22 ` [PATCH bpf 0/2] bpf: fix bpf_d_path() helper prototype Matt Bobrowski
2 siblings, 1 reply; 6+ messages in thread
From: Shuran Liu @ 2025-12-01 14: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, electronlsr,
Zesen Liu, Peili Gao, Haoran Ni
Add a simple LSM BPF program and a corresponding test_progs test case
to exercise bpf_d_path() and ensure that prefix comparisons on the
returned path keep working.
The LSM program hooks bprm_check_security, calls bpf_d_path() on the
binary being executed, and compares the returned path against the
"/tmp/" prefix. The result is recorded in an array map.
The user space test runs /tmp/bpf_d_path_test (copied from /bin/true)
and checks that the BPF program records a successful prefix match.
Without the preceding fix to bpf_d_path()'s helper prototype, the
test can fail due to the verifier incorrectly assuming that the
buffer contents are unchanged across the helper call and misoptimizing
the program. With the fix applied, the test passes.
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>
---
.../selftests/bpf/prog_tests/d_path_lsm.c | 27 ++++++++++++
.../selftests/bpf/progs/d_path_lsm.bpf.c | 43 +++++++++++++++++++
2 files changed, 70 insertions(+)
create mode 100644 tools/testing/selftests/bpf/prog_tests/d_path_lsm.c
create mode 100644 tools/testing/selftests/bpf/progs/d_path_lsm.bpf.c
diff --git a/tools/testing/selftests/bpf/prog_tests/d_path_lsm.c b/tools/testing/selftests/bpf/prog_tests/d_path_lsm.c
new file mode 100644
index 000000000000..92aad744ed12
--- /dev/null
+++ b/tools/testing/selftests/bpf/prog_tests/d_path_lsm.c
@@ -0,0 +1,27 @@
+// SPDX-License-Identifier: GPL-2.0-only
+#include <test_progs.h>
+#include "d_path_lsm.skel.h"
+
+void test_d_path_lsm(void)
+{
+ struct d_path_lsm *skel = NULL;
+ int err, map_fd, key = 0, val = 0;
+
+ skel = d_path_lsm__open_and_load();
+ if (!ASSERT_OK_PTR(skel, "open_and_load"))
+ return;
+
+ err = d_path_lsm__attach(skel);
+ if (!ASSERT_OK(err, "attach"))
+ goto out;
+
+ system("cp /bin/true /tmp/bpf_d_path_test 2>/dev/null || :");
+ system("/tmp/bpf_d_path_test >/dev/null 2>&1");
+
+ map_fd = bpf_map__fd(skel->maps.result);
+ err = bpf_map_lookup_elem(map_fd, &key, &val);
+ ASSERT_OK(err, "lookup_result");
+ ASSERT_EQ(val, 1, "prefix_match");
+out:
+ d_path_lsm__destroy(skel);
+}
diff --git a/tools/testing/selftests/bpf/progs/d_path_lsm.bpf.c b/tools/testing/selftests/bpf/progs/d_path_lsm.bpf.c
new file mode 100644
index 000000000000..36f9ff37e817
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/d_path_lsm.bpf.c
@@ -0,0 +1,43 @@
+// SPDX-License-Identifier: GPL-2.0-only
+#include "vmlinux.h"
+#include <bpf/bpf_helpers.h>
+#include <bpf/bpf_tracing.h>
+
+char LICENSE[] SEC("license") = "GPL";
+
+#define FILENAME_MAX_SIZE 256
+#define TARGET_DIR "/tmp/"
+#define TARGET_DIR_LEN 5
+
+struct {
+ __uint(type, BPF_MAP_TYPE_ARRAY);
+ __uint(max_entries, 1);
+ __type(key, int);
+ __type(value, int);
+} result SEC(".maps");
+
+SEC("lsm/bprm_check_security")
+int BPF_PROG(d_path_lsm_prog, struct linux_binprm *bprm)
+{
+ char path[FILENAME_MAX_SIZE] = {};
+ long len;
+ int key = 0;
+ int val = 0;
+
+ len = bpf_d_path(&bprm->file->f_path, path, sizeof(path));
+ if (len < 0)
+ return 0;
+
+#pragma unroll
+ for (int i = 0; i < TARGET_DIR_LEN; i++) {
+ if ((u8)path[i] != (u8)TARGET_DIR[i]) {
+ val = -1; /* mismatch */
+ bpf_map_update_elem(&result, &key, &val, BPF_ANY);
+ return 0;
+ }
+ }
+
+ val = 1; /* prefix match */
+ bpf_map_update_elem(&result, &key, &val, BPF_ANY);
+ return 0;
+}
--
2.52.0
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH bpf 2/2] selftests/bpf: add regression test for bpf_d_path()
2025-12-01 14:38 ` [PATCH bpf 2/2] selftests/bpf: add regression test for bpf_d_path() Shuran Liu
@ 2025-12-01 19:16 ` Matt Bobrowski
0 siblings, 0 replies; 6+ messages in thread
From: Matt Bobrowski @ 2025-12-01 19:16 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 Mon, Dec 01, 2025 at 10:38:13PM +0800, Shuran Liu wrote:
> Add a simple LSM BPF program and a corresponding test_progs test case
> to exercise bpf_d_path() and ensure that prefix comparisons on the
> returned path keep working.
>
n> The LSM program hooks bprm_check_security, calls bpf_d_path() on the
> binary being executed, and compares the returned path against the
> "/tmp/" prefix. The result is recorded in an array map.
>
> The user space test runs /tmp/bpf_d_path_test (copied from /bin/true)
> and checks that the BPF program records a successful prefix match.
>
> Without the preceding fix to bpf_d_path()'s helper prototype, the
> test can fail due to the verifier incorrectly assuming that the
> buffer contents are unchanged across the helper call and misoptimizing
> the program. With the fix applied, the test passes.
>
> 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>
> ---
> .../selftests/bpf/prog_tests/d_path_lsm.c | 27 ++++++++++++
> .../selftests/bpf/progs/d_path_lsm.bpf.c | 43 +++++++++++++++++++
> 2 files changed, 70 insertions(+)
> create mode 100644 tools/testing/selftests/bpf/prog_tests/d_path_lsm.c
> create mode 100644 tools/testing/selftests/bpf/progs/d_path_lsm.bpf.c
>
> diff --git a/tools/testing/selftests/bpf/prog_tests/d_path_lsm.c b/tools/testing/selftests/bpf/prog_tests/d_path_lsm.c
> new file mode 100644
> index 000000000000..92aad744ed12
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/prog_tests/d_path_lsm.c
I don't see why adding yet another new bpf_d_path() related test to
prog_tests is warranted here. Why not simply incorporate this
additional test case into the preexisting bpf_d_path() related
prog_tests source file i.e. tools/testing/selftests/bpf/d_path.c?
> @@ -0,0 +1,27 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +#include <test_progs.h>
> +#include "d_path_lsm.skel.h"
> +
> +void test_d_path_lsm(void)
> +{
> + struct d_path_lsm *skel = NULL;
> + int err, map_fd, key = 0, val = 0;
> +
> + skel = d_path_lsm__open_and_load();
> + if (!ASSERT_OK_PTR(skel, "open_and_load"))
> + return;
> +
> + err = d_path_lsm__attach(skel);
> + if (!ASSERT_OK(err, "attach"))
> + goto out;
> +
> + system("cp /bin/true /tmp/bpf_d_path_test 2>/dev/null || :");
> + system("/tmp/bpf_d_path_test >/dev/null 2>&1");
> +
> + map_fd = bpf_map__fd(skel->maps.result);
> + err = bpf_map_lookup_elem(map_fd, &key, &val);
> + ASSERT_OK(err, "lookup_result");
> + ASSERT_EQ(val, 1, "prefix_match");
> +out:
> + d_path_lsm__destroy(skel);
> +}
>
> diff --git a/tools/testing/selftests/bpf/progs/d_path_lsm.bpf.c b/tools/testing/selftests/bpf/progs/d_path_lsm.bpf.c
> new file mode 100644
> index 000000000000..36f9ff37e817
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/progs/d_path_lsm.bpf.c
> @@ -0,0 +1,43 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +#include "vmlinux.h"
> +#include <bpf/bpf_helpers.h>
> +#include <bpf/bpf_tracing.h>
> +
> +char LICENSE[] SEC("license") = "GPL";
> +
> +#define FILENAME_MAX_SIZE 256
> +#define TARGET_DIR "/tmp/"
> +#define TARGET_DIR_LEN 5
> +
> +struct {
> + __uint(type, BPF_MAP_TYPE_ARRAY);
> + __uint(max_entries, 1);
> + __type(key, int);
> + __type(value, int);
> +} result SEC(".maps");
> +
> +SEC("lsm/bprm_check_security")
> +int BPF_PROG(d_path_lsm_prog, struct linux_binprm *bprm)
> +{
> + char path[FILENAME_MAX_SIZE] = {};
> + long len;
> + int key = 0;
> + int val = 0;
> +
> + len = bpf_d_path(&bprm->file->f_path, path, sizeof(path));
> + if (len < 0)
> + return 0;
> +
> +#pragma unroll
> + for (int i = 0; i < TARGET_DIR_LEN; i++) {
> + if ((u8)path[i] != (u8)TARGET_DIR[i]) {
> + val = -1; /* mismatch */
> + bpf_map_update_elem(&result, &key, &val, BPF_ANY);
> + return 0;
> + }
> + }
> +
> + val = 1; /* prefix match */
> + bpf_map_update_elem(&result, &key, &val, BPF_ANY);
> + return 0;
Will this not flake, like, maybe a lot? Mismatches are being reported
for every non-matched prefix. Meaning, other threads that are racing
alongside your system(3) invocations and going through
security_bprm_check() could very well reset your BPF_MAP_TYPE_ARRAY
element value back to -1 before your userspace code even has a chance
to assert it? Perhaps you can make this test a little more
deterministic by filtering by the expected PID?
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH bpf 0/2] bpf: fix bpf_d_path() helper prototype
2025-12-01 14:38 [PATCH bpf 0/2] bpf: fix bpf_d_path() helper prototype Shuran Liu
2025-12-01 14:38 ` [PATCH bpf 1/2] bpf: mark bpf_d_path() buffer as writeable Shuran Liu
2025-12-01 14:38 ` [PATCH bpf 2/2] selftests/bpf: add regression test for bpf_d_path() Shuran Liu
@ 2025-12-01 19:22 ` Matt Bobrowski
2 siblings, 0 replies; 6+ messages in thread
From: Matt Bobrowski @ 2025-12-01 19:22 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
On Mon, Dec 01, 2025 at 10:38:11PM +0800, Shuran Liu wrote:
> 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.
FTR, I strongly encourage any new BPF LSM implementation to consider
using the newer BPF kfunc alternative instead, being
bpf_path_d_path().
> 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.
This is the correct thing to do, appreciate you sending through the
fix.
> Patch 2 adds a minimal selftest under tools/testing/selftests/bpf that
> hooks bprm_check_security, calls bpf_d_path() on a binary under /tmp/,
> and verifies that the prefix comparison on the returned path keeps
> working.
Makes sense to add a test for this regression, but please also see my
comments against this patch.
> On my local setup, tools/testing/selftests/bpf does not build fully
> due to unrelated tests using newer helpers. I validated this series by
> manually reproducing the issue with a small LSM program and by
> building and running only the new d_path_lsm test on kernels with and
> without patch 1 applied.
>
> Thanks,
> Shuran Liu
>
> 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 +-
> .../selftests/bpf/prog_tests/d_path_lsm.c | 27 ++++++++++++
> .../selftests/bpf/progs/d_path_lsm.bpf.c | 43 +++++++++++++++++++
> 3 files changed, 71 insertions(+), 1 deletion(-)
> create mode 100644 tools/testing/selftests/bpf/prog_tests/d_path_lsm.c
> create mode 100644 tools/testing/selftests/bpf/progs/d_path_lsm.bpf.c
>
> --
> 2.52.0
>
^ permalink raw reply [flat|nested] 6+ messages in thread