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.
