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 <[email protected]> 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/[email protected]?part=1
