Re: [PATCH bpf-next v2 3/5] selftests/bpf: XOR and OR range computation tests.

[Eduard Zingerman <eddyz87@gmail.com>]

On Wed, 2024-04-17 at 13:23 +0100, Cupertino Miranda wrote:

[...]

> +SEC("socket")
> +__description("bounds check for reg32 <= 1, 0 xor (0,1)")
> +__success __failure_unpriv
> +__msg_unpriv("R0 min value is outside of the allowed memory range")
> +__retval(0)
> +__naked void t_0_xor_01(void)
> +{
> +	asm volatile ("					\
> +	call %[bpf_get_prandom_u32];                    \
> +	r6 = r0;                                        \
> +	r1 = 0;						\
> +	*(u64*)(r10 - 8) = r1;				\
> +	r2 = r10;					\
> +	r2 += -8;					\
> +	r1 = %[map_hash_8b] ll;				\
> +	call %[bpf_map_lookup_elem];			\
> +	if r0 != 0 goto l0_%=;				\
> +	exit;						\
> +l0_%=:	w1 = 0;						\
> +	r6 >>= 63;					\
> +	w1 ^= w6;					\
> +	if w1 <= 1 goto l1_%=;				\
> +	r0 = *(u64*)(r0 + 8);				\
> +l1_%=:	r0 = 0;						\
> +	exit;						\
> +"	:
> +	: __imm(bpf_map_lookup_elem),
> +	  __imm_addr(map_hash_8b),
> +	  __imm(bpf_get_prandom_u32)
> +	: __clobber_all);
> +}
> +

I think that this test case (and one below) should be simplified,
e.g. as follows:

SEC("socket")
__success __log_level(2)
__msg("5: (af) r0 ^= r6                      ; R0_w=scalar(smin=smin32=0,smax=umax=smax32=umax32=255,var_off=(0x0; 0xff))")
__naked void non_const_xor_src_dst(void)
{
	asm volatile ("					\
	call %[bpf_get_prandom_u32];                    \
	r6 = r0;					\
	call %[bpf_get_prandom_u32];                    \
	r6 &= 0xff;					\
	r0 &= 0x0f;					\
	r0 ^= r6;					\
	exit;						\
"	:
	: __imm(bpf_map_lookup_elem),
	  __imm_addr(map_hash_8b),
	  __imm(bpf_get_prandom_u32)
	: __clobber_all);
}

Patch #2 allows verifier to compute dst range for xor operation with
non-constant src and dst registers, which is exactly what checked when
verifier log for instruction "r0 ^= r6" is verified.
Manipulations with maps, unpriv behavior and retval are just a distraction.