linux-kernel - Re: [PATCH bpf 2/2] selftests/bpf: add regression test for bpf_d

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <aS3pqFCze_gmYq0y@google.com>
Date: Mon, 1 Dec 2025 19:16:56 +0000
From: Matt Bobrowski <mattbobrowski@...gle.com>
To: Shuran Liu <electronlsr@...il.com>
Cc: song@...nel.org, bpf@...r.kernel.org, ast@...nel.org,
	daniel@...earbox.net, andrii@...nel.org, martin.lau@...ux.dev,
	eddyz87@...il.com, yonghong.song@...ux.dev,
	john.fastabend@...il.com, kpsingh@...nel.org, sdf@...ichev.me,
	haoluo@...gle.com, jolsa@...nel.org, rostedt@...dmis.org,
	mhiramat@...nel.org, mathieu.desnoyers@...icios.com,
	linux-kernel@...r.kernel.org, linux-trace-kernel@...r.kernel.org,
	Zesen Liu <ftyg@...e.com>, Peili Gao <gplhust955@...il.com>,
	Haoran Ni <haoran.ni.cs@...il.com>
Subject: Re: [PATCH bpf 2/2] selftests/bpf: add regression test for
 bpf_d_path()

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@...e.com>
> Signed-off-by: Zesen Liu <ftyg@...e.com>
> Co-developed-by: Peili Gao <gplhust955@...il.com>
> Signed-off-by: Peili Gao <gplhust955@...il.com>
> Co-developed-by: Haoran Ni <haoran.ni.cs@...il.com>
> Signed-off-by: Haoran Ni <haoran.ni.cs@...il.com>
> Signed-off-by: Shuran Liu <electronlsr@...il.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?