Re: [PATCH bpf-next v1 3/4] selftests/bpf: allow tests from verifier.c not to drop CAP_SYS_ADMIN

[Eduard Zingerman <eddyz87@gmail.com>]

On Tue, 2025-06-24 at 14:55 -0700, Alexei Starovoitov wrote:
> On Tue, Jun 24, 2025 at 12:10 PM Eduard Zingerman <eddyz87@gmail.com> wrote:
> > 
> > Originally prog_tests/verifier.c was developed to run tests ported
> > from test_verifier binary. test_verifier runs tests with CAP_SYS_ADMIN
> > dropped, hence this behaviour was copied in prog_tests/verifier.c.
> > BPF_OBJ_GET_NEXT_ID BPF syscall command fails w/o CAP_SYS_ADMIN and
> > this prevents libbpf from loading module BTFs.
> 
> You need this only because of 'bpf_kfunc_trusted_num_test' access
> in patch 4?

Yes.

> Can you use kernel kfunc instead?

Should be able to.

> This needs more thought.
> s/RUN/RUN_FULL_CAPS/ just because of kfunc in the bpf_testmod
> doesn't look like a good long term approach.
> 
> I thought we agreed to relax BPF_OBJ_GET_NEXT_ID to allow for CAP_BPF.
> Probably even unpriv can do it.
> Just knowing a set of prog, map, bpf IDs is not a security threat.
> 
> BPF_BTF_GET_FD_BY_ID can also be allowed for unpriv,
> since one can do it already from /sys/kernel/btf/

Makes sense to me.

> > This commit adds an optout from capability drop.
> > 
> > Signed-off-by: Eduard Zingerman <eddyz87@gmail.com>
> > ---
> >  .../testing/selftests/bpf/prog_tests/verifier.c | 17 +++++++++++------
> >  1 file changed, 11 insertions(+), 6 deletions(-)
> > 
> > diff --git a/tools/testing/selftests/bpf/prog_tests/verifier.c b/tools/testing/selftests/bpf/prog_tests/verifier.c
> > index c9da06741104..cedb86d8f717 100644
> > --- a/tools/testing/selftests/bpf/prog_tests/verifier.c
> > +++ b/tools/testing/selftests/bpf/prog_tests/verifier.c
> > @@ -115,14 +115,16 @@ struct test_val {
> >  __maybe_unused
> >  static void run_tests_aux(const char *skel_name,
> >                           skel_elf_bytes_fn elf_bytes_factory,
> > -                         pre_execution_cb pre_execution_cb)
> > +                         pre_execution_cb pre_execution_cb,
> > +                         bool drop_sysadmin)
> 
> I have an allergic reaction to bool arguments.
> 
> >         run_tests_aux("verifier_array_access",
> >                       verifier_array_access__elf_bytes,
> > -                     init_array_access_maps);
> > +                     init_array_access_maps,
> > +                     true);
> 
> This is not readable without looking at the argument name.

I'll drop this change in v2.