[PATCH bpf-next] bpf: Use memmove() to load bytes from an skb and an xdp_buff

[PATCH bpf-next] bpf: Use memmove() to load bytes from an skb and an xdp_buff

[Ayushman Rout]

bpf_dynptr_read() on an skb or xdp dynptr ends in bpf_skb_load_bytes()
or bpf_xdp_load_bytes(). The destination can be a slice of the same
packet, so it can overlap the source, which memcpy() does not allow.
On x86 with FSRM memcpy() copies forward, so a destination above the
source gets the first bytes repeated:

  p = bpf_dynptr_slice_rdwr(&ptr, 8, NULL, 64);
  bpf_dynptr_read(p, 64, &ptr, 0, 0);

leaves bytes 8-71 of the packet as bytes 0-7 repeated, not bytes 0-63.
Reproduced with BPF_PROG_TEST_RUN on both an xdp and a tc program.

Use memmove(), as the store side does since commit 1db084f42884
("bpf: Use memmove() to store bytes in an skb") and commit ab08863c58b1
("bpf: Use memmove() to store bytes in an xdp_buff"). A read that spans
xdp frags still goes through bpf_xdp_copy_buf() and is not changed
here.

Fixes: 2012c867c800 ("bpf: verifier: Accept dynptr mem as mem in helpers")
Signed-off-by: Ayushman Rout <ayushmanrout27@gmail.com>
Assisted-by: LLM
---
 net/core/filter.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/core/filter.c b/net/core/filter.c
index b7ac145b68b1..35d38e02fc1e 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -1789,7 +1789,7 @@ BPF_CALL_4(bpf_skb_load_bytes, const struct sk_buff *, skb, u32, offset,
 	if (unlikely(!ptr))
 		goto err_clear;
 	if (ptr != to)
-		memcpy(to, ptr, len);
+		memmove(to, ptr, len);
 
 	return 0;
 err_clear:
@@ -4298,7 +4298,7 @@ BPF_CALL_4(bpf_xdp_load_bytes, struct xdp_buff *, xdp, u32, offset,
 	if (!ptr)
 		bpf_xdp_copy_buf(xdp, offset, buf, len, false);
 	else
-		memcpy(buf, ptr, len);
+		memmove(buf, ptr, len);
 
 	return 0;
 }
-- 
2.54.0

Re: [PATCH bpf-next] bpf: Use memmove() to load bytes from an skb and an xdp_buff

[bot+bpf-ci]

[-- Attachment #1: Type: text/plain, Size: 2071 bytes --]

> bpf: Use memmove() to load bytes from an skb and an xdp_buff

TL;DR: bpf_skb_load_bytes_relative() still uses memcpy() and may corrupt data
when its destination is an overlapping dynptr slice; should it get the same
memmove() change, or should the commit message explain why not?

> diff --git a/net/core/filter.c b/net/core/filter.c
> --- a/net/core/filter.c
> +++ b/net/core/filter.c
> @@ -1789,7 +1789,7 @@ BPF_CALL_4(bpf_skb_load_bytes, const struct sk_buff *, skb, u32, offset,
>  	if (unlikely(!ptr))
>  		goto err_clear;
>  	if (ptr != to)
> -		memcpy(to, ptr, len);
> +		memmove(to, ptr, len);
>
>  	return 0;
>  err_clear:

Should bpf_skb_load_bytes_relative() get the same change?

It has the same pattern and still uses memcpy():

net/core/filter.c:bpf_skb_load_bytes_relative() {
    ...
	ptr = start + offset;

	if (likely(ptr + len <= end)) {
		memcpy(to, ptr, len);
		return 0;
	}
    ...
}

Its proto declares .arg3_type = ARG_PTR_TO_UNINIT_MEM, the same arg type as
bpf_skb_load_bytes() uses for 'to', so the verifier should also accept a
slice returned by bpf_dynptr_slice_rdwr() as the destination there.  The
helper is available to tc programs through tc_cls_act_func_proto() and to
sk_filter programs.

Since ptr is only checked against skb_tail_pointer(), the source can overlap
a rdwr slice taken from the linear area.  For example, in a tc program:

  p = bpf_dynptr_slice_rdwr(&ptr, 8, NULL, 64);
  bpf_skb_load_bytes_relative(skb, 0, p, 64, BPF_HDR_START_NET);

This takes the memcpy() path with dst = src + 8, which on x86 with FSRM
would repeat the first 8 bytes, the same corruption the commit message
describes for bpf_dynptr_read().

The Fixes: tag and the subject ("load bytes from an skb") would seem to
cover this helper as well.  If it is left out on purpose, could the commit
message say so?


---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md

CI run summary: https://github.com/kernel-patches/bpf/actions/runs/39587398602

Re: [PATCH bpf-next] bpf: Use memmove() to load bytes from an skb and an xdp_buff

[Alexei Starovoitov]

On Wed Oct 21, 2026 at 11:04 AM UTC, bot+bpf-ci wrote:
>> bpf: Use memmove() to load bytes from an skb and an xdp_buff
>
> TL;DR: bpf_skb_load_bytes_relative() still uses memcpy() and may corrupt data
> when its destination is an overlapping dynptr slice; should it get the same
> memmove() change, or should the commit message explain why not?
>
>> diff --git a/net/core/filter.c b/net/core/filter.c
>> --- a/net/core/filter.c
>> +++ b/net/core/filter.c
>> @@ -1789,7 +1789,7 @@ BPF_CALL_4(bpf_skb_load_bytes, const struct sk_buff *, skb, u32, offset,
>>  	if (unlikely(!ptr))
>>  		goto err_clear;
>>  	if (ptr != to)
>> -		memcpy(to, ptr, len);
>> +		memmove(to, ptr, len);
>>
>>  	return 0;
>>  err_clear:
>
> Should bpf_skb_load_bytes_relative() get the same change?

+1

pw-bot: cr