[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

[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

[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

[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

[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

[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