On 10/10, Mina Almasry wrote:
> TX devmem bindings (NETDEV_CMD_BIND_TX) have no bound RX queues, so
> mp_dmabuf_devmem_uninstall() never runs when binding->dev or
> binding->vdev unregisters. As a result, binding->dev and binding->vdev
> dangle pointing to freed struct net_device memory, and the dma_buf
> remains attached and mapped after device removal.
>
> Closing the netlink socket after netdev unregistration dereferences the
> freed struct net_device in netdev_nl_sock_priv_destroy():
>
> BUG: unable to handle page fault for address: ffff88824ca9b000
> Oops: Oops: 0000 [#1] SMP KASAN NOPTI
> RIP: 0010:netdev_nl_sock_priv_destroy+0xbd/0x1e0
> Call Trace:
> genl_release+0xf5/0x190
> netlink_release+0xd01/0x18d0
> __sock_release+0xb0/0x270
> sock_close+0x18/0x20
>
> Add net_devmem_dev_unregister() called on NETDEV_UNREGISTER:
> - When binding->dev (the physical DMA-capable device) unregisters,
> erase the binding ID, clear both binding->dev and binding->vdev, wait
> for in-flight RCU/TX readers via synchronize_net(), and unmap the
> dma_buf under binding->lock.
> - When only binding->vdev (virtual device) unregisters while
> binding->dev lives, only clear binding->vdev so no new TX sends match
> the virtual device while allowing in-flight TX packets on binding->dev
> to finish DMA safely.
>
> Fixes: bd61848900bf ("net: devmem: Implement TX path")
> Fixes: 1abe839b34ae ("net: devmem: support TX over NETMEM_TX_NO_DMA devices")
> Cc: Kaiyuan Zhang <[email protected]>
> Cc: Stanislav Fomichev <[email protected]>
> Cc: Bobby Eshleman <[email protected]>
> Cc: Tariq Toukan <[email protected]>
> Cc: Kaifeng Wang <[email protected]>
> Signed-off-by: Mina Almasry <[email protected]>
> ---
> net/core/devmem.c | 45 ++++++++++++++++++++++++++++++++++++++++++
> net/core/devmem.h | 5 +++++
> net/core/netdev-genl.c | 1 +
> 3 files changed, 51 insertions(+)
>
> diff --git a/net/core/devmem.c b/net/core/devmem.c
> index 5e4070b82df44..da7b3e8abaa13 100644
> --- a/net/core/devmem.c
> +++ b/net/core/devmem.c
> @@ -169,6 +169,51 @@ void net_devmem_unbind_dmabuf(struct
> net_devmem_dmabuf_binding *binding)
> percpu_ref_kill(&binding->ref);
> }
>
> +void net_devmem_dev_unregister(struct net_device *dev)
> +{
> + struct net_devmem_dmabuf_binding *binding;
> + unsigned long xa_idx;
> +
[..]
> + rcu_read_lock();
> + xa_for_each(&net_devmem_dmabuf_bindings, xa_idx, binding) {
> + if (!percpu_ref_tryget(&binding->ref))
> + continue;
> + rcu_read_unlock();
You use net_devmem_dmabuf_binding_put below but do not use
net_devmem_dmabuf_binding_get here.
Also, I feel like rcu_read_lock/rcu_read_unlock dance is confusing. bindings
are not following rcu lifetimes. Can we race with
__net_devmem_dmabuf_binding_free
here? (xa_for_each returns a valid binding, then before we do try_get it
gets kfree'd)
