On Wed, 2025-02-19 at 04:51 -0800, Kumar Kartikeya Dwivedi wrote:
> Ensure that once we get the return value and write to a stack slot it
> may potentially alias, we don't get confused about the state of the
> stack. Without the fix in the previous patch, we will assume the load
> from r8 into r0 before will always be from a map value, but in the case
> where the returned value is the passed in buffer, we're writing to fp-8
> and will overwrite the map value stored there.
>
> Signed-off-by: Kumar Kartikeya Dwivedi <memxor@xxxxxxxxx>
> ---
> .../testing/selftests/bpf/progs/dynptr_fail.c | 45 +++++++++++++++++++
> 1 file changed, 45 insertions(+)
>
> diff --git a/tools/testing/selftests/bpf/progs/dynptr_fail.c b/tools/testing/selftests/bpf/progs/dynptr_fail.c
> index bd8f15229f5c..4584bf53c5f8 100644
> --- a/tools/testing/selftests/bpf/progs/dynptr_fail.c
> +++ b/tools/testing/selftests/bpf/progs/dynptr_fail.c
> @@ -1735,3 +1735,48 @@ int test_dynptr_reg_type(void *ctx)
> global_call_bpf_dynptr((const struct bpf_dynptr *)current);
> return 0;
> }
> +
> +SEC("?tc")
> +__failure __msg("R8 invalid mem access 'scalar'") __log_level(2)
> +int dynptr_slice_rdwr_overwrite(struct __sk_buff *ctx)
> +{
> + asm volatile (
Nit: having a few comments with equivalent C code would help
understand this test case.
> + "r6 = %[array_map4] ll; \
> + r9 = r1; \
> + r1 = r6; \
> + r2 = r10; \
> + r2 += -8; \
> + call %[bpf_map_lookup_elem]; \
> + if r0 == 0 goto rjmp1; \
> + *(u64 *)(r10 - 8) = r0; \
> + r8 = r0; \
> + r1 = r9; \
> + r2 = 0; \
> + r3 = r10; \
> + r3 += -24; \
> + call %[bpf_dynptr_from_skb]; \
> + r1 = r10; \
> + r1 += -24; \
> + r2 = 0; \
> + r3 = r10; \
> + r3 += -8; \
> + r4 = 8; \
> + call %[bpf_dynptr_slice_rdwr]; \
> + if r0 == 0 goto rjmp1; \
> + r1 = 0; \
> + *(u64 *)(r10 - 8) = r8; \
> + *(u64 *)(r0 + 0) = r1; \
> + r8 = *(u64 *)(r10 - 8); \
> + r0 = *(u64 *)(r8 + 0); \
> + rjmp1: \
Note: 'rjmp1' would be a global label.
An alternative would be to either use 'goto 1f' and label '1:',
or use the '%=' counter: 'goto rjmp1_%=', 'rjmp_%=:'.
These would make label names unique for this inline assembly block.
See https://gcc.gnu.org/onlinedocs/gcc/Extended-Asm.html#Special-format-strings ,
And https://sourceware.org/binutils/docs/as/Symbol-Names.html .
> + r0 = 0; \
> + "
> + :
> + : __imm(bpf_map_lookup_elem),
> + __imm(bpf_dynptr_from_skb),
> + __imm(bpf_dynptr_slice_rdwr),
> + __imm_addr(array_map4)
> + : __clobber_all
> + );
> + return 0;
> +}
