On Mon, Aug 10, 2026 at 11:55 AM Ilya Maximets <[email protected]> wrote: > > On 8/10/26 8:09 PM, Mina Almasry wrote: > > 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 <[email protected]> > > Cc: Stanislav Fomichev <[email protected]> > > Cc: Bobby Eshleman <[email protected]> > > Cc: Florian Westphal <[email protected]> > > Cc: Aaron Conole <[email protected]> > > Cc: Eelco Chaudron <[email protected]> > > Cc: Ilya Maximets <[email protected]> > > Signed-off-by: Mina Almasry <[email protected]> > > Reviewed-by: Pavel Begunkov <[email protected]> > > > > --- > > 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/[email protected] > > > > 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. > > > 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 _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
