Thread (1 message) 1 message, 1 author, 2025-12-02

Re: [PATCH bpf v2 2/2] selftests/bpf: fix and consolidate d_path LSM regression test

From: Matt Bobrowski <mattbobrowski@google.com>
Date: 2025-12-02 10:51:25
Also in: bpf, lkml

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())?
quoted
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:
quoted
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 <redacted>
Signed-off-by: Zesen Liu <redacted>
Co-developed-by: Peili Gao <redacted>
Signed-off-by: Peili Gao <redacted>
Co-developed-by: Haoran Ni <redacted>
Signed-off-by: Haoran Ni <redacted>
Signed-off-by: Shuran Liu <redacted>
Feel free to add:

Reviewed-by: Matt Bobrowski <mattbobrowski@google.com <mailto:mattbobrowski@google.com>>
quoted
---
.../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.
quoted
+	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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help