Re: [PATCH v8 bpf-next 10/11] selftests/bpf: add new verifier_gotox test

[Anton Protopopov <a.s.protopopov@gmail.com>]

On 25/10/28 02:45PM, bot+bpf-ci@kernel.org wrote:
> 
> ```
> commit 817a92582203a0d4c62d2de0abae0bd4a5418fa3
> Author: Anton Protopopov <a.s.protopopov@gmail.com>
> 
> selftests/bpf: add new verifier_gotox test
> 
> This commit adds a new BPF selftest file to validate the gotox
> (indirect jump) instruction functionality. The tests cover various
> edge cases including reserved field validation, bounds checking,
> alignment, and register usage.
> 
> > diff --git a/tools/testing/selftests/bpf/prog_tests/verifier.c b/tools/testing/selftests/bpf/prog_tests/verifier.c
> > index c0e8ffdaa..4b4b081b4 100644
> > --- a/tools/testing/selftests/bpf/prog_tests/verifier.c
> > +++ b/tools/testing/selftests/bpf/prog_tests/verifier.c
> 
> [ ... ]
> 
> > diff --git a/tools/testing/selftests/bpf/progs/verifier_gotox.c b/tools/testing/selftests/bpf/progs/verifier_gotox.c
> > new file mode 100644
> > index 000000000..f7bf0a5e0
> > --- /dev/null
> > +++ b/tools/testing/selftests/bpf/progs/verifier_gotox.c
> 
> [ ... ]
> 
> > +SEC("socket")
> > +__failure __msg("writes into insn_array not allowed")
> > +__naked void jump_table_no_writes(void)
> > +{
> > +	asm volatile ("						\
> > +	.pushsection .jumptables,\"\",@progbits;		\
> > +jt0_%=:								\
> > +	.quad ret0_%=;						\
> > +	.quad ret1_%=;						\
> > +	.size jt0_%=, 16;					\
> > +	.global jt0_%=;						\
> > +	.popsection;						\
> > +								\
> > +	r0 = jt0_%= ll;						\
> > +	r0 += 8;						\
> > +	r1 = 0xbeef;						\
> > +	*(u64 *)(r0 + 0) = r1;					\
> 
> [ ... ]
> 
> > +SEC("socket")
> > +__failure __msg("jump table for insn 4 points outside of the subprog [0,10]")
> > +__naked void jump_table_outside_subprog(void)
> > +{
> > +	asm volatile ("						\
> > +	.pushsection .jumptables,\"\",@progbits;		\
> > +jt0_%=:								\
> > +	.quad ret0_%=;						\
> > +	.quad ret1_%=;						\
> > +	.quad ret_out_%=;					\
> > +	.size jt0_%=, 24;					\
> > +	.global jt0_%=;						\
> > +	.popsection;						\
> > +								\
> > +	r0 = jt0_%= ll;						\
> > +	r0 += 8;						\
> > +	*(u64 *)(r0 + 0) = r1;					\
> > +	.8byte %[gotox_r0];					\
> 
> Does jump_table_outside_subprog() use the correct operation?  This test

For this test it doesn't matter: the error triggered by this test happens
before the check for the writes. (But thanks AI, I will change the line
for better readability.)

> writes to the jump table (line marked with * above), but the expected
> error message indicates it should be testing bounds checking, not write
> protection. All other bounds validation tests in the file use read
> operations like "r0 = *(u64 *)(r0 + 0);" (see jump_table_misaligned_access,
> jump_table_invalid_mem_acceess_pos, jump_table_invalid_mem_acceess_neg).
> The write operation will cause the verifier to reject the program with
> "writes into insn_array not allowed" before it can validate whether the
> jump target is within the subprogram boundary.
> 
> Should this be changed to a read operation to properly test the intended
> bounds check?
> 
> 
> ```
> 
> ---
> AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
> See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md
> 
> In-Reply-To-Subject: `selftests/bpf: add new verifier_gotox test`
> CI run summary: https://github.com/kernel-patches/bpf/actions/runs/18878193924