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

[Bobby Eshleman <bobbyeshleman@gmail.com>]

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>