From: "Alexei Starovoitov" <alexei.starovoitov@gmail.com>
To: "Ayushman Rout" <ayushmanrout27@gmail.com>, <bpf@vger.kernel.org>
Cc: "Daniel Borkmann" <daniel@iogearbox.net>,
"Andrii Nakryiko" <andrii@kernel.org>,
"Eduard Zingerman" <eddyz87@gmail.com>,
"Kumar Kartikeya Dwivedi" <memxor@gmail.com>,
"Martin KaFai Lau" <martin.lau@linux.dev>,
"Song Liu" <song@kernel.org>,
"Yonghong Song" <yonghong.song@linux.dev>,
"Jiri Olsa" <jolsa@kernel.org>,
"Emil Tsalapatis" <emil@etsalapatis.com>,
"Ihor Solodrai" <ihor.solodrai@linux.dev>,
"John Fastabend" <john.fastabend@gmail.com>,
"Stanislav Fomichev" <sdf@fomichev.me>,
"David S. Miller" <davem@davemloft.net>,
"Eric Dumazet" <edumazet@kernel.org>,
"Jakub Kicinski" <kuba@kernel.org>,
"Paolo Abeni" <pabeni@redhat.com>,
"Simon Horman" <horms@kernel.org>,
"Jesper Dangaard Brouer" <hawk@kernel.org>,
"Daniel Rosenberg" <drosen@google.com>, <netdev@vger.kernel.org>,
<linux-kernel@vger.kernel.org>
Subject: Re: [PATCH bpf 1/3] bpf: Fix bpf_xdp_copy_buf() when the buffer overlaps the packet
Date: Sun, 25 Oct 2026 00:57:22 +0000 [thread overview]
Message-ID: <DMDIENB8G58I.3IFV81HSBAA8H@gmail.com> (raw)
In-Reply-To: <20261024092659.76422-2-ayushmanrout27@gmail.com>
On Sat, Oct 24, 2026 at 02:26 AM Ayushman Rout <ayushmanrout27@gmail.com> wrote:
> + for (pass = 0; pass < 2; pass++) {
[...]
> + bool self = orig >= ptr_buf && orig < ptr_buf + ptr_len;
> + bool turn = pass == flush;
> +
> + if (self == turn) {
This is too clever for what it fixes.
Every load, store and bpf_xdp_output() that crosses into a frag now
walks the frags twice, to look for a buf that is on the stack, in a map
or in the perf buffer.
memmove() in bpf_xdp_load_bytes() and bpf_xdp_store_bytes() was a one
liner. This is not.
bpf_dynptr_copy() is there to move bytes within a packet, and it's the
only realistic way to get here with a slice: when one range fits into
a segment and the other one doesn't, it passes the slice of the first
as a buffer to __bpf_dynptr_read() or __bpf_dynptr_write().
With patch 2 the chunk loop copes with any overlap. Use it for every
copy within one packet and leave bpf_xdp_copy_buf() alone.
bpf_dynptr_read() and bpf_dynptr_write() with a slice as a buffer and
a range that crosses into a frag stay as they are. I don't think anyone
needs that.
The read and write tests of patch 3 become bpf_dynptr_copy() tests
where only one of the ranges crosses into frag0.
Something like this instead of patches 1 and 2 (compile tested only):
diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
index 648abe1..b74c036 100644
--- a/kernel/bpf/helpers.c
+++ b/kernel/bpf/helpers.c
@@ -3291,8 +3291,10 @@ __bpf_kfunc int bpf_dynptr_copy(const struct bpf_dynptr *dst_ptr, u64 dst_off,
{
const struct bpf_dynptr_kern *dst = (struct bpf_dynptr_kern *)dst_ptr;
const struct bpf_dynptr_kern *src = (struct bpf_dynptr_kern *)src_ptr;
+ bool same = src->data == dst->data;
void *src_slice, *dst_slice;
char buf[256];
+ bool rev;
u64 off;
src_slice = bpf_dynptr_slice(src_ptr, src_off, NULL, size);
@@ -3303,25 +3305,31 @@ __bpf_kfunc int bpf_dynptr_copy(const struct bpf_dynptr *dst_ptr, u64 dst_off,
return 0;
}
- if (src_slice)
+ /* The other range is copied piece by piece. A slice of the same packet
+ * may overlap with it, so it cannot be the buffer for that.
+ */
+ if (src_slice && !same)
return __bpf_dynptr_write(dst, dst_off, src_slice, size, 0);
- if (dst_slice)
+ if (dst_slice && !same)
return __bpf_dynptr_read(dst_slice, size, src, src_off, 0);
if (bpf_dynptr_check_off_len(dst, dst_off, size) ||
bpf_dynptr_check_off_len(src, src_off, size))
return -E2BIG;
+ /* like memmove(): from the end if the destination is above the source */
+ rev = same && dst->offset + dst_off > src->offset + src_off;
off = 0;
while (off < size) {
u64 chunk_sz = min_t(u64, sizeof(buf), size - off);
+ u64 pos = rev ? size - off - chunk_sz : off;
int err;
- err = __bpf_dynptr_read(buf, chunk_sz, src, src_off + off, 0);
+ err = __bpf_dynptr_read(buf, chunk_sz, src, src_off + pos, 0);
if (err)
return err;
- err = __bpf_dynptr_write(dst, dst_off + off, buf, chunk_sz, 0);
+ err = __bpf_dynptr_write(dst, dst_off + pos, buf, chunk_sz, 0);
if (err)
return err;
pw-bot: cr
next prev parent reply other threads:[~2026-10-25 0:57 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-10-24 9:26 [PATCH bpf 0/3] bpf: Fix overlapping copies in xdp and dynptr helpers Ayushman Rout
2026-10-24 9:26 ` [PATCH bpf 1/3] bpf: Fix bpf_xdp_copy_buf() when the buffer overlaps the packet Ayushman Rout
2026-10-25 0:57 ` Alexei Starovoitov [this message]
2026-10-24 9:26 ` [PATCH bpf 2/3] bpf: Fix bpf_dynptr_copy() for overlapping ranges in one packet Ayushman Rout
2026-10-24 9:26 ` [PATCH bpf 3/3] selftests/bpf: Test overlapping dynptr copies on xdp frags Ayushman Rout
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=DMDIENB8G58I.3IFV81HSBAA8H@gmail.com \
--to=alexei.starovoitov@gmail.com \
--cc=andrii@kernel.org \
--cc=ayushmanrout27@gmail.com \
--cc=bpf@vger.kernel.org \
--cc=daniel@iogearbox.net \
--cc=davem@davemloft.net \
--cc=drosen@google.com \
--cc=eddyz87@gmail.com \
--cc=edumazet@kernel.org \
--cc=emil@etsalapatis.com \
--cc=hawk@kernel.org \
--cc=horms@kernel.org \
--cc=ihor.solodrai@linux.dev \
--cc=john.fastabend@gmail.com \
--cc=jolsa@kernel.org \
--cc=kuba@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=martin.lau@linux.dev \
--cc=memxor@gmail.com \
--cc=netdev@vger.kernel.org \
--cc=pabeni@redhat.com \
--cc=sdf@fomichev.me \
--cc=song@kernel.org \
--cc=yonghong.song@linux.dev \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.