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

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

[Mina Almasry]

skb_zerocopy() fails to propagate the unreadable flag when copying
unreadable fragments, causing target skbs to appear as readable memory.

This patch fixes the flag propagation. Additionally, it returns -EFAULT
if readable fragments are mixed with unreadable fragments during
extraction, and returns -EFAULT in openvswitch queue_userspace_packet().

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>
Cc: Willem de Bruijn <willemb@google.com>
Signed-off-by: Mina Almasry <almasrymina@google.com>
Reviewed-by: Pavel Begunkov <asml.silence@gmail.com>
Reviewed-by: Ilya Maximets <i.maximets@ovn.org>
---
v4:
- Changed commit references from payload types to readable/unreadable
  fragments.
---
 net/core/skbuff.c          | 13 ++++++++++++-
 net/openvswitch/datapath.c |  3 +++
 2 files changed, 15 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..482893a5f67dc 100644
--- a/net/openvswitch/datapath.c
+++ b/net/openvswitch/datapath.c
@@ -467,6 +467,9 @@ static int queue_userspace_packet(struct datapath *dp, struct sk_buff *skb,
 	if (!dp_ifindex)
 		return -ENODEV;
 
+	if (!skb_frags_readable(skb))
+		return -EFAULT;
+
 	if (skb_vlan_tag_present(skb)) {
 		nskb = skb_clone(skb, GFP_ATOMIC);
 		if (!nskb)

base-commit: 24ef02f934eeb48830cff6b739abc3c62b1d107b
-- 
2.55.0.691.gc56d675ccc-goog

[PATCH net v4 2/2] net: tcp: block mixing readable and unreadable frags

[Mina Almasry]

Protect tcp_sendmsg_locked() from mistakenly mixing readable and
unreadable page fragments in the same SKB.

Check that the devmem binding matches the existing SKB's readability.
If a mismatch is detected, avoid collapsing and create a new segment.

Fixes: bd61848900bff ("net: devmem: Implement TX path")
Suggested-by: Eric Dumazet <edumazet@google.com>
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>
---
v4:
- Adopt Eric's suggestion to check 'binding' against SKB readability
  alongside tcp_skb_can_collapse_to().
- Drop pre-existing Reviewed-by tags due to significant implementation changes.
v3: https://lore.kernel.org/r/20260811195405.3979177-2-almasrymina@google.com
v2: https://lore.kernel.org/r/20260810180956.2348280-2-almasrymina@google.com
v1: https://lore.kernel.org/r/20260801125308.1342897-3-almasrymina@google.com
---
 net/ipv4/tcp.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
index 455441f1b6949..b4237d0e994d6 100644
--- a/net/ipv4/tcp.c
+++ b/net/ipv4/tcp.c
@@ -1240,7 +1240,8 @@ int tcp_sendmsg_locked(struct sock *sk, struct msghdr *msg, size_t size)
 
 		trace_tcp_sendmsg_locked(sk, msg, skb, size_goal);
 
-		if (copy <= 0 || !tcp_skb_can_collapse_to(skb)) {
+		if (copy <= 0 || !tcp_skb_can_collapse_to(skb) ||
+		    unlikely(skb_frags_readable(skb) != !binding)) {
 			bool first_skb;
 
 new_segment:
-- 
2.55.0.691.gc56d675ccc-goog

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

[Paolo Abeni]

This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
Full review at: https://netdev-ai.bots.linux.dev/sashiko/
---
net: core: propagate unreadable flag in skb_zerocopy

The patch makes skb_zerocopy() propagate the unreadable flag to the
destination skb, returns -EFAULT when a readable head payload would be
mixed with unreadable frags, and rejects unreadable skbs in openvswitch
queue_userspace_packet().

> 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

[ ... ]

> @@ -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;
>  	}

[Severity: Medium]
On this -ENOMEM path, is it safe to re-derive the head page from
from->head after skb_orphan_frags() has already run?

skb_orphan_frags() -> skb_copy_ubufs() drops the clone first:

net/core/skbuff.c:skb_copy_ubufs() {
	...
	if (skb_shared(skb) || skb_unclone(skb, gfp_mask))
		return -EINVAL;
	...
		page = alloc_pages(gfp_mask | __GFP_COMP, order);
		if (!page) {
			...
			return -ENOMEM;
		}
}

and skb_unclone() -> pskb_expand_head() replaces the buffer:

net/core/skbuff.c:pskb_expand_head() {
	...
	skb->head     = data;
	skb->head_frag = 0;
	...
}

So when the -ENOMEM comes from the alloc_pages() loop, from->head can
already point at a different (freshly allocated) buffer than the page that
was get_page()'d earlier in the hlen == 0 branch:

	page = virt_to_head_page(from->head);
	offset = from->data - (unsigned char *)page_address(page);
	__skb_fill_netmem_desc(to, 0, page_to_netmem(page), offset, plen);
	get_page(page);
	j = 1;

Does that leak the reference on the original head-fragment page, and apply
the put_page() to an unrelated buffer? Today the misdirected put_page() is
quiet because pskb_expand_head() installs slab/kmalloc memory and put_page()
early-returns for folio_test_slab()/folio_test_large_kmalloc() folios, but
the reference on the page that was actually taken is never returned.

Would it be more robust to keep the page pointer captured before get_page()
in a local and use put_page(page) in both new error paths?

I was not able to find an in-tree producer that combines head_frag == 1 with
skb_zcopy() set, so this path may not be reachable today.

[Severity: Low]
This hunk looks like it also fixes an older, unrelated head-page reference
leak. __skb_fill_netmem_desc() does not bump skb_shinfo(to)->nr_frags:

include/linux/skbuff.h:__skb_fill_netmem_desc() {
	__skb_fill_netmem_desc_noacc(skb_shinfo(skb), i, netmem, off, size);
	...
}

and skb_zerocopy() only sets it at the very end with
skb_shinfo(to)->nr_frags = j;. So before this patch, returning -ENOMEM here
left @to with nr_frags == 0 and the extra head-page reference was never
dropped when the caller freed it (consume_skb(user_skb) in
queue_userspace_packet(), kfree_skb() in nfqnl_build_packet_message()).

That leak dates back to 36d5fe6a0007 ("core, nfqueue, openvswitch: Orphan
frags in skb_zerocopy and handle errors") and is unrelated to unreadable
frags, while the changelog only mentions unreadable-flag propagation and the
new -EFAULT returns, and the Fixes: tag points at 65249feb6b3d.

Could this hunk be split into its own patch with Fixes: 36d5fe6a0007, or at
least described in the commit message, so that stable backports reach the
pre-6.12 kernels that also contain the leak?

> +	if (i > 0 && from->unreadable)
> +		to->unreadable = 1;
> +

[ ... ]
-- 
This is an AI-generated review.

Re: [PATCH net v4 2/2] net: tcp: block mixing readable and unreadable frags

[Paolo Abeni]

This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
Full review at: https://netdev-ai.bots.linux.dev/sashiko/
---
net: tcp: block mixing readable and unreadable frags

This adds a check in tcp_sendmsg_locked() so an existing tail SKB is not
collapsed into when its readability does not match the presence of a
devmem binding, forcing a new segment instead.

> diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
> index 455441f1b6949..b4237d0e994d6 100644
> --- a/net/ipv4/tcp.c
> +++ b/net/ipv4/tcp.c
> @@ -1240,7 +1240,8 @@ int tcp_sendmsg_locked(struct sock *sk, struct msghdr *msg, size_t size)
>  
>  		trace_tcp_sendmsg_locked(sk, msg, skb, size_goal);
>  
> -		if (copy <= 0 || !tcp_skb_can_collapse_to(skb)) {
> +		if (copy <= 0 || !tcp_skb_can_collapse_to(skb) ||
> +		    unlikely(skb_frags_readable(skb) != !binding)) {
>  			bool first_skb;
>  
>  new_segment:

[Severity: Medium]
Is binding alone the right proxy here for "this send will append
unreadable frags"?

What actually selects the devmem filler is the zc dispatch in the loop
plus __zerocopy_sg_from_iter():

net/core/datagram.c:__zerocopy_sg_from_iter() {
	...
	else if (binding)
		ret = zerocopy_fill_skb_from_devmem(skb, from, length, binding);
	else
		ret = zerocopy_fill_skb_from_iter(skb, from, length);
}

and that is only reached from the zc == MSG_ZEROCOPY arm of
tcp_sendmsg_locked().

Can binding != NULL coexist with zc == 0?  In tcp_sendmsg_locked() the
binding is taken whenever the cmsg carries dmabuf_id:

		if (sk->sk_route_caps & NETIF_F_SG)
			zc = MSG_ZEROCOPY;
		else
			uarg_to_msgzc(uarg)->zerocopy = 0;

		if (!sockc_err && sockc.dmabuf_id) {
			binding = net_devmem_get_binding(sk, sockc.dmabuf_id);

while zc is only set under NETIF_F_SG.  The one dmabuf_id validation
does not look at SG at all:

	if (!sockc_err && sockc.dmabuf_id &&
	    (!(flags & MSG_ZEROCOPY) || !sock_flag(sk, SOCK_ZEROCOPY))) {
		err = -EINVAL;

And sk_setup_caps() in net/core/sock.c only force-adds NETIF_F_SG when
sk_can_gso() holds:

	if (unlikely(sk->sk_gso_disabled))
		sk->sk_route_caps &= ~NETIF_F_GSO_MASK;
	if (sk_can_gso(sk)) {
		if (dst->header_len && !xfrm_dst_offload_ok(dst)) {
			sk->sk_route_caps &= ~NETIF_F_GSO_MASK;
		} else {
			sk->sk_route_caps |= NETIF_F_SG | NETIF_F_HW_CSUM;

so a TCP-MD5/TCP-AO socket (sk_gso_disable()) or an IPsec dst on a device
with sg turned off ends up with binding != NULL and zc == 0.

In that state, does the new predicate go wrong in both directions?

If the tail is readable or newly allocated, skb_frags_readable(skb) !=
!binding evaluates to true != false, so every iteration takes
goto new_segment and the write is split into one skb per iteration, even
though the zc == 0 path only appends readable page frags via
skb_fill_page_desc().

If the tail is already unreadable from an earlier devmem send, the test
is false != false, collapsing is allowed, and skb_fill_page_desc() adds a
readable page frag to an skb that __skb_fill_netmem_desc() already marked
unreadable = 1 — the mixing this patch aims to prevent.  Would
validate_xmit_unreadable_skb() catch that?  It only looks at the first
frag:

net/core/dev.c:validate_xmit_unreadable_skb() {
	if (shinfo->nr_frags > 0) {
		niov = netmem_to_net_iov(skb_frag_netmem(&shinfo->frags[0]));

Would keying the new clause on binding && zc == MSG_ZEROCOPY, or
rejecting dmabuf_id when the route cannot do SG, line the predicate up
with the code that actually fills the frags?

Note the mixing described in the second direction pre-dates this patch
(there was no check at all before); the incorrect predicate and the
forced re-segmentation are new.  Reaching this also needs a local
privileged setup (dmabuf TX binding plus MD5/AO or IPsec plus an ethtool
feature change), and in that configuration a devmem send through the copy
path would likely already fail in skb_copy_to_page_nocache() since the
iovec holds dmabuf offsets rather than user addresses, so this is stated
as an observation rather than a demonstrated corruption path.
-- 
This is an AI-generated review.

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

[Mina Almasry]

On Tue, Aug 18, 2026 at 3:26 AM Paolo Abeni <pabeni@redhat.com> wrote:
>
> This is an AI-generated review of your patch. The human sending this
> email has considered the AI review valid, or at least plausible.
> Full review at: https://netdev-ai.bots.linux.dev/sashiko/

Thanks Paolo. Responding to the specific feedback inline, but in
general, clashiko is pointing out pre-existing issues. They should be
fixed absolutely but I'd prefer to submit separate fixes for them. I'm
a bit worried about going down this rabbit hole forever. Let me know
what you think.

> ---
> net: core: propagate unreadable flag in skb_zerocopy
>
> The patch makes skb_zerocopy() propagate the unreadable flag to the
> destination skb, returns -EFAULT when a readable head payload would be
> mixed with unreadable frags, and rejects unreadable skbs in openvswitch
> queue_userspace_packet().
>
> > 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
>
> [ ... ]
>
> > @@ -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;
> >       }
>
> [Severity: Medium]
> On this -ENOMEM path, is it safe to re-derive the head page from
> from->head after skb_orphan_frags() has already run?
>

Probably not. I think this is a pre-existing issue in skb_orphan_frags
though. I think skb_orphan_frags should check !skb_frags_readable()
before doing anything so that it doens't modify the skb and then
return -EFAULT on a modified skb.

> skb_orphan_frags() -> skb_copy_ubufs() drops the clone first:
>
> net/core/skbuff.c:skb_copy_ubufs() {
>         ...
>         if (skb_shared(skb) || skb_unclone(skb, gfp_mask))
>                 return -EINVAL;
>         ...
>                 page = alloc_pages(gfp_mask | __GFP_COMP, order);
>                 if (!page) {
>                         ...
>                         return -ENOMEM;
>                 }
> }
>
> and skb_unclone() -> pskb_expand_head() replaces the buffer:
>
> net/core/skbuff.c:pskb_expand_head() {
>         ...
>         skb->head     = data;
>         skb->head_frag = 0;
>         ...
> }
>
> So when the -ENOMEM comes from the alloc_pages() loop, from->head can
> already point at a different (freshly allocated) buffer than the page that
> was get_page()'d earlier in the hlen == 0 branch:
>
>         page = virt_to_head_page(from->head);
>         offset = from->data - (unsigned char *)page_address(page);
>         __skb_fill_netmem_desc(to, 0, page_to_netmem(page), offset, plen);
>         get_page(page);
>         j = 1;
>
> Does that leak the reference on the original head-fragment page, and apply
> the put_page() to an unrelated buffer? Today the misdirected put_page() is
> quiet because pskb_expand_head() installs slab/kmalloc memory and put_page()
> early-returns for folio_test_slab()/folio_test_large_kmalloc() folios, but
> the reference on the page that was actually taken is never returned.
>
> Would it be more robust to keep the page pointer captured before get_page()
> in a local and use put_page(page) in both new error paths?
>
> I was not able to find an in-tree producer that combines head_frag == 1 with
> skb_zcopy() set, so this path may not be reachable today.
>
> [Severity: Low]
> This hunk looks like it also fixes an older, unrelated head-page reference
> leak. __skb_fill_netmem_desc() does not bump skb_shinfo(to)->nr_frags:
>
> include/linux/skbuff.h:__skb_fill_netmem_desc() {
>         __skb_fill_netmem_desc_noacc(skb_shinfo(skb), i, netmem, off, size);
>         ...
> }
>
> and skb_zerocopy() only sets it at the very end with
> skb_shinfo(to)->nr_frags = j;. So before this patch, returning -ENOMEM here
> left @to with nr_frags == 0 and the extra head-page reference was never
> dropped when the caller freed it (consume_skb(user_skb) in
> queue_userspace_packet(), kfree_skb() in nfqnl_build_packet_message()).
>
> That leak dates back to 36d5fe6a0007 ("core, nfqueue, openvswitch: Orphan
> frags in skb_zerocopy and handle errors") and is unrelated to unreadable
> frags, while the changelog only mentions unreadable-flag propagation and the
> new -EFAULT returns, and the Fixes: tag points at 65249feb6b3d.
>
> Could this hunk be split into its own patch with Fixes: 36d5fe6a0007, or at
> least described in the commit message, so that stable backports reach the
> pre-6.12 kernels that also contain the leak?
>

Right, I added this hunk because sashkio reported this pre-existing
issue in a previous iteration. I could fork it into its own fix, but
for the follow up pre-existing issues it's reporting now, it maybe
makes sense to also fix those in follow up fixes.

--
Thanks,
Mina

Re: [PATCH net v4 2/2] net: tcp: block mixing readable and unreadable frags

[Mina Almasry]

On Tue, Aug 18, 2026 at 3:26 AM Paolo Abeni <pabeni@redhat.com> wrote:
>
> This is an AI-generated review of your patch. The human sending this
> email has considered the AI review valid, or at least plausible.
> Full review at: https://netdev-ai.bots.linux.dev/sashiko/
> ---
> net: tcp: block mixing readable and unreadable frags
>
> This adds a check in tcp_sendmsg_locked() so an existing tail SKB is not
> collapsed into when its readability does not match the presence of a
> devmem binding, forcing a new segment instead.
>
> > diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
> > index 455441f1b6949..b4237d0e994d6 100644
> > --- a/net/ipv4/tcp.c
> > +++ b/net/ipv4/tcp.c
> > @@ -1240,7 +1240,8 @@ int tcp_sendmsg_locked(struct sock *sk, struct msghdr *msg, size_t size)
> >
> >               trace_tcp_sendmsg_locked(sk, msg, skb, size_goal);
> >
> > -             if (copy <= 0 || !tcp_skb_can_collapse_to(skb)) {
> > +             if (copy <= 0 || !tcp_skb_can_collapse_to(skb) ||
> > +                 unlikely(skb_frags_readable(skb) != !binding)) {
> >                       bool first_skb;
> >
> >  new_segment:
>
> [Severity: Medium]
> Is binding alone the right proxy here for "this send will append
> unreadable frags"?
>
> What actually selects the devmem filler is the zc dispatch in the loop
> plus __zerocopy_sg_from_iter():
>
> net/core/datagram.c:__zerocopy_sg_from_iter() {
>         ...
>         else if (binding)
>                 ret = zerocopy_fill_skb_from_devmem(skb, from, length, binding);
>         else
>                 ret = zerocopy_fill_skb_from_iter(skb, from, length);
> }
>
> and that is only reached from the zc == MSG_ZEROCOPY arm of
> tcp_sendmsg_locked().
>
> Can binding != NULL coexist with zc == 0?  In tcp_sendmsg_locked() the
> binding is taken whenever the cmsg carries dmabuf_id:
>
>                 if (sk->sk_route_caps & NETIF_F_SG)
>                         zc = MSG_ZEROCOPY;
>                 else
>                         uarg_to_msgzc(uarg)->zerocopy = 0;
>
>                 if (!sockc_err && sockc.dmabuf_id) {
>                         binding = net_devmem_get_binding(sk, sockc.dmabuf_id);
>
> while zc is only set under NETIF_F_SG.  The one dmabuf_id validation
> does not look at SG at all:
>
>         if (!sockc_err && sockc.dmabuf_id &&
>             (!(flags & MSG_ZEROCOPY) || !sock_flag(sk, SOCK_ZEROCOPY))) {
>                 err = -EINVAL;
>
> And sk_setup_caps() in net/core/sock.c only force-adds NETIF_F_SG when
> sk_can_gso() holds:
>
>         if (unlikely(sk->sk_gso_disabled))
>                 sk->sk_route_caps &= ~NETIF_F_GSO_MASK;
>         if (sk_can_gso(sk)) {
>                 if (dst->header_len && !xfrm_dst_offload_ok(dst)) {
>                         sk->sk_route_caps &= ~NETIF_F_GSO_MASK;
>                 } else {
>                         sk->sk_route_caps |= NETIF_F_SG | NETIF_F_HW_CSUM;
>
> so a TCP-MD5/TCP-AO socket (sk_gso_disable()) or an IPsec dst on a device
> with sg turned off ends up with binding != NULL and zc == 0.
>
> In that state, does the new predicate go wrong in both directions?
>
> If the tail is readable or newly allocated, skb_frags_readable(skb) !=
> !binding evaluates to true != false, so every iteration takes
> goto new_segment and the write is split into one skb per iteration, even
> though the zc == 0 path only appends readable page frags via
> skb_fill_page_desc().
>
> If the tail is already unreadable from an earlier devmem send, the test
> is false != false, collapsing is allowed, and skb_fill_page_desc() adds a
> readable page frag to an skb that __skb_fill_netmem_desc() already marked
> unreadable = 1 — the mixing this patch aims to prevent.  Would
> validate_xmit_unreadable_skb() catch that?  It only looks at the first
> frag:
>
> net/core/dev.c:validate_xmit_unreadable_skb() {
>         if (shinfo->nr_frags > 0) {
>                 niov = netmem_to_net_iov(skb_frag_netmem(&shinfo->frags[0]));
>
> Would keying the new clause on binding && zc == MSG_ZEROCOPY, or
> rejecting dmabuf_id when the route cannot do SG, line the predicate up
> with the code that actually fills the frags?
>
> Note the mixing described in the second direction pre-dates this patch
> (there was no check at all before); the incorrect predicate and the
> forced re-segmentation are new.  Reaching this also needs a local
> privileged setup (dmabuf TX binding plus MD5/AO or IPsec plus an ethtool
> feature change), and in that configuration a devmem send through the copy
> path would likely already fail in skb_copy_to_page_nocache() since the
> iovec holds dmabuf offsets rather than user addresses, so this is stated
> as an observation rather than a demonstrated corruption path.

Pre-existing issue I think. We indeed assume binding != NULL means the
send is devmem and zc is set. We missed that we may not set zc if
NETIF_F_SG is not enabled. Probably the code setting binding to
non-NULL should check if zc is set. I think this is best reserved for
its own follow up fix, but let me know if not Paolo.

--
Thanks,
Mina