Re: [PATCH net v4 1/4] net: devmem: unmap dma_buf synchronously on queue uninstall

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] net: devmem: UAF due to leased queue mismatch in mp_dmabuf_devmem_uninstall()
- [High] net: devmem: UAF on dma_dev when physical device backing leased queue unregisters
--

commit e1f5260c45393c53db73f20e6a31f72ae2f54f17
Author: Mina Almasry <almasrymina@google.com>

net: devmem: unmap dma_buf synchronously on queue uninstall

When a bound net_device is unregistered while userspace still holds a
netlink or TCP socket reference on a devmem binding,
mp_dmabuf_devmem_uninstall() clears binding->dev to NULL so
netdev_nl_sock_priv_destroy() will not touch the unregistered
net_device, but leaves the dma_buf attachment mapped until
__net_devmem_dmabuf_binding_free() runs later on socket close.

> diff --git a/net/core/devmem.c b/net/core/devmem.c
> index f4d60654ce7fd..20d95b24d5951 100644
> --- a/net/core/devmem.c
> +++ b/net/core/devmem.c
[ ... ]
> @@ -147,12 +183,62 @@ void net_devmem_unbind_dmabuf(struct net_devmem_dmabuf_binding *binding)
>  
>  		rxq_idx = get_netdev_rx_queue_index(rxq);
>  
> -		netif_mp_close_rxq(binding->dev, rxq_idx, &mp_params);
> +		if (netif_mp_close_rxq(binding->dev, rxq_idx, &mp_params))
> +			busy = true;

[Severity: High]
Can this dereference a freed virtual device or queue?

When a dmabuf is bound to a virtual device's leased queue,
net_devmem_bind_dmabuf_to_queue() stores the virtual queue in
binding->bound_rxqs. However, netif_mp_open_rxq() delegates the memory
provider setup to the underlying physical queue, leaving the virtual queue's
mp_params.mp_ops as NULL.

If the virtual device is unregistered while the socket is still open, its
dev_memory_provider_uninstall() skips the virtual queue entirely since its
mp_ops is NULL, leaving the binding intact.

On socket close, this loop iterates over binding->bound_rxqs and uses the
freed virtual device pointer (binding->dev) and freed virtual queue,
potentially causing a use-after-free.

[ ... ]
> +static bool
> +net_devmem_dev_unregister_rx(struct net_devmem_dmabuf_binding *binding,
> +			     unsigned long xa_idx, struct net_device *dev)
> +{
> +	return binding->direction == DMA_FROM_DEVICE &&
> +	       READ_ONCE(binding->dev) == dev &&
> +	       !xa_get_mark(&net_devmem_dmabuf_bindings, xa_idx,
> +			    NET_DEVMEM_BINDING_LIVE);
> +}

[Severity: High]
Does this strict device match prevent the physical device from handling its
own unregistration when it backs a leased queue?

If userspace binds a dmabuf to a virtual device with leased queues,
binding->dev is set to the virtual device.

If the underlying physical device unregisters (e.g., driver unbind),
net_devmem_dev_unregister() will ignore the binding because binding->dev
matches the virtual device, not the physical device.

This allows the physical device to finish unregistration and free its dma_dev
while the dmabuf remains mapped, leading to a use-after-free when the netlink
socket is eventually closed.

[ ... ]
> @@ -529,16 +615,19 @@ static void mp_dmabuf_devmem_uninstall(void *mp_priv,
>  	unsigned long xa_idx;
>  
>  	xa_for_each(&binding->bound_rxqs, xa_idx, bound_rxq) {
> -		if (bound_rxq == rxq) {
> -			xa_erase(&binding->bound_rxqs, xa_idx);
> -			if (xa_empty(&binding->bound_rxqs)) {
> -				mutex_lock(&binding->lock);
> -				ASSERT_EXCLUSIVE_WRITER(binding->dev);
> -				WRITE_ONCE(binding->dev, NULL);
> -				mutex_unlock(&binding->lock);
> -			}
> -			break;
> +		if (bound_rxq != rxq)
> +			continue;
> +
> +		xa_erase(&binding->bound_rxqs, xa_idx);

[Severity: High]
Will this mismatch prevent the cleanup of the binding during physical device
unregistration?

During physical device unregistration, dev_memory_provider_uninstall() calls
mp_dmabuf_devmem_uninstall() with the physical queue (phys_rxq) because the
memory provider was installed on it. However, binding->bound_rxqs contains
the virtual queue (virt_rxq). 

Because the queues do not match, this loop will continue and skip erasing the
queue and unmapping the binding.

Combined with the device matching issue above, this prevents the physical
device from detaching the binding when it unregisters.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261015163915.1166043-1-almasrymina@google.com?part=1