Thread (4 messages) flat view 4 messages, 2 authors, 4d ago

Re: [PATCH net v2 1/2] net: core: propagate unreadable flag in skb_zerocopy

From: Mina Almasry <hidden>
Date: 2026-08-10 19:39:49
Also in: lkml

On Mon, Aug 10, 2026 at 11:55 AM Ilya Maximets [off-list ref] wrote:
On 8/10/26 8:09 PM, Mina Almasry wrote:
quoted
skb_zerocopy() fails to propagate the unreadable flag when copying
devmem fragments, causing target skbs to appear as readable memory.

This patch fixes the flag propagation. Additionally, it returns -EFAULT
if standard payload is mixed with unreadable devmem fragments during
extraction, and clamps unreadable skb lengths in openvswitch
queue_userspace_packet() to avert truncated invalid payloads.

Fixes: 65249feb6b3d ("net: add support for skbs with unreadable frags")
Cc: Pavel Begunkov <asml.silence@gmail.com>
Cc: Stanislav Fomichev <sdf@fomichev.me>
Cc: Bobby Eshleman <redacted>
Cc: Florian Westphal <fw@strlen.de>
Cc: Aaron Conole <aconole@redhat.com>
Cc: Eelco Chaudron <echaudro@redhat.com>
Cc: Ilya Maximets <i.maximets@ovn.org>
Signed-off-by: Mina Almasry <redacted>
Reviewed-by: Pavel Begunkov <asml.silence@gmail.com>

---
v2:
- Return -EFAULT when mixing unreadable and readable frags (Pavel).
- Clamp unreadable skb lengths for openvswitch queue drops (sashiko).
v1: https://lore.kernel.org/r/20260801125308.1342897-1-almasrymina@google.com (local)

Openvswitch maintainers: PTAL at the openvswitch changes closely. They
are reported by sashiko as an also-need part of this fix:
https://sashiko.dev/#/patchset/20260706155219.23757-1-fw%40strlen.de
Hmm.  FWIW, I do not see anything about openvswitch at that page.
quoted
diff --git a/net/openvswitch/datapath.c b/net/openvswitch/datapath.c
index ae69b2cabab9e..7c663d7846174 100644
--- a/net/openvswitch/datapath.c
+++ b/net/openvswitch/datapath.c
@@ -480,6 +480,8 @@ static int queue_userspace_packet(struct datapath *dp, struct sk_buff *skb,
      }

      skb_len = min(skb->len, cutlen);
+     if (!skb_frags_readable(skb))
+             skb_len = min_t(size_t, skb_len, skb_headlen(skb));
I'm not very familiar with the devmem and the unreadable frags, but if
there is really no way to read 'skb_len' bytes of the packet, it must not
be delivered to userspace.  Delivering truncated packet will confuse
ovs-vswitchd and the packet will be dropped or delivered truncated to the
destination.  We should return something like -EFAULT here and the caller
will drop the packet (MISS upcall) or continue processing if the failure
is not fatal (ACTION upcall).

This practically makes devmem incompatible with OVS, I suppose, as upcalls
are the primary mechanism for initial packet processing, before the datapath
flows are installed.

If there is a way to read this memory, we should make a full copy here.
There is indeed 'no way' to read this memory. That's the short version
at least. The long version is that they can read it on the GPU but
this requires the userspace/caller to understand it's in the GPU.

OK, sounds like the right thing to do is return -EFAULT here. In the
future someone may see value in making openvswitch unreadable skb
compatible I guess.

I'll rev after 24hrs, and see if there is any other feedback.


-- 
Thanks,
Mina
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help