[PATCH bpf 1/3] bpf: Fix bpf_xdp_copy_buf() when the buffer overlaps the packet

[Ayushman Rout]

bpf_xdp_copy_buf() moves data between a caller buffer and an xdp_buff
one segment at a time (the linear part, then each frag) with memcpy().
bpf_xdp_load_bytes(), bpf_xdp_store_bytes() and bpf_dynptr_slice() with
a buffer all use it.

The buffer does not have to be outside the packet. A program can pass a
slice of the same packet, for example one from bpf_dynptr_slice_rdwr().
Then a piece that is copied early can overwrite bytes that a later piece
still has to read. memcpy() is also not valid for overlapping memory.

Example: a 100 byte linear part, a 4096 byte frag0, a buffer at
frag0 + 10 and a 64 byte load from offset 90. The first piece copies
linear[90..100) to frag0[10..20), which destroys data that the second
piece needs from frag0[0..54). A store fails in the same way.

Commit ab08863c58b1 ("bpf: Use memmove() to store bytes in an
xdp_buff") fixed the case where the range is inside one segment. A
range that crosses from the linear part into a frag still goes through
here and is not covered.

Use memmove() for every piece. For a load, copy the piece that is in the
same segment as the buffer first. For a store, copy it last. This
assumes that the frags do not overlap each other.

The selftests added later in this series fail without this patch.

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

diff --git a/net/core/filter.c b/net/core/filter.c
index 09c60a58c82a..716efafe7e0a 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -4210,47 +4210,58 @@ static const struct bpf_func_proto bpf_xdp_adjust_head_proto = {
 void bpf_xdp_copy_buf(struct xdp_buff *xdp, unsigned long off,
 		      void *buf, unsigned long len, bool flush)
 {
-	unsigned long ptr_len, ptr_off = 0;
+	unsigned long ptr_len, ptr_off, o, l;
 	skb_frag_t *next_frag, *end_frag;
 	struct skb_shared_info *sinfo;
+	u8 *ptr_buf, *b, *orig = buf;
 	void *src, *dst;
-	u8 *ptr_buf;
+	int pass;
 
 	if (likely(xdp->data_end - xdp->data >= off + len)) {
 		src = flush ? buf : xdp->data + off;
 		dst = flush ? xdp->data + off : buf;
-		memcpy(dst, src, len);
+		memmove(dst, src, len);
 		return;
 	}
 
 	sinfo = xdp_get_shared_info_from_buff(xdp);
 	end_frag = &sinfo->frags[sinfo->nr_frags];
-	next_frag = &sinfo->frags[0];
 
-	ptr_len = xdp->data_end - xdp->data;
-	ptr_buf = xdp->data;
+	for (pass = 0; pass < 2; pass++) {
+		o = off;
+		l = len;
+		b = buf;
+		ptr_off = 0;
+		ptr_len = xdp->data_end - xdp->data;
+		ptr_buf = xdp->data;
+		next_frag = &sinfo->frags[0];
+
+		while (true) {
+			if (o < ptr_off + ptr_len) {
+				unsigned long copy_off = o - ptr_off;
+				unsigned long copy_len = min(l, ptr_len - copy_off);
+				bool self = orig >= ptr_buf && orig < ptr_buf + ptr_len;
+				bool turn = pass == flush;
+
+				if (self == turn) {
+					src = flush ? b : ptr_buf + copy_off;
+					dst = flush ? ptr_buf + copy_off : b;
+					memmove(dst, src, copy_len);
+				}
 
-	while (true) {
-		if (off < ptr_off + ptr_len) {
-			unsigned long copy_off = off - ptr_off;
-			unsigned long copy_len = min(len, ptr_len - copy_off);
+				o += copy_len;
+				l -= copy_len;
+				b += copy_len;
+			}
 
-			src = flush ? buf : ptr_buf + copy_off;
-			dst = flush ? ptr_buf + copy_off : buf;
-			memcpy(dst, src, copy_len);
+			if (!l || next_frag == end_frag)
+				break;
 
-			off += copy_len;
-			len -= copy_len;
-			buf += copy_len;
+			ptr_off += ptr_len;
+			ptr_buf = skb_frag_address(next_frag);
+			ptr_len = skb_frag_size(next_frag);
+			next_frag++;
 		}
-
-		if (!len || next_frag == end_frag)
-			break;
-
-		ptr_off += ptr_len;
-		ptr_buf = skb_frag_address(next_frag);
-		ptr_len = skb_frag_size(next_frag);
-		next_frag++;
 	}
 }
 
-- 
2.54.0