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

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

[Mina Almasry]

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 <bobbyeshleman@gmail.com>
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 <almasrymina@google.com>
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

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
---
 net/core/skbuff.c          | 13 ++++++++++++-
 net/openvswitch/datapath.c |  2 ++
 2 files changed, 14 insertions(+), 1 deletion(-)

diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index ba3dbac80fb49..d21af68156950 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -3870,7 +3870,8 @@ EXPORT_SYMBOL_GPL(skb_zerocopy_headlen);
  *	Return value:
  *	0: everything is OK
  *	-ENOMEM: couldn't orphan frags of @from due to lack of memory
- *	-EFAULT: skb_copy_bits() found some problem with skb geometry
+ *	-EFAULT: skb_copy_bits() found some problem with skb geometry, or readable head
+ *      payload would be mixed with unreadable frags.
  */
 int
 skb_zerocopy(struct sk_buff *to, struct sk_buff *from, int len, int hlen)
@@ -3905,10 +3906,17 @@ skb_zerocopy(struct sk_buff *to, struct sk_buff *from, int len, int hlen)
 		}
 	}
 
+	if (!skb_frags_readable(from) && j > 0 && len) {
+		put_page(virt_to_head_page(from->head));
+		return -EFAULT;
+	}
+
 	skb_len_add(to, len + plen);
 
 	if (unlikely(skb_orphan_frags(from, GFP_ATOMIC))) {
 		skb_tx_error(from);
+		if (j > 0)
+			put_page(virt_to_head_page(from->head));
 		return -ENOMEM;
 	}
 	skb_zerocopy_clone(to, from, GFP_ATOMIC);
@@ -3928,6 +3936,9 @@ skb_zerocopy(struct sk_buff *to, struct sk_buff *from, int len, int hlen)
 	}
 	skb_shinfo(to)->nr_frags = j;
 
+	if (i > 0 && from->unreadable)
+		to->unreadable = 1;
+
 	return 0;
 }
 EXPORT_SYMBOL_GPL(skb_zerocopy);
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));
 	if (nla_attr_size(skb_len) > USHRT_MAX) {
 		err = -EFBIG;
 		goto out;

base-commit: dd057113ac7ba5bdd2aed3d9405305911152f911
-- 
2.55.0.679.g6767b8d81c-goog

[PATCH net v2 2/2] net: tcp: block standard payload injection into devmem skbs

[Mina Almasry]

Protect tcp_sendmsg_locked() from mistakenly appending non-zerocopy
page fragments to unreadable devmem skbs. Create a new segment instead.

Fixes: bd61848900bff ("net: devmem: Implement TX path")
Cc: Pavel Begunkov <asml.silence@gmail.com>
Cc: Stanislav Fomichev <sdf@fomichev.me>
Cc: Bobby Eshleman <bobbyeshleman@gmail.com>
Signed-off-by: Mina Almasry <almasrymina@google.com>
Reviewed-by: Pavel Begunkov <asml.silence@gmail.com>

---
v2: No changes.
v1: https://lore.kernel.org/r/20260801125308.1342897-3-almasrymina@google.com
---
 net/ipv4/tcp.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
index 455441f1b6949..186a36c698798 100644
--- a/net/ipv4/tcp.c
+++ b/net/ipv4/tcp.c
@@ -1278,6 +1278,11 @@ int tcp_sendmsg_locked(struct sock *sk, struct msghdr *msg, size_t size)
 		if (copy > msg_data_left(msg))
 			copy = msg_data_left(msg);
 
+		if (zc != MSG_ZEROCOPY && unlikely(!skb_frags_readable(skb))) {
+			tcp_mark_push(tp, skb);
+			goto new_segment;
+		}
+
 		if (zc == 0) {
 			bool merge = true;
 			int i = skb_shinfo(skb)->nr_frags;
-- 
2.55.0.679.g6767b8d81c-goog

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

[Ilya Maximets]

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@gmail.com>
> Cc: Stanislav Fomichev <sdf@fomichev.me>
> Cc: Bobby Eshleman <bobbyeshleman@gmail.com>
> 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 <almasrymina@google.com>
> 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
> 
> 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.

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@ovn.org> 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@gmail.com>
> > Cc: Stanislav Fomichev <sdf@fomichev.me>
> > Cc: Bobby Eshleman <bobbyeshleman@gmail.com>
> > 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 <almasrymina@google.com>
> > 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
> >
> > 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