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.deHmm. 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