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.
Best regards, Ilya Maximets.
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
