All of lore.kernel.org
 help / color / mirror / Atom feed
From: Matt Bobrowski <mattbobrowski@google.com>
To: Alexei Starovoitov <alexei.starovoitov@gmail.com>
Cc: Shuran Liu <electronlsr@gmail.com>, Song Liu <song@kernel.org>,
	bpf <bpf@vger.kernel.org>, Alexei Starovoitov <ast@kernel.org>,
	Daniel Borkmann <daniel@iogearbox.net>,
	Andrii Nakryiko <andrii@kernel.org>,
	Martin KaFai Lau <martin.lau@linux.dev>,
	Eduard <eddyz87@gmail.com>,
	Yonghong Song <yonghong.song@linux.dev>,
	John Fastabend <john.fastabend@gmail.com>,
	KP Singh <kpsingh@kernel.org>,
	Stanislav Fomichev <sdf@fomichev.me>, Hao Luo <haoluo@google.com>,
	Jiri Olsa <jolsa@kernel.org>,
	Steven Rostedt <rostedt@goodmis.org>,
	Masami Hiramatsu <mhiramat@kernel.org>,
	Mathieu Desnoyers <mathieu.desnoyers@efficios.com>,
	LKML <linux-kernel@vger.kernel.org>,
	linux-trace-kernel <linux-trace-kernel@vger.kernel.org>,
	Daniel Xu <dxu@dxuuu.xyz>,
	"open list:KERNEL SELFTEST FRAMEWORK"
	<linux-kselftest@vger.kernel.org>, Shuah Khan <shuah@kernel.org>,
	Zesen Liu <ftyg@live.com>, Peili Gao <gplhust955@gmail.com>,
	Haoran Ni <haoran.ni.cs@gmail.com>
Subject: Re: [PATCH bpf v3 2/2] selftests/bpf: fix and consolidate d_path LSM regression test
Date: Wed, 3 Dec 2025 10:32:10 +0000	[thread overview]
Message-ID: <aTARqrMyC36CXa_L@google.com> (raw)
In-Reply-To: <CAADnVQJQj=mdFbPf7nmc0+qZVC4RCK5AbJvNQv2W--tvGyzzVA@mail.gmail.com>

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.

  reply	other threads:[~2025-12-03 10:32 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-12-02 14:19 [PATCH bpf v3 0/2] bpf: fix bpf_d_path() helper prototype Shuran Liu
2025-12-02 14:19 ` [PATCH bpf v3 1/2] bpf: mark bpf_d_path() buffer as writeable Shuran Liu
2025-12-02 14:19 ` [PATCH bpf v3 2/2] selftests/bpf: fix and consolidate d_path LSM regression test Shuran Liu
2025-12-02 18:39   ` Song Liu
2025-12-04  4:34     ` Shuran Liu
2025-12-04 21:41       ` Song Liu
2025-12-03  1:21   ` Alexei Starovoitov
2025-12-03 10:32     ` Matt Bobrowski [this message]
2025-12-04  4:39       ` Shuran Liu

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=aTARqrMyC36CXa_L@google.com \
    --to=mattbobrowski@google.com \
    --cc=alexei.starovoitov@gmail.com \
    --cc=andrii@kernel.org \
    --cc=ast@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=daniel@iogearbox.net \
    --cc=dxu@dxuuu.xyz \
    --cc=eddyz87@gmail.com \
    --cc=electronlsr@gmail.com \
    --cc=ftyg@live.com \
    --cc=gplhust955@gmail.com \
    --cc=haoluo@google.com \
    --cc=haoran.ni.cs@gmail.com \
    --cc=john.fastabend@gmail.com \
    --cc=jolsa@kernel.org \
    --cc=kpsingh@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-kselftest@vger.kernel.org \
    --cc=linux-trace-kernel@vger.kernel.org \
    --cc=martin.lau@linux.dev \
    --cc=mathieu.desnoyers@efficios.com \
    --cc=mhiramat@kernel.org \
    --cc=rostedt@goodmis.org \
    --cc=sdf@fomichev.me \
    --cc=shuah@kernel.org \
    --cc=song@kernel.org \
    --cc=yonghong.song@linux.dev \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.