[PATCH net v1] net: devmem: prevent mixing fragments from different bindings

[PATCH net v1] net: devmem: prevent mixing fragments from different bindings

[Mina Almasry]

validate_xmit_unreadable_skb() only inspects shinfo->frags[0] and
assumes all fragments in an unreadable skb belong to that same devmem
binding. However, tcp_sendmsg_locked() only checks that readability
matches the presence of a binding (skb_frags_readable(skb) != !binding),
allowing consecutive sendmsg() calls with different dmabuf bindings to
collapse into the same skb and bypass per-device and unbind checks in
validate_xmit_unreadable_skb().

Add net_devmem_skb_binding() to query the binding associated with an
skb, reuse it in validate_xmit_unreadable_skb(), and check in
zerocopy_fill_skb_from_devmem() that existing fragments match the target
binding.

Fixes: bd61848900bff ("net: devmem: Implement TX path")
Cc: Pavel Begunkov <asml.silence@gmail.com>
Cc: Stanislav Fomichev <sdf@fomichev.me>
Cc: Bobby Eshleman <bobbyeshleman@meta.com>
Signed-off-by: Mina Almasry <almasrymina@google.com>
---
 net/core/datagram.c |  2 +-
 net/core/dev.c      | 14 ++++----------
 net/core/devmem.h   | 23 +++++++++++++++++++++++
 3 files changed, 28 insertions(+), 11 deletions(-)

diff --git a/net/core/datagram.c b/net/core/datagram.c
index 173b5d97bd409..ed8f1045f3cca 100644
--- a/net/core/datagram.c
+++ b/net/core/datagram.c
@@ -712,7 +712,7 @@ zerocopy_fill_skb_from_devmem(struct sk_buff *skb, struct iov_iter *from,
 	size_t virt_addr, size, off;
 	struct net_iov *niov;
 
-	if (i && skb_frags_readable(skb))
+	if (i && net_devmem_skb_binding(skb) != binding)
 		return -EFAULT;
 
 	/* Devmem filling works by taking an IOVEC from the user where the
diff --git a/net/core/dev.c b/net/core/dev.c
index e76762e29360e..ad2b587dfee27 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -4054,8 +4054,7 @@ static struct sk_buff *sk_validate_xmit_skb(struct sk_buff *skb,
 static struct sk_buff *validate_xmit_unreadable_skb(struct sk_buff *skb,
 						    struct net_device *dev)
 {
-	struct skb_shared_info *shinfo;
-	struct net_iov *niov;
+	struct net_devmem_dmabuf_binding *binding;
 
 	if (likely(skb_frags_readable(skb) ||
 		   dev->netmem_tx == NETMEM_TX_NO_DMA))
@@ -4064,14 +4063,9 @@ static struct sk_buff *validate_xmit_unreadable_skb(struct sk_buff *skb,
 	if (dev->netmem_tx == NETMEM_TX_NONE)
 		goto out_free;
 
-	shinfo = skb_shinfo(skb);
-
-	if (shinfo->nr_frags > 0) {
-		niov = netmem_to_net_iov(skb_frag_netmem(&shinfo->frags[0]));
-		if (net_is_devmem_iov(niov) &&
-		    READ_ONCE(net_devmem_iov_binding(niov)->dev) != dev)
-			goto out_free;
-	}
+	binding = net_devmem_skb_binding(skb);
+	if (binding && READ_ONCE(binding->dev) != dev)
+		goto out_free;
 
 out:
 	return skb;
diff --git a/net/core/devmem.h b/net/core/devmem.h
index 4a293a7d1149c..8c74037633ae8 100644
--- a/net/core/devmem.h
+++ b/net/core/devmem.h
@@ -10,6 +10,7 @@
 #ifndef _NET_DEVMEM_H
 #define _NET_DEVMEM_H
 
+#include <linux/skbuff.h>
 #include <net/netmem.h>
 #include <net/netdev_netlink.h>
 
@@ -118,6 +119,22 @@ net_devmem_iov_binding(const struct net_iov *niov)
 	return net_devmem_iov_to_chunk_owner(niov)->binding;
 }
 
+static inline struct net_devmem_dmabuf_binding *
+net_devmem_skb_binding(const struct sk_buff *skb)
+{
+	const struct skb_shared_info *shinfo = skb_shinfo(skb);
+	const struct net_iov *niov;
+
+	if (skb_frags_readable(skb) || !shinfo->nr_frags)
+		return NULL;
+
+	niov = skb_frag_net_iov(&shinfo->frags[0]);
+	if (!niov || !net_is_devmem_iov(niov))
+		return NULL;
+
+	return net_devmem_iov_binding(niov);
+}
+
 static inline u32 net_devmem_iov_binding_id(const struct net_iov *niov)
 {
 	return net_devmem_iov_binding(niov)->id;
@@ -243,6 +260,12 @@ net_devmem_iov_binding(const struct net_iov *niov)
 {
 	return NULL;
 }
+
+static inline struct net_devmem_dmabuf_binding *
+net_devmem_skb_binding(const struct sk_buff *skb)
+{
+	return NULL;
+}
 #endif
 
 #endif /* _NET_DEVMEM_H */

base-commit: 6d25ffca055a77787c21a36b66c253f76239411b
-- 
2.56.0.385.gd3acb90ef8-goog

Re: [PATCH net v1] net: devmem: prevent mixing fragments from different bindings

[netdev-bot+sinfo]

Hi!

This is an automated message. This series looks like a fix, but its
commit messages seem to be missing some information:

 - How the issue was discovered, e.g. hit in production, hit during
   development, syzbot report, manual code inspection, LLM or static
   analysis tool scan.

 - Whether the issue was actually triggered, or is only theoretical
   (e.g. found by code inspection). If it was triggered please include
   the symptoms, like the stack trace or error messages.

Please do not repost the series just to address the above. Instead,
reply to this email with the missing information, so that reviewers
can take it into account. If the series needs another revision for
other reasons, please include the information in the commit messages
then.

The evaluation is done by an LLM so it may be wrong, if you think
that is the case please reply and explain.

Re: [PATCH net v1] net: devmem: prevent mixing fragments from different bindings

[Mina Almasry]

On Wed, Oct 7, 2026 at 8:45 PM <netdev-bot+sinfo@kernel.org> wrote:
>
> Hi!
>
> This is an automated message. This series looks like a fix, but its
> commit messages seem to be missing some information:
>
>  - How the issue was discovered, e.g. hit in production, hit during
>    development, syzbot report, manual code inspection, LLM or static
>    analysis tool scan.
>
>  - Whether the issue was actually triggered, or is only theoretical
>    (e.g. found by code inspection). If it was triggered please include
>    the symptoms, like the stack trace or error messages.
>
> Please do not repost the series just to address the above. Instead,
> reply to this email with the missing information, so that reviewers
> can take it into account. If the series needs another revision for
> other reasons, please include the information in the commit messages
> then.
>
> The evaluation is done by an LLM so it may be wrong, if you think
> that is the case please reply and explain.

Sorry for the double post!

The LLM discovered the issue while I was working on something
adjacent. Manual code inspection by me confirmed the issue is real.

The issue is not actually triggered. It is theoratical.

-- 
Thanks,
Mina

Re: [PATCH net v1] net: devmem: prevent mixing fragments from different bindings

[Bobby Eshleman]

On Thu, Oct 08, 2026 at 03:39:26AM +0000, Mina Almasry wrote:
> validate_xmit_unreadable_skb() only inspects shinfo->frags[0] and
> assumes all fragments in an unreadable skb belong to that same devmem
> binding. However, tcp_sendmsg_locked() only checks that readability
> matches the presence of a binding (skb_frags_readable(skb) != !binding),
> allowing consecutive sendmsg() calls with different dmabuf bindings to
> collapse into the same skb and bypass per-device and unbind checks in
> validate_xmit_unreadable_skb().
> 
> Add net_devmem_skb_binding() to query the binding associated with an
> skb, reuse it in validate_xmit_unreadable_skb(), and check in
> zerocopy_fill_skb_from_devmem() that existing fragments match the target
> binding.
> 
> Fixes: bd61848900bff ("net: devmem: Implement TX path")
> Cc: Pavel Begunkov <asml.silence@gmail.com>
> Cc: Stanislav Fomichev <sdf@fomichev.me>
> Cc: Bobby Eshleman <bobbyeshleman@meta.com>
> Signed-off-by: Mina Almasry <almasrymina@google.com>
> ---
>  net/core/datagram.c |  2 +-
>  net/core/dev.c      | 14 ++++----------
>  net/core/devmem.h   | 23 +++++++++++++++++++++++
>  3 files changed, 28 insertions(+), 11 deletions(-)
> 
> diff --git a/net/core/datagram.c b/net/core/datagram.c
> index 173b5d97bd409..ed8f1045f3cca 100644
> --- a/net/core/datagram.c
> +++ b/net/core/datagram.c
> @@ -712,7 +712,7 @@ zerocopy_fill_skb_from_devmem(struct sk_buff *skb, struct iov_iter *from,
>  	size_t virt_addr, size, off;
>  	struct net_iov *niov;
>  
> -	if (i && skb_frags_readable(skb))
> +	if (i && net_devmem_skb_binding(skb) != binding)
>  		return -EFAULT;
>  
>  	/* Devmem filling works by taking an IOVEC from the user where the
> diff --git a/net/core/dev.c b/net/core/dev.c
> index e76762e29360e..ad2b587dfee27 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -4054,8 +4054,7 @@ static struct sk_buff *sk_validate_xmit_skb(struct sk_buff *skb,
>  static struct sk_buff *validate_xmit_unreadable_skb(struct sk_buff *skb,
>  						    struct net_device *dev)
>  {
> -	struct skb_shared_info *shinfo;
> -	struct net_iov *niov;
> +	struct net_devmem_dmabuf_binding *binding;
>  
>  	if (likely(skb_frags_readable(skb) ||
>  		   dev->netmem_tx == NETMEM_TX_NO_DMA))
> @@ -4064,14 +4063,9 @@ static struct sk_buff *validate_xmit_unreadable_skb(struct sk_buff *skb,
>  	if (dev->netmem_tx == NETMEM_TX_NONE)
>  		goto out_free;
>  
> -	shinfo = skb_shinfo(skb);
> -
> -	if (shinfo->nr_frags > 0) {
> -		niov = netmem_to_net_iov(skb_frag_netmem(&shinfo->frags[0]));
> -		if (net_is_devmem_iov(niov) &&
> -		    READ_ONCE(net_devmem_iov_binding(niov)->dev) != dev)
> -			goto out_free;
> -	}
> +	binding = net_devmem_skb_binding(skb);
> +	if (binding && READ_ONCE(binding->dev) != dev)
> +		goto out_free;
>  
>  out:
>  	return skb;
> diff --git a/net/core/devmem.h b/net/core/devmem.h
> index 4a293a7d1149c..8c74037633ae8 100644
> --- a/net/core/devmem.h
> +++ b/net/core/devmem.h
> @@ -10,6 +10,7 @@
>  #ifndef _NET_DEVMEM_H
>  #define _NET_DEVMEM_H
>  
> +#include <linux/skbuff.h>
>  #include <net/netmem.h>
>  #include <net/netdev_netlink.h>
>  
> @@ -118,6 +119,22 @@ net_devmem_iov_binding(const struct net_iov *niov)
>  	return net_devmem_iov_to_chunk_owner(niov)->binding;
>  }
>  
> +static inline struct net_devmem_dmabuf_binding *
> +net_devmem_skb_binding(const struct sk_buff *skb)
> +{
> +	const struct skb_shared_info *shinfo = skb_shinfo(skb);
> +	const struct net_iov *niov;
> +
> +	if (skb_frags_readable(skb) || !shinfo->nr_frags)
> +		return NULL;
> +
> +	niov = skb_frag_net_iov(&shinfo->frags[0]);
> +	if (!niov || !net_is_devmem_iov(niov))
> +		return NULL;
> +
> +	return net_devmem_iov_binding(niov);
> +}
> +
>  static inline u32 net_devmem_iov_binding_id(const struct net_iov *niov)
>  {
>  	return net_devmem_iov_binding(niov)->id;
> @@ -243,6 +260,12 @@ net_devmem_iov_binding(const struct net_iov *niov)
>  {
>  	return NULL;
>  }
> +
> +static inline struct net_devmem_dmabuf_binding *
> +net_devmem_skb_binding(const struct sk_buff *skb)
> +{
> +	return NULL;
> +}
>  #endif
>  
>  #endif /* _NET_DEVMEM_H */
> 
> base-commit: 6d25ffca055a77787c21a36b66c253f76239411b
> -- 
> 2.56.0.385.gd3acb90ef8-goog
> 

Makes sense to me. Thanks.

Reviewed-by: Bobby Eshleman <bobbyeshleman@meta.com>

Re: [PATCH net v1] net: devmem: prevent mixing fragments from different bindings

[Pavel Begunkov]

On 10/8/26 04:39, Mina Almasry wrote:
> validate_xmit_unreadable_skb() only inspects shinfo->frags[0] and
> assumes all fragments in an unreadable skb belong to that same devmem
> binding. However, tcp_sendmsg_locked() only checks that readability
> matches the presence of a binding (skb_frags_readable(skb) != !binding),
> allowing consecutive sendmsg() calls with different dmabuf bindings to
> collapse into the same skb and bypass per-device and unbind checks in
> validate_xmit_unreadable_skb().
> 
> Add net_devmem_skb_binding() to query the binding associated with an
> skb, reuse it in validate_xmit_unreadable_skb(), and check in
> zerocopy_fill_skb_from_devmem() that existing fragments match the target
> binding.

Looks good

Pavel Begunkov <asml.silence@gmail.com>

-- 
Pavel Begunkov

Re: [PATCH net v1] net: devmem: prevent mixing fragments from different bindings

[Pavel Begunkov]

On 10/8/26 14:03, Pavel Begunkov wrote:
> On 10/8/26 04:39, Mina Almasry wrote:
>> validate_xmit_unreadable_skb() only inspects shinfo->frags[0] and
>> assumes all fragments in an unreadable skb belong to that same devmem
>> binding. However, tcp_sendmsg_locked() only checks that readability
>> matches the presence of a binding (skb_frags_readable(skb) != !binding),
>> allowing consecutive sendmsg() calls with different dmabuf bindings to
>> collapse into the same skb and bypass per-device and unbind checks in
>> validate_xmit_unreadable_skb().
>>
>> Add net_devmem_skb_binding() to query the binding associated with an
>> skb, reuse it in validate_xmit_unreadable_skb(), and check in
>> zerocopy_fill_skb_from_devmem() that existing fragments match the target
>> binding.
> 
> Looks good
> 
> Pavel Begunkov <asml.silence@gmail.com>

Oops, should be reviewed-by

Reviewed-by: Pavel Begunkov <asml.silence@gmail.com>

-- 
Pavel Begunkov

Re: [PATCH net v1] net: devmem: prevent mixing fragments from different bindings

[Stanislav Fomichev]

On 10/08, Mina Almasry wrote:
> validate_xmit_unreadable_skb() only inspects shinfo->frags[0] and
> assumes all fragments in an unreadable skb belong to that same devmem
> binding. However, tcp_sendmsg_locked() only checks that readability
> matches the presence of a binding (skb_frags_readable(skb) != !binding),
> allowing consecutive sendmsg() calls with different dmabuf bindings to
> collapse into the same skb and bypass per-device and unbind checks in
> validate_xmit_unreadable_skb().
> 
> Add net_devmem_skb_binding() to query the binding associated with an
> skb, reuse it in validate_xmit_unreadable_skb(), and check in
> zerocopy_fill_skb_from_devmem() that existing fragments match the target
> binding.
> 
> Fixes: bd61848900bff ("net: devmem: Implement TX path")
> Cc: Pavel Begunkov <asml.silence@gmail.com>
> Cc: Stanislav Fomichev <sdf@fomichev.me>
> Cc: Bobby Eshleman <bobbyeshleman@meta.com>
> Signed-off-by: Mina Almasry <almasrymina@google.com>

Acked-by: Stanislav Fomichev <sdf@fomichev.me>

Re: [PATCH net v1] net: devmem: prevent mixing fragments from different bindings

[Mina Almasry]

On Sun, Oct 11, 2026 at 8:00 AM <netdev-bot+sashiko@kernel.org> wrote:
>
> Thank you for your contribution! Sashiko AI review found 2 potential
> issue(s) to consider.
>
> Critical: 0 · High: 2 (1) · Medium: 0 · Low: 0
>
> If you believe none of the issues are legitimate, please reply to each
> issue of Medium severity or higher and explain why the AI is wrong.
> If any of them are legitimate and you plan to respin, please let patchwork
> know by including "pw-bot: cr" as a separate line at the end of your reply
> (one such reply per series is enough).
>
> - [High] This patch makes a valid devmem MSG_ZEROCOPY sendmsg() fail with
>   -EFAULT.
> - [High] The commit says it prevents mixing fragments from different
>   devmem bindings in one skb, but it only guards the…
>
> Pre-existing issues:
> - [High] This problem wasn't introduced by this patch, but it undermines
>   the validation the patch hardens.
>

Both these issues seem valid. I could fix them in follow up patches
but it may be good to try to fix them now. Let me take a stab at it.

pw-bot: cr

-- 
Thanks,
Mina

Re: [PATCH net v1] net: devmem: prevent mixing fragments from different bindings

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 2 potential
issue(s) to consider.

Critical: 0 · High: 2 (1) · Medium: 0 · Low: 0

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [High] This patch makes a valid devmem MSG_ZEROCOPY sendmsg() fail with
  -EFAULT.
- [High] The commit says it prevents mixing fragments from different
  devmem bindings in one skb, but it only guards the…

Pre-existing issues:
- [High] This problem wasn't introduced by this patch, but it undermines
  the validation the patch hardens.

--

net: devmem: prevent mixing fragments from different bindings

This patch adds net_devmem_skb_binding(), which returns the devmem binding
of an unreadable skb's first frag. validate_xmit_unreadable_skb() now uses
it, and zerocopy_fill_skb_from_devmem() now refuses to append frags from a
different binding to an skb that already has frags. The aim is to stop
consecutive sendmsg() calls with different dmabuf bindings from sharing
one skb and getting past the per-device and unbind checks at xmit time.

> diff --git a/net/core/datagram.c b/net/core/datagram.c
> index 173b5d97bd409..ed8f1045f3cca 100644
> --- a/net/core/datagram.c
> +++ b/net/core/datagram.c
> @@ -712,7 +712,7 @@ zerocopy_fill_skb_from_devmem(struct sk_buff *skb, struct iov_iter *from,
>  	size_t virt_addr, size, off;
>  	struct net_iov *niov;
>  
> -	if (i && skb_frags_readable(skb))
> +	if (i && net_devmem_skb_binding(skb) != binding)
>  		return -EFAULT;

[Severity: High]
Can this make a valid devmem MSG_ZEROCOPY sendmsg() fail with -EFAULT?

Segment selection in tcp_sendmsg_locked() still compares only readability,
not binding identity:

net/ipv4/tcp.c:tcp_sendmsg_locked() {
    ...
		if (copy <= 0 || !tcp_skb_can_collapse_to(skb) ||
		    unlikely(skb_frags_readable(skb) != !binding)) {
    ...
}

Say the unsent tail skb holds frags from binding A, and the next sendmsg()
passes binding B through SCM_DEVMEM_DMABUF. The condition above is false,
so TCP appends to the tail skb.

On back-to-back calls, msg_zerocopy_realloc() extends and returns the same
uarg, so skb_zerocopy_iter_stream() does not return -EEXIST. We then reach
the new check with nr_frags > 0 and net_devmem_skb_binding(skb) == A != B.

The -EFAULT is not one of the errors that start a new segment:

net/ipv4/tcp.c:tcp_sendmsg_locked() {
    ...
			err = skb_zerocopy_iter_stream(sk, skb, msg, copy, uarg,
						       binding);
			if (err == -EMSGSIZE || err == -EEXIST) {
				tcp_mark_push(tp, skb);
				goto new_segment;
			}
			if (err < 0)
				goto do_error;
    ...
}

With copied == 0, userspace gets -EFAULT. Retries fail the same way until
the tail skb leaves the write queue. That can take a while if the socket
is corked, MSG_MORE is used, Nagle or TSO defers, or cwnd is the limit.

The old "i && skb_frags_readable(skb)" check could not be reached from
TCP, because the tcp_sendmsg_locked() condition above already filtered
that case. The new check can be reached.

net_devmem_get_binding() accepts any binding whose dev (or vdev) matches
the socket's dst device. So using two dmabufs bound to the same NIC on one
socket is legitimate, and it worked before this patch.

Would it be better to start a new segment when the binding changes? One
option is to add net_devmem_skb_binding(skb) != binding to the
tcp_sendmsg_locked() condition. Another is to return -EEXIST or -EMSGSIZE
here.

> diff --git a/net/core/dev.c b/net/core/dev.c
> index e76762e29360e..ad2b587dfee27 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -4054,8 +4054,7 @@ static struct sk_buff *sk_validate_xmit_skb(struct sk_buff *skb,
>  static struct sk_buff *validate_xmit_unreadable_skb(struct sk_buff *skb,
>  						    struct net_device *dev)
>  {
> -	struct skb_shared_info *shinfo;
> -	struct net_iov *niov;
> +	struct net_devmem_dmabuf_binding *binding;
>  
>  	if (likely(skb_frags_readable(skb) ||
>  		   dev->netmem_tx == NETMEM_TX_NO_DMA))

[Severity: High]
This issue was not introduced by this patch, but does this early return
let MTU probe skbs skip all of the devmem checks?

tcp_mtu_probe() allocates the probe skb as a readable skb:

net/ipv4/tcp_output.c:tcp_mtu_probe() {
    ...
	nskb = tcp_stream_alloc_skb(sk, GFP_ATOMIC, false);
    ...
}

tcp_clone_payload() then copies the devmem net_iov frags into nskb with
skb_frag_page_copy() and skb_frag_ref(). It never sets to->unreadable; it
only propagates SKBFL_SHARED_FRAG.

So skb_frags_readable(nskb) is true, and validate_xmit_unreadable_skb()
returns here, before the NETMEM_TX_NONE, binding->dev and unbind checks.

The mislabelled skb also looks readable to any code that reads the
payload. With an AF_PACKET tap on the egress device, for example,
tpacket_rcv() calls skb_copy_bits(). That passes its !skb_frags_readable()
guard and then kmaps skb_frag_page(), which is NULL for a net_iov.

packet_rcv() recvmsg, BPF socket filters and skb_checksum_help() appear to
be exposed the same way.

A separate question: skb_frag_page() is NULL for every net_iov, so can the
merge test in tcp_clone_payload() join frags from different niovs?

net/ipv4/tcp_output.c:tcp_clone_payload() {
    ...
			if (lastfrag &&
			    skb_frag_page(fragfrom) == skb_frag_page(lastfrag) &&
			    skb_frag_off(fragfrom) == skb_frag_off(lastfrag) +
						      skb_frag_size(lastfrag)) {
				skb_frag_size_add(lastfrag, todo);
    ...
}

net_devmem_get_niov_at() sets off = virt_addr % PAGE_SIZE. Take an iov at
dmabuf offset 0 with length 1000, followed by an iov at offset
8192 + 1000. That gives niov0 {off 0, len 1000} and niov2 {off 1000}.

Those two frags would be merged into one niov0 frag, and the wrong dmabuf
bytes would go on the wire.

> @@ -4064,14 +4063,9 @@ static struct sk_buff *validate_xmit_unreadable_skb(struct sk_buff *skb,
>  	if (dev->netmem_tx == NETMEM_TX_NONE)
>  		goto out_free;
>  
> -	shinfo = skb_shinfo(skb);
> -
> -	if (shinfo->nr_frags > 0) {
> -		niov = netmem_to_net_iov(skb_frag_netmem(&shinfo->frags[0]));
> -		if (net_is_devmem_iov(niov) &&
> -		    READ_ONCE(net_devmem_iov_binding(niov)->dev) != dev)
> -			goto out_free;
> -	}
> +	binding = net_devmem_skb_binding(skb);
> +	if (binding && READ_ONCE(binding->dev) != dev)
> +		goto out_free;

[Severity: High]
The commit message says this prevents mixing fragments from different
bindings. Is the tcp_sendmsg_locked() append path the only place where
that mixing can happen?

Adjacent write-queue skbs with different bindings can still be created
after this patch. This happens when the binding A skb reached size_goal,
was sent with MSG_EOR, or msg_zerocopy_realloc() allocated a new uarg
(-EEXIST).

With MTU probing enabled and cwnd >= 11, tcp_mtu_probe() checks:

net/ipv4/tcp_output.c:tcp_can_coalesce_send_queue_head() {
    ...
		if (tcp_has_tx_tstamp(skb) || !tcp_skb_can_collapse(skb, next))
			return false;
    ...
}

tcp_skb_can_collapse() compares readability but not the binding:

include/net/tcp.h:tcp_skb_can_collapse() {
	return likely(tcp_skb_can_collapse_to(to) &&
		      mptcp_skb_can_collapse(to, from) &&
		      !skb_cmp_decrypted(to, from) &&
		      skb_pure_zcopy_same(to, from) &&
		      skb_frags_readable(to) == skb_frags_readable(from));
}

tcp_clone_payload() then copies the frags of both skbs into one probe
skb. net_devmem_skb_binding() only looks at frags[0]. So even if the probe
skb were correctly marked unreadable (see above), this check would only
validate binding A's dev. Binding B's dev and unbind state would never be
checked.

The other merge sites look covered. tcp_grow_skb() and SACK shifting go
through skb_shift(), which refuses zerocopy skbs. Retransmit collapse is
blocked because tcp_can_collapse() rejects unreadable skbs.

Should tcp_skb_can_collapse() or tcp_can_coalesce_send_queue_head() also
compare net_devmem_skb_binding() for the two skbs?

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261008034027.1403188-1-almasrymina%40google.com

Re: [PATCH net v1] net: devmem: prevent mixing fragments from different bindings

[netdev-bot+sinfo]

Hi!

This is an automated message. This series looks like a fix, but its
commit messages seem to be missing some information:

 - How the issue was discovered, e.g. hit in production, hit during
   development, syzbot report, manual code inspection, LLM or static
   analysis tool scan.

 - Whether the issue was actually triggered, or is only theoretical
   (e.g. found by code inspection). If it was triggered please include
   the symptoms, like the stack trace or error messages.

Please do not repost the series just to address the above. Instead,
reply to this email with the missing information, so that reviewers
can take it into account. If the series needs another revision for
other reasons, please include the information in the commit messages
then.

The evaluation is done by an LLM so it may be wrong, if you think
that is the case please reply and explain.

[PATCH net v1] net: devmem: prevent mixing fragments from different bindings

[Mina Almasry]

validate_xmit_unreadable_skb() only inspects shinfo->frags[0] and
assumes all fragments in an unreadable skb belong to that same devmem
binding. However, tcp_sendmsg_locked() only checks that readability
matches the presence of a binding (skb_frags_readable(skb) != !binding),
allowing consecutive sendmsg() calls with different dmabuf bindings to
collapse into the same skb and bypass per-device and unbind checks in
validate_xmit_unreadable_skb().

Add net_devmem_skb_binding() to query the binding associated with an
skb, reuse it in validate_xmit_unreadable_skb(), and check in
zerocopy_fill_skb_from_devmem() that existing fragments match the target
binding.

Fixes: bd61848900bff ("net: devmem: Implement TX path")
Cc: Pavel Begunkov <asml.silence@gmail.com>
Cc: Stanislav Fomichev <sdf@fomichev.me>
Cc: Bobby Eshleman <bobbyeshleman@meta.com>
Signed-off-by: Mina Almasry <almasrymina@google.com>
---
 net/core/datagram.c |  2 +-
 net/core/dev.c      | 14 ++++----------
 net/core/devmem.h   | 23 +++++++++++++++++++++++
 3 files changed, 28 insertions(+), 11 deletions(-)

diff --git a/net/core/datagram.c b/net/core/datagram.c
index 173b5d97bd409..ed8f1045f3cca 100644
--- a/net/core/datagram.c
+++ b/net/core/datagram.c
@@ -712,7 +712,7 @@ zerocopy_fill_skb_from_devmem(struct sk_buff *skb, struct iov_iter *from,
 	size_t virt_addr, size, off;
 	struct net_iov *niov;
 
-	if (i && skb_frags_readable(skb))
+	if (i && net_devmem_skb_binding(skb) != binding)
 		return -EFAULT;
 
 	/* Devmem filling works by taking an IOVEC from the user where the
diff --git a/net/core/dev.c b/net/core/dev.c
index e76762e29360e..ad2b587dfee27 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -4054,8 +4054,7 @@ static struct sk_buff *sk_validate_xmit_skb(struct sk_buff *skb,
 static struct sk_buff *validate_xmit_unreadable_skb(struct sk_buff *skb,
 						    struct net_device *dev)
 {
-	struct skb_shared_info *shinfo;
-	struct net_iov *niov;
+	struct net_devmem_dmabuf_binding *binding;
 
 	if (likely(skb_frags_readable(skb) ||
 		   dev->netmem_tx == NETMEM_TX_NO_DMA))
@@ -4064,14 +4063,9 @@ static struct sk_buff *validate_xmit_unreadable_skb(struct sk_buff *skb,
 	if (dev->netmem_tx == NETMEM_TX_NONE)
 		goto out_free;
 
-	shinfo = skb_shinfo(skb);
-
-	if (shinfo->nr_frags > 0) {
-		niov = netmem_to_net_iov(skb_frag_netmem(&shinfo->frags[0]));
-		if (net_is_devmem_iov(niov) &&
-		    READ_ONCE(net_devmem_iov_binding(niov)->dev) != dev)
-			goto out_free;
-	}
+	binding = net_devmem_skb_binding(skb);
+	if (binding && READ_ONCE(binding->dev) != dev)
+		goto out_free;
 
 out:
 	return skb;
diff --git a/net/core/devmem.h b/net/core/devmem.h
index 4a293a7d1149c..8c74037633ae8 100644
--- a/net/core/devmem.h
+++ b/net/core/devmem.h
@@ -10,6 +10,7 @@
 #ifndef _NET_DEVMEM_H
 #define _NET_DEVMEM_H
 
+#include <linux/skbuff.h>
 #include <net/netmem.h>
 #include <net/netdev_netlink.h>
 
@@ -118,6 +119,22 @@ net_devmem_iov_binding(const struct net_iov *niov)
 	return net_devmem_iov_to_chunk_owner(niov)->binding;
 }
 
+static inline struct net_devmem_dmabuf_binding *
+net_devmem_skb_binding(const struct sk_buff *skb)
+{
+	const struct skb_shared_info *shinfo = skb_shinfo(skb);
+	const struct net_iov *niov;
+
+	if (skb_frags_readable(skb) || !shinfo->nr_frags)
+		return NULL;
+
+	niov = skb_frag_net_iov(&shinfo->frags[0]);
+	if (!niov || !net_is_devmem_iov(niov))
+		return NULL;
+
+	return net_devmem_iov_binding(niov);
+}
+
 static inline u32 net_devmem_iov_binding_id(const struct net_iov *niov)
 {
 	return net_devmem_iov_binding(niov)->id;
@@ -243,6 +260,12 @@ net_devmem_iov_binding(const struct net_iov *niov)
 {
 	return NULL;
 }
+
+static inline struct net_devmem_dmabuf_binding *
+net_devmem_skb_binding(const struct sk_buff *skb)
+{
+	return NULL;
+}
 #endif
 
 #endif /* _NET_DEVMEM_H */

base-commit: 6d25ffca055a77787c21a36b66c253f76239411b
-- 
2.56.0.385.gd3acb90ef8-goog