On Mon, Dec 2, 2024 at 12:38 AM Kumar Kartikeya Dwivedi <memxor@xxxxxxxxx> wrote: > > From: Eduard Zingerman <eddyz87@xxxxxxxxx> > > Add a __caps_unpriv annotation so that tests requiring specific > capabilities while dropping the rest can conveniently specify them > during selftest declaration instead of munging with capabilities at > runtime from the testing binary. > > While at it, let us convert test_verifier_mtu to use this new support > instead. > > The original diff for this idea is available at link [0]. > > [0]: https://lore.kernel.org/bpf/a1e48f5d9ae133e19adc6adf27e19d585e06bab4.camel@xxxxxxxxx > > Signed-off-by: Eduard Zingerman <eddyz87@xxxxxxxxx> > [ Kartikeya: rebase on bpf-next, remove unnecessary bits, convert test_verifier_mtu ] > Signed-off-by: Kumar Kartikeya Dwivedi <memxor@xxxxxxxxx> > --- > .../selftests/bpf/prog_tests/verifier.c | 19 +-------- > tools/testing/selftests/bpf/progs/bpf_misc.h | 2 + > .../selftests/bpf/progs/verifier_mtu.c | 3 +- > tools/testing/selftests/bpf/test_loader.c | 41 +++++++++++++++++++ > 4 files changed, 46 insertions(+), 19 deletions(-) > > diff --git a/tools/testing/selftests/bpf/prog_tests/verifier.c b/tools/testing/selftests/bpf/prog_tests/verifier.c > index d9f65adb456b..3ee40ee9413a 100644 > --- a/tools/testing/selftests/bpf/prog_tests/verifier.c > +++ b/tools/testing/selftests/bpf/prog_tests/verifier.c > @@ -225,24 +225,7 @@ void test_verifier_xdp(void) { RUN(verifier_xdp); } > void test_verifier_xdp_direct_packet_access(void) { RUN(verifier_xdp_direct_packet_access); } > void test_verifier_bits_iter(void) { RUN(verifier_bits_iter); } > void test_verifier_lsm(void) { RUN(verifier_lsm); } > - > -void test_verifier_mtu(void) > -{ > - __u64 caps = 0; > - int ret; > - > - /* In case CAP_BPF and CAP_PERFMON is not set */ > - ret = cap_enable_effective(1ULL << CAP_BPF | 1ULL << CAP_NET_ADMIN, &caps); > - if (!ASSERT_OK(ret, "set_cap_bpf_cap_net_admin")) > - return; > - ret = cap_disable_effective(1ULL << CAP_SYS_ADMIN | 1ULL << CAP_PERFMON, NULL); > - if (!ASSERT_OK(ret, "disable_cap_sys_admin")) > - goto restore_cap; > - RUN(verifier_mtu); > -restore_cap: > - if (caps) > - cap_enable_effective(caps, NULL); > -} > +void test_verifier_mtu(void) { RUN(verifier_mtu); } > > static int init_test_val_map(struct bpf_object *obj, char *map_name) > { > diff --git a/tools/testing/selftests/bpf/progs/bpf_misc.h b/tools/testing/selftests/bpf/progs/bpf_misc.h > index eccaf955e394..cd9dd427a91d 100644 > --- a/tools/testing/selftests/bpf/progs/bpf_misc.h > +++ b/tools/testing/selftests/bpf/progs/bpf_misc.h > @@ -106,6 +106,7 @@ > * __arch_* Specify on which architecture the test case should be tested. > * Several __arch_* annotations could be specified at once. > * When test case is not run on current arch it is marked as skipped. > + * __caps_unpriv Specify the capabilities that should be set when running the test > */ > #define __msg(msg) __attribute__((btf_decl_tag("comment:test_expect_msg=" XSTR(__COUNTER__) "=" msg))) > #define __xlated(msg) __attribute__((btf_decl_tag("comment:test_expect_xlated=" XSTR(__COUNTER__) "=" msg))) > @@ -129,6 +130,7 @@ > #define __arch_x86_64 __arch("X86_64") > #define __arch_arm64 __arch("ARM64") > #define __arch_riscv64 __arch("RISCV64") > +#define __caps_unpriv(caps) __attribute__((btf_decl_tag("comment:test_caps_unpriv=" XSTR(caps)))) > > /* Convenience macro for use with 'asm volatile' blocks */ > #define __naked __attribute__((naked)) > diff --git a/tools/testing/selftests/bpf/progs/verifier_mtu.c b/tools/testing/selftests/bpf/progs/verifier_mtu.c > index 70c7600a26a0..88b1fa5f6030 100644 > --- a/tools/testing/selftests/bpf/progs/verifier_mtu.c > +++ b/tools/testing/selftests/bpf/progs/verifier_mtu.c > @@ -6,7 +6,8 @@ > > SEC("tc/ingress") > __description("uninit/mtu: write rejected") > -__failure __msg("invalid indirect read from stack") > +__success __failure_unpriv __msg_unpriv("invalid indirect read from stack") nit: I'd move unpriv specification to a separate line to make it easier to follow that __success is for privileged, while unpriv has expected failure with a message > +__caps_unpriv(CAP_BPF) original code was setting both CAP_BPF and CAP_NET_ADMIN, but you are changing to just CAP_BPF? Any reason why? > int tc_uninit_mtu(struct __sk_buff *ctx) > { > __u32 mtu; [...]
