Skip to content

Commit

Permalink
selftests/bpf: Fix pointer arithmetic in test_xdp_do_redirect
Browse files Browse the repository at this point in the history
Cast operation has a higher precedence than addition. The code here
wants to zero the 2nd half of the 64-bit metadata, but due to a pointer
arithmetic mistake, it writes the zero at offset 16 instead.

Just adding parentheses around "data + 4" would fix this, but I think
this will be slightly better readable with array syntax.

I was unable to test this with tools/testing/selftests/bpf/vmtest.sh,
because my glibc is newer than glibc in the provided VM image.
So I just checked the difference in the compiled code.
objdump -S tools/testing/selftests/bpf/xdp_do_redirect.test.o:
  -	*((__u32 *)data) = 0x42; /* metadata test value */
  +	((__u32 *)data)[0] = 0x42; /* metadata test value */
        be7:	48 8d 85 30 fc ff ff 	lea    -0x3d0(%rbp),%rax
        bee:	c7 00 42 00 00 00    	movl   $0x42,(%rax)
  -	*((__u32 *)data + 4) = 0;
  +	((__u32 *)data)[1] = 0;
        bf4:	48 8d 85 30 fc ff ff 	lea    -0x3d0(%rbp),%rax
  -     bfb:	48 83 c0 10          	add    $0x10,%rax
  +     bfb:	48 83 c0 04          	add    $0x4,%rax
        bff:	c7 00 00 00 00 00    	movl   $0x0,(%rax)

Fixes: 5640b6d ("selftests/bpf: fix "metadata marker" getting overwritten by the netstack")
Signed-off-by: Michal Schmidt <[email protected]>
Signed-off-by: Andrii Nakryiko <[email protected]>
Reviewed-by: Toke Høiland-Jørgensen <[email protected]>
Link: https://lore.kernel.org/bpf/[email protected]
  • Loading branch information
michich authored and anakryiko committed May 6, 2024
1 parent 8e6d9ae commit e549b39
Showing 1 changed file with 2 additions and 2 deletions.
4 changes: 2 additions & 2 deletions tools/testing/selftests/bpf/prog_tests/xdp_do_redirect.c
Original file line number Diff line number Diff line change
Expand Up @@ -107,8 +107,8 @@ void test_xdp_do_redirect(void)
.attach_point = BPF_TC_INGRESS);

memcpy(&data[sizeof(__u64)], &pkt_udp, sizeof(pkt_udp));
*((__u32 *)data) = 0x42; /* metadata test value */
*((__u32 *)data + 4) = 0;
((__u32 *)data)[0] = 0x42; /* metadata test value */
((__u32 *)data)[1] = 0;

skel = test_xdp_do_redirect__open();
if (!ASSERT_OK_PTR(skel, "skel"))
Expand Down

0 comments on commit e549b39

Please sign in to comment.