Re: [PATCH bpf-next 2/2] selftests/bpf: Add more tests for loading insn arrays with offsets

[Mykyta Yatsenko <mykyta.yatsenko5@gmail.com>]

On 4/1/26 5:15 PM, Anton Protopopov wrote:
> A typical series of load instructions for a gotox looks like
> 
>     r1 = &map + offset1
>     r1 += offset2
>     r1 = *(r1 + offset3)
>     gotox r1
> 
> Here offset3 is, normally, equal to zero; but this is not guaranteed.
> Extend selftests with tests for non-zero offset3 and, while here, also
> add tests for negative offsets (the offset1 and the sum of three offsets
> still have to be non-negative).
> 
> Signed-off-by: Anton Protopopov <a.s.protopopov@gmail.com>
> ---
>   .../selftests/bpf/prog_tests/bpf_gotox.c      | 114 +++++++++++-------
>   1 file changed, 73 insertions(+), 41 deletions(-)
> 
> diff --git a/tools/testing/selftests/bpf/prog_tests/bpf_gotox.c b/tools/testing/selftests/bpf/prog_tests/bpf_gotox.c
> index 75b0cf2467ab..594adf698fdb 100644
> --- a/tools/testing/selftests/bpf/prog_tests/bpf_gotox.c
> +++ b/tools/testing/selftests/bpf/prog_tests/bpf_gotox.c
> @@ -317,7 +317,7 @@ static void check_ldimm64_off_load(struct bpf_gotox *skel __always_unused)
>   
>   static int __check_ldimm64_gotox_prog_load(struct bpf_insn *insns,
>   					   __u32 insn_cnt,
> -					   __u32 off1, __u32 off2)
> +					   int off1, int off2, int off3)
>   {
>   	const __u32 values[] = {5, 7, 9, 11, 13, 15};
>   	const __u32 max_entries = ARRAY_SIZE(values);
> @@ -349,16 +349,46 @@ static int __check_ldimm64_gotox_prog_load(struct bpf_insn *insns,
>   	/* r1 += off2 */
>   	insns[2].imm = off2;
>   
> +	/* r1 = *(r1 + off3) */
> +	insns[3].off = off3;
> +
>   	ret = prog_load(insns, insn_cnt);
>   	close(map_fd);
>   	return ret;
>   }
>   
> -static void reject_offsets(struct bpf_insn *insns, __u32 insn_cnt, __u32 off1, __u32 off2)
> +static void
> +allow_offsets(struct bpf_insn *insns, __u32 insn_cnt, int off1, int off2, int off3)
> +{
> +	LIBBPF_OPTS(bpf_test_run_opts, topts);
> +	int prog_fd, err;
> +	char s[128] = "";
> +
> +	prog_fd = __check_ldimm64_gotox_prog_load(insns, insn_cnt, off1, off2, off3);
> +	snprintf(s, sizeof(s), "__check_ldimm64_gotox_prog_load(%u,%u,%u)", off1, off2, off3);

here offsets are int, but printed with %u.

> +	if (!ASSERT_GE(prog_fd, 0, s))
> +		return;
> +
> +	err = bpf_prog_test_run_opts(prog_fd, &topts);
> +	if (!ASSERT_OK(err, "test_run_opts err")) {
> +		close(prog_fd);
> +		return;
> +	}
> +
> +	if (!ASSERT_EQ(topts.retval, (off1 + off2 + off3) / 8, "test_run_opts retval")) {
> +		close(prog_fd);
> +		return;
> +	}
> +
> +	close(prog_fd);
> +}
> +
> +static void
> +reject_offsets(struct bpf_insn *insns, __u32 insn_cnt, __u32 off1, __u32 off2, __u32 off3)

If offsets can be negative, should these arguments be int rather than 
__u32, also matching reject_offsets?

>   {
>   	int prog_fd;
>   
> -	prog_fd = __check_ldimm64_gotox_prog_load(insns, insn_cnt, off1, off2);
> +	prog_fd = __check_ldimm64_gotox_prog_load(insns, insn_cnt, off1, off2, off3);
>   	if (!ASSERT_EQ(prog_fd, -EACCES, "__check_ldimm64_gotox_prog_load"))
>   		close(prog_fd);
>   }
> @@ -376,7 +406,7 @@ static void check_ldimm64_off_gotox(struct bpf_gotox *skel __always_unused)
>   		 * The program rewrites the offsets in the instructions below:
>   		 *     r1 = &map + offset1
>   		 *     r1 += offset2
> -		 *     r1 = *r1
> +		 *     r1 = *(r1 + offset3)
>   		 *     gotox r1
>   		 */
>   		BPF_LD_IMM64_RAW(BPF_REG_1, BPF_PSEUDO_MAP_VALUE, 0),
> @@ -403,43 +433,45 @@ static void check_ldimm64_off_gotox(struct bpf_gotox *skel __always_unused)
>   		BPF_MOV64_IMM(BPF_REG_0, 5),
>   		BPF_EXIT_INSN(),
>   	};
> -	int prog_fd, err;
> -	__u32 off1, off2;
> -
> -	/* allow all combinations off1 + off2 < 6 */
> -	for (off1 = 0; off1 < 6; off1++) {
> -		for (off2 = 0; off1 + off2 < 6; off2++) {
> -			LIBBPF_OPTS(bpf_test_run_opts, topts);
> -
> -			prog_fd = __check_ldimm64_gotox_prog_load(insns, ARRAY_SIZE(insns),
> -								  off1 * 8, off2 * 8);
> -			if (!ASSERT_GE(prog_fd, 0, "__check_ldimm64_gotox_prog_load"))
> -				return;
> -
> -			err = bpf_prog_test_run_opts(prog_fd, &topts);
> -			if (!ASSERT_OK(err, "test_run_opts err")) {
> -				close(prog_fd);
> -				return;
> -			}
> -
> -			if (!ASSERT_EQ(topts.retval, off1 + off2, "test_run_opts retval")) {
> -				close(prog_fd);
> -				return;
> -			}
> -
> -			close(prog_fd);
> -		}
> -	}
> -
> -	/* reject off1 + off2 >= 6 */
> -	reject_offsets(insns, ARRAY_SIZE(insns), 8 * 3, 8 * 3);
> -	reject_offsets(insns, ARRAY_SIZE(insns), 8 * 7, 8 * 0);
> -	reject_offsets(insns, ARRAY_SIZE(insns), 8 * 0, 8 * 7);
> -
> -	/* reject (off1 + off2) % 8 != 0 */
> -	reject_offsets(insns, ARRAY_SIZE(insns), 3, 3);
> -	reject_offsets(insns, ARRAY_SIZE(insns), 7, 0);
> -	reject_offsets(insns, ARRAY_SIZE(insns), 0, 7);
> +	int off1, off2, off3;
> +
> +	/* allow all combinations off1 + off2 + off3 < 6 */
> +	for (off1 = 0; off1 < 6; off1++)
> +		for (off2 = 0; off1 + off2 < 6; off2++)
> +			for (off3 = 0; off1 + off2 + off3 < 6; off3++)
> +				allow_offsets(insns, ARRAY_SIZE(insns),
> +					      off1 * 8, off2 * 8, off3 * 8);
> +
> +	/* allow for some offsets to be negative */
> +	allow_offsets(insns, ARRAY_SIZE(insns), 8 * 3, 0, -(8 * 3));
> +	allow_offsets(insns, ARRAY_SIZE(insns), 8 * 3, -(8 * 3), 0);
> +	allow_offsets(insns, ARRAY_SIZE(insns), 0, 8 * 3, -(8 * 3));
> +	allow_offsets(insns, ARRAY_SIZE(insns), 8 * 4, 0, -(8 * 2));
> +	allow_offsets(insns, ARRAY_SIZE(insns), 8 * 4, -(8 * 2), 0);
> +	allow_offsets(insns, ARRAY_SIZE(insns), 0, 8 * 4, -(8 * 2));
> +
> +	/* disallow negative sums of offsets */
> +	reject_offsets(insns, ARRAY_SIZE(insns), 8 * 3, 0, -(8 * 4));
> +	reject_offsets(insns, ARRAY_SIZE(insns), 8 * 3, -(8 * 4), 0);
> +	reject_offsets(insns, ARRAY_SIZE(insns), 0, 8 * 3, -(8 * 4));
> +
> +	/* disallow the off1 to be negative in any case */
> +	reject_offsets(insns, ARRAY_SIZE(insns), -8 * 1, 0, 0);
> +	reject_offsets(insns, ARRAY_SIZE(insns), -8 * 1, 8 * 1, 0);
> +	reject_offsets(insns, ARRAY_SIZE(insns), -8 * 1, 8 * 1, 8 * 1);
> +
> +	/* reject off1 + off2 + off3 >= 6 */
> +	reject_offsets(insns, ARRAY_SIZE(insns), 8 * 3, 8 * 3, 8 * 0);
> +	reject_offsets(insns, ARRAY_SIZE(insns), 8 * 7, 8 * 0, 8 * 0);
> +	reject_offsets(insns, ARRAY_SIZE(insns), 8 * 0, 8 * 7, 8 * 0);
> +	reject_offsets(insns, ARRAY_SIZE(insns), 8 * 3, 8 * 0, 8 * 3);
> +	reject_offsets(insns, ARRAY_SIZE(insns), 8 * 0, 8 * 3, 8 * 3);
> +
> +	/* reject (off1 + off2) % 8 != 0, off3 % 8 != 0 */
> +	reject_offsets(insns, ARRAY_SIZE(insns), 3, 3, 0);
> +	reject_offsets(insns, ARRAY_SIZE(insns), 7, 0, 0);
> +	reject_offsets(insns, ARRAY_SIZE(insns), 0, 7, 0);
> +	reject_offsets(insns, ARRAY_SIZE(insns), 0, 0, 7);
>   }
>   
>   void test_bpf_gotox(void)