Re: [PATCH bpf-next v1 1/2] selftests/bpf: Handle EOPNOTSUPP gracefully in test_loader

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] selftests/bpf: indiscriminate EOPNOTSUPP skip in run_subtest()
--

commit 4fea47634d9e4204dd230efbc533c57d0013c98f
Author: Tiezhu Yang <yangtiezhu@loongson.cn>

selftests/bpf: Handle EOPNOTSUPP gracefully in test_loader

This commit updates run_subtest() in test_loader to call test__skip() when
bpf_object__load() fails with errno == EOPNOTSUPP in the
!subspec->expect_failure path.

> diff --git a/tools/testing/selftests/bpf/test_loader.c b/tools/testing/selftests/bpf/test_loader.c
> index d24e4fedae46f..021818afbd3bf 100644
> --- a/tools/testing/selftests/bpf/test_loader.c
> +++ b/tools/testing/selftests/bpf/test_loader.c
> @@ -1474,6 +1474,10 @@ void run_subtest(struct test_loader *tester,
>  			goto tobj_cleanup;
>  		}
>  	} else {
> +		if (err < 0 && errno == EOPNOTSUPP) {
> +			test__skip();
> +			goto tobj_cleanup;
> +		}

[Severity: Medium]
Does checking errno == EOPNOTSUPP after bpf_object__load() in the
!subspec->expect_failure branch mask real verifier, BTF, CO-RE, and JIT
regressions while still leaving __failure and __prepare_priv subtests
broken when BPF_MAP_TYPE_ARENA is unsupported?

In tools/lib/bpf/libbpf.c, bpf_object__load() executes both
bpf_object_prepare() (BTF loading, CO-RE relocations, and
bpf_object__create_maps()) and bpf_object__load_progs() (program
verification and JIT compilation). During these steps, -EOPNOTSUPP is
returned for many conditions unrelated to missing platform hardware
extensions, such as add_kfunc_call(), require_callx_jit(),
process_timer_func(), sanitize_check_bounds(), check_hmul_jit(), and
check_attach_btf_id() in kernel/bpf/verifier.c, convert_ctx_accesses() in
kernel/bpf/fixups.c, btf_validate_return_type() and btf_scan_decl_tags()
in kernel/bpf/btf.c, check_struct_ops_btf_id() in
kernel/bpf/bpf_struct_ops.c, and bpf_core_apply_relo_insn() in
tools/lib/bpf/relo_core.c. Skipping any !subspec->expect_failure subtest
on EOPNOTSUPP will cause regressions in those paths to be silently skipped
instead of failing.

At the same time, when BPF_MAP_TYPE_ARENA is unsupported on the platform,
arena_map_alloc() in kernel/bpf/arena.c returns -EOPNOTSUPP during
bpf_object_prepare() -> bpf_object__create_maps(), so bpf_object__load()
aborts before bpf_object__load_progs() is called and leaves
tester->log_buf empty. For __failure subtests in objects declaring an
arena map (such as access_through_number in
tools/testing/selftests/bpf/progs/verifier_arena.c or arena_arg_bad_reg in
tools/testing/selftests/bpf/progs/arena_kfunc.c), run_subtest() takes the
subspec->expect_failure branch:

tools/testing/selftests/bpf/test_loader.c:run_subtest() {
	...
	if (subspec->expect_failure) {
		if (!ASSERT_ERR(err, "unexpected_load_success")) {
			emit_verifier_log(tester->log_buf, false /*force*/);
			goto tobj_cleanup;
		}
	}
	...
	err = validate_msgs(tester->log_buf, &subspec->expect_msgs,
			    emit_verifier_log);
}

Here ASSERT_ERR() succeeds on the map creation error, and validate_msgs()
then fails because the verifier never ran and tester->log_buf is empty.
Similarly, if unpriv && spec->prepare_priv is set, bpf_object__prepare()
in run_subtest() fails at ASSERT_OK(err, "obj_prepare") before reaching
bpf_object__load():

tools/testing/selftests/bpf/test_loader.c:run_subtest() {
	...
	if (unpriv && spec->prepare_priv) {
		err = bpf_object__prepare(tobj);
		if (!ASSERT_OK(err, "obj_prepare"))
			goto tobj_cleanup;
	...
}

In addition, because run_subtest() enables map autocreation for all maps
in tobj in privileged mode, any object declaring an arena map alongside
unrelated non-arena subtests (such as compute_live_registers.c,
verifier_zext.c, and verifier_ldsx.c) will fail map creation in
bpf_object_prepare() and skip all non-arena subtests in that object when
BPF_MAP_TYPE_ARENA is unsupported.

Would it be better to explicitly probe for BPF_MAP_TYPE_ARENA support
(similar to libbpf_probe_bpf_map_type(BPF_MAP_TYPE_ARENA, NULL) in
test_verifier_hmul_arena() in
tools/testing/selftests/bpf/prog_tests/verifier_hmul_arena.c) or handle
unsupported map creation specifically, rather than catching all EOPNOTSUPP
errors from bpf_object__load()?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261102081230.7063-1-yangtiezhu@loongson.cn?part=1