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

[PATCH bpf-next v2] 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.

bpf_skb_load_bytes_relative() has the same pattern and is reachable
from tc and sk_filter programs, so use memmove() there as well.
Reproduced with BPF_PROG_TEST_RUN on a tc program: loading into a
slice at offset 8 gave wrong bytes before the change and correct
bytes after.

Fixes: 2012c867c800 ("bpf: verifier: Accept dynptr mem as mem in helpers")
Signed-off-by: Ayushman Rout <ayushmanrout27@gmail.com>
Assisted-by: LLM
---
v1: https://lore.kernel.org/bpf/20261021103957.17532-1-ayushmanrout27@gmail.com/
v1 -> v2: also use memmove() in bpf_skb_load_bytes_relative() (review by bot and Alexei)

 net/core/filter.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/net/core/filter.c b/net/core/filter.c
index b7ac145b68b1..09c60a58c82a 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:
@@ -1871,7 +1871,7 @@ BPF_CALL_5(bpf_skb_load_bytes_relative, const struct sk_buff *, skb,
 	ptr = start + offset;
 
 	if (likely(ptr + len <= end)) {
-		memcpy(to, ptr, len);
+		memmove(to, ptr, len);
 		return 0;
 	}
 
@@ -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 v2] bpf: Use memmove() to load bytes from an skb and an xdp_buff

[bot+bpf-ci]

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

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

TL;DR: memmove() fix leaves the multi-frag xdp path (bpf_xdp_copy_buf) using
forward memcpy(), so bpf_dynptr_read() into an overlapping rdwr slice may
still corrupt data; the store path looks similar.

> diff --git a/net/core/filter.c b/net/core/filter.c
> --- a/net/core/filter.c
> +++ b/net/core/filter.c

[ ... ]

> @@ -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;
>  }

The commit message says "A read that spans xdp frags still goes through
bpf_xdp_copy_buf() and is not changed here."

Can the same corruption still happen on that path?

bpf_xdp_copy_buf() copies segment by segment and writes into buf as it goes:

    memcpy(dst, src, copy_len);

For an xdp dynptr, bpf_dynptr_slice_rdwr() with a NULL buffer returns a
direct pointer into whichever frag holds the requested range, so buf can
still be a slice of the same packet.

Consider a linear part of 100 bytes and a 4096 byte frag0:

    dst = bpf_dynptr_slice_rdwr(&ptr, 110, NULL, 64);   /* frag0 + 10 */
    bpf_dynptr_read(dst, 64, &ptr, 90, 0);

    bpf_dynptr_read()
      __bpf_dynptr_read()
        __bpf_xdp_load_bytes()
          bpf_xdp_load_bytes()
            bpf_xdp_pointer(xdp, 90, 64)   /* NULL, since 90 + 64 > 100 */
            bpf_xdp_copy_buf(xdp, 90, dst, 64, false)

The first memcpy() copies linear[90..100) onto frag0[10..20), which
clobbers source bytes that have not been read yet.

The next memcpy() then copies frag0[0..54) onto frag0[20..74), a forward
copy over an overlapping range.

A user-space model of this loop with forward copying returned 44 of 64 bytes
wrong.

The skb side does not look affected, since skb slices are always in the
linear head and skb_copy_bits() copies into a destination below the source.

Would it make sense to make the multi-segment copy overlap safe, for example
by going through a bounce buffer, or else to say in the commit message that
the xdp frag case remains broken?  The store path
(bpf_xdp_store_bytes() -> bpf_xdp_copy_buf(..., true)) has the same shape.


---
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/39772397846

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

[patchwork-bot+netdevbpf]

Hello:

This patch was applied to bpf/bpf.git (master)
by Alexei Starovoitov <ast@kernel.org>:

On Thu, 22 Oct 2026 16:53:11 +0530 you wrote:
> 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);
> 
> [...]

Here is the summary with links:
  - [bpf-next,v2] bpf: Use memmove() to load bytes from an skb and an xdp_buff
    https://git.kernel.org/bpf/bpf/c/b93cf2c8a055

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html