Re: [PATCH bpf-next 2/2] selftests/bpf: Add cross-subprog gotox target coverage

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The test `check_cross_subprog_gotox_target` incorrectly expects `bpf_prog_load` to fail with `-EACCES`, but the verifier actually rejects cross-subprog jump targets with `-EINVAL`, causing a false test failure.
--

commit f7ef2af9b62d2a415c2072928a61150823abda72
Author: Nuoqi Gui <gnq25@mails.tsinghua.edu.cn>
selftests/bpf: Add cross-subprog gotox target coverage

This commit adds a gotox regression test with two one-entry INSN_ARRAY maps
to verify that the control flow graph handles indirect jumps correctly. It
ensures that when a verified path loads a map whose target points to another
subprog, the program load is rejected.

> diff --git a/tools/testing/selftests/bpf/prog_tests/bpf_gotox.c b/tools/testing/selftests/bpf/prog_tests/bpf_gotox.c
> index 73dc63882b7d5..866b4a14ccb7c 100644
> --- a/tools/testing/selftests/bpf/prog_tests/bpf_gotox.c
> +++ b/tools/testing/selftests/bpf/prog_tests/bpf_gotox.c

[ ... ]

> @@ -393,6 +417,52 @@ reject_offsets(struct bpf_insn *insns, __u32 insn_cnt, int off1, int off2, int o
>  		close(prog_fd);
>  }
>  
> +static void
> +check_cross_subprog_gotox_target(struct bpf_gotox *skel __always_unused)
> +{

[ ... ]

> +	prog_fd = bpf_prog_load(BPF_PROG_TYPE_SOCKET_FILTER, NULL, "GPL",
> +				insns, ARRAY_SIZE(insns), NULL);
> +	if (!ASSERT_EQ(prog_fd, -EACCES, "cross_subprog_gotox_prog_load"))

[Severity: Medium]
Will this assertion fail because the verifier returns -EINVAL instead of
-EACCES?

When the verifier inspects the gotox target via check_indirect_jump(), the
target fails the is_cfg_indirect_jump_target() check. This causes the
verifier to explicitly reject the load with -EINVAL.

The memory access itself passes the verifier's check_map_access() bounds
check since it reads from a valid offset, so we don't get the -EACCES return
value here.

Should this test expect -EINVAL instead?

> +		close(prog_fd);
> +
> +	close(bad_fd);
> +	close(good_fd);
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260609-f01-02-gotox-bpf-next-v1-0-b441d63a1559@mails.tsinghua.edu.cn?part=2