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

Shuran Liu posted 2 patches 2 months, 1 week ago
There is a newer version of this series
[PATCH bpf v3 2/2] selftests/bpf: fix and consolidate d_path LSM regression test
Posted by Shuran Liu 2 months, 1 week ago
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
Re: [PATCH bpf v3 2/2] selftests/bpf: fix and consolidate d_path LSM regression test
Posted by Alexei Starovoitov 2 months ago
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
Re: [PATCH bpf v3 2/2] selftests/bpf: fix and consolidate d_path LSM regression test
Posted by Matt Bobrowski 2 months ago
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.
Re: [PATCH bpf v3 2/2] selftests/bpf: fix and consolidate d_path LSM regression test
Posted by Shuran Liu 2 months ago
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
Re: [PATCH bpf v3 2/2] selftests/bpf: fix and consolidate d_path LSM regression test
Posted by Song Liu 2 months ago
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
>
Re: [PATCH bpf v3 2/2] selftests/bpf: fix and consolidate d_path LSM regression test
Posted by Shuran Liu 2 months ago
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
Re: [PATCH bpf v3 2/2] selftests/bpf: fix and consolidate d_path LSM regression test
Posted by Song Liu 2 months ago
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