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

[Mina Almasry]

On Mon, Aug 10, 2026 at 11:55 AM Ilya Maximets <i.maximets@xxxxxxx> 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 <asml.silence@xxxxxxxxx>
> > Cc: Stanislav Fomichev <sdf@xxxxxxxxxxx>
> > Cc: Bobby Eshleman <bobbyeshleman@xxxxxxxxx>
> > Cc: Florian Westphal <fw@xxxxxxxxx>
> > Cc: Aaron Conole <aconole@xxxxxxxxxx>
> > Cc: Eelco Chaudron <echaudro@xxxxxxxxxx>
> > Cc: Ilya Maximets <i.maximets@xxxxxxx>
> > Signed-off-by: Mina Almasry <almasrymina@xxxxxxxxxx>
> > Reviewed-by: Pavel Begunkov <asml.silence@xxxxxxxxx>
> >
> > ---
> > 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@xxxxxxxxxx
> >
> > 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