linux-kernel - Re: [PATCH v4 bpf-next 2/2] selftests/bpf: Add is_kernel parameter to LSM/bpf test programs

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: <CAPhsuW6RrUiXaQe1HBYOvwUx2GFaA-RKx22955A2StsP2erTeA@mail.gmail.com>
Date: Tue, 4 Mar 2025 19:32:06 -0800
From: Song Liu <song@...nel.org>
To: Paul Moore <paul@...l-moore.com>
Cc: Blaise Boscaccy <bboscaccy@...ux.microsoft.com>, James Morris <jmorris@...ei.org>, 
	"Serge E. Hallyn" <serge@...lyn.com>, Alexei Starovoitov <ast@...nel.org>, 
	Daniel Borkmann <daniel@...earbox.net>, John Fastabend <john.fastabend@...il.com>, 
	Andrii Nakryiko <andrii@...nel.org>, Martin KaFai Lau <martin.lau@...ux.dev>, 
	Eduard Zingerman <eddyz87@...il.com>, Yonghong Song <yonghong.song@...ux.dev>, 
	KP Singh <kpsingh@...nel.org>, Stanislav Fomichev <sdf@...ichev.me>, Hao Luo <haoluo@...gle.com>, 
	Jiri Olsa <jolsa@...nel.org>, Stephen Smalley <stephen.smalley.work@...il.com>, 
	Ondrej Mosnacek <omosnace@...hat.com>, linux-security-module@...r.kernel.org, 
	linux-kernel@...r.kernel.org, bpf@...r.kernel.org, selinux@...r.kernel.org
Subject: Re: [PATCH v4 bpf-next 2/2] selftests/bpf: Add is_kernel parameter to
 LSM/bpf test programs

On Tue, Mar 4, 2025 at 6:14 PM Paul Moore <paul@...l-moore.com> wrote:
>
> On Tue, Mar 4, 2025 at 8:26 PM Blaise Boscaccy
> <bboscaccy@...ux.microsoft.com> wrote:
> > Paul Moore <paul@...l-moore.com> writes:
> > > On Tue, Mar 4, 2025 at 3:31 PM Blaise Boscaccy
> > > <bboscaccy@...ux.microsoft.com> wrote:
> > >>
> > >> The security_bpf LSM hook now contains a boolean parameter specifying
> > >> whether an invocation of the bpf syscall originated from within the
> > >> kernel. Here, we update the function signature of relevant test
> > >> programs to include that new parameter.
> > >>
> > >> Signed-off-by: Blaise Boscaccy bboscaccy@...ux.microsoft.com
> > >> ---
> > >>  tools/testing/selftests/bpf/progs/rcu_read_lock.c           | 3 ++-
> > >>  tools/testing/selftests/bpf/progs/test_cgroup1_hierarchy.c  | 4 ++--
> > >>  tools/testing/selftests/bpf/progs/test_kfunc_dynptr_param.c | 6 +++---
> > >>  tools/testing/selftests/bpf/progs/test_lookup_key.c         | 2 +-
> > >>  tools/testing/selftests/bpf/progs/test_ptr_untrusted.c      | 2 +-
> > >>  tools/testing/selftests/bpf/progs/test_task_under_cgroup.c  | 2 +-
> > >>  tools/testing/selftests/bpf/progs/test_verify_pkcs7_sig.c   | 2 +-
> > >>  7 files changed, 11 insertions(+), 10 deletions(-)
> > >
> > > I see that Song requested that the changes in this patch be split out
> > > back in the v3 revision, will that cause git bisect issues if patch
> > > 1/2 is applied but patch 2/2 is not, or is there some BPF magic that
> > > ensures that the selftests will still run properly?
> > >
> >
> > So there isn't any type checking in the bpf program's function
> > arguments against the LSM hook definitions, so it shouldn't cause any
> > build issues. To the best of my knowledge, the new is_kernel boolean
> > flag will end up living in r3. None of the current tests reference
> > that parameter, so if we bisected and ended up on the previous commit,
> > the bpf test programs would in a worst-case scenario simply clobber that
> > register, which shouldn't effect any test outcomes unless a test program
> > was somehow dependent on an uninitialized value in a scratch register.
>
> Esh.  With that in mind, I'd argue that the two patches really should
> just be one patch as you did before.  The patches are both pretty
> small and obviously related so it really shouldn't be an issue.
>
> However, since we need this patchset in order to properly implement
> BPF signature verification I'm not going to make a fuss if Song feels
> strongly that the selftest changes should be split into their own
> patch.

On second thought, I think it makes sense to merge the two patches.

Blasie, please update 1/2 based on Paul's comment, merge the two
patches, and resend. You can keep my Acked-by.

Do we need this in the LSM tree before the upcoming merge window?
If not, we would prefer to carry it in bpf-next.

Thanks,
Song