Re: [PATCH net-next v4 3/8] net: devmem: support TX over NETMEM_TX_NO_DMA devices

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [Critical] Dangling net_device pointers in TX bindings cause a Critical Use-After-Free during socket destruction.
- [High] Unrefcounted binding->vdev and binding->dev pointers create an ABA vulnerability allowing DMA domain bypass.
- [Medium] Missing error rollback in netdev_nl_bind_tx_doit() leads to dmabuf binding leak.
--

commit 20d630c8bbbcb31ad867556dab2a588deb3faa88
Author: Bobby Eshleman <bobbyeshleman@meta.com>

    net: devmem: support TX over NETMEM_TX_NO_DMA devices
    
    When a netkit virtual device leases queues from a physical NIC, devmem
    TX bindings created on the netkit device must still result in the dmabuf
    being mapped for dma by the physical device. This patch accomplishes
    this by teaching the bind handler to search for the underlying
    DMA-capable device by looking it up via leased rx queues.

> diff --git a/net/core/devmem.c b/net/core/devmem.c
> index cde4c89bc146a..1c67e7524246c 100644
> --- a/net/core/devmem.c
> +++ b/net/core/devmem.c
> @@ -212,6 +212,7 @@ net_devmem_bind_dmabuf(struct net_device *dev,
>  	}
>  
>  	binding->dev = dev;
> +	binding->vdev = vdev;
>  	xa_init_flags(&binding->bound_rxqs, XA_FLAGS_ALLOC);

Since binding->dev and binding->vdev are stored without holding a reference
count, what happens if the underlying physical or virtual net_device is
unregistered and freed while the binding is still active?

For RX bindings, mp_dmabuf_devmem_uninstall() safely clears binding->dev
when the device unregisters. However, TX bindings have an empty bound_rxqs
list and never receive this callback.

If a user application keeps the netlink socket open after the device is
removed, and later closes it, could netdev_nl_sock_priv_destroy() access
the stale binding->dev pointer and cause a use-after-free?

[ ... ]

> @@ -397,7 +398,8 @@ struct net_devmem_dmabuf_binding *net_devmem_get_binding(struct sock *sk,
>  	 */
>  	dst_dev = dst_dev_rcu(dst);
>  	if (unlikely(!dst_dev) ||
> -	    unlikely(dst_dev != READ_ONCE(binding->dev))) {
> +	    unlikely(dst_dev != READ_ONCE(binding->dev) &&
> +		     dst_dev != READ_ONCE(binding->vdev))) {

If the devices are unregistered and the slab allocator reuses the memory
address for a newly created, unrelated net_device, could this pointer
comparison falsely pass due to aliasing (the ABA problem)?

Could this trick the kernel into authorizing the new device to transmit
using a DMABUF mapped exclusively for the original device's IOMMU domain?

>  		err = -ENODEV;
>  		goto out_unlock;
>  	}

[ ... ]

> diff --git a/net/core/netdev-genl.c b/net/core/netdev-genl.c
> index 4d2c49371cdb4..b4d48f3672a5b 100644
> --- a/net/core/netdev-genl.c
> +++ b/net/core/netdev-genl.c

[ ... ]

> @@ -1171,22 +1205,41 @@ int netdev_nl_bind_tx_doit(struct sk_buff *skb, struct genl_info *info)
>  		goto err_unlock_netdev;
>  	}
>  
> -	dma_dev = netdev_queue_get_dma_dev(netdev, 0, NETDEV_QUEUE_TYPE_TX);
> -	binding = net_devmem_bind_dmabuf(netdev, dma_dev, DMA_TO_DEVICE,
> -					 dmabuf_fd, priv, info->extack);
> +	bind_dev = netdev_find_netmem_tx_dev(netdev);
> +	if (!bind_dev) {
> +		err = -EOPNOTSUPP;
> +		NL_SET_ERR_MSG(info->extack,
> +			       "No DMA-capable device found for netmem TX");
> +		goto err_unlock_netdev;
> +	}
> +
> +	if (bind_dev != netdev)
> +		netdev_lock(bind_dev);
> +
> +	dma_dev = netdev_queue_get_dma_dev(bind_dev, 0, NETDEV_QUEUE_TYPE_TX);
> +
> +	binding = net_devmem_bind_dmabuf(bind_dev,
> +					 bind_dev != netdev ? netdev : NULL,
> +					 dma_dev, DMA_TO_DEVICE, dmabuf_fd,
> +					 priv, info->extack);
>  	if (IS_ERR(binding)) {
>  		err = PTR_ERR(binding);
> -		goto err_unlock_netdev;
> +		goto err_unlock_bind_dev;
>  	}
>  
>  	nla_put_u32(rsp, NETDEV_A_DMABUF_ID, binding->id);
>  	genlmsg_end(rsp, hdr);
>  
> +	if (bind_dev != netdev)
> +		netdev_unlock(bind_dev);
>  	netdev_unlock(netdev);
>  	mutex_unlock(&priv->lock);
>  
>  	return genlmsg_reply(rsp, info);

If genlmsg_reply() fails here, is the dmabuf binding leaked?

The RX path (netdev_nl_bind_rx_doit) correctly rolls back by calling
net_devmem_unbind_dmabuf() on reply failure. However, it looks like the
TX path returns the error to the user but leaves the orphaned binding
attached to the netlink socket.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260511-tcp-dm-netkit-v4-0-841b78b99d74@meta.com?part=3