[PATCH net v4 0/4] net: devmem: fix RX and TX binding teardown on device unregistration

[PATCH net v4 0/4] net: devmem: fix RX and TX binding teardown on device unregistration

[Mina Almasry]

Fix two device-unregistration lifecycle bugs in devmem TCP bindings and
add netdevsim support plus selftests covering both RX and TX device
unregistration:

1. net: devmem: unmap dma_buf synchronously on queue uninstall
   Synchronously unmap and detach binding->attachment under binding->lock
   when the last RX queue is uninstalled in mp_dmabuf_devmem_uninstall()
   or closed in net_devmem_unbind_dmabuf(), and unmap unbound RX bindings
   whose queue failed to restart on NETDEV_UNREGISTER, preventing
   slab-use-after-free in dma_unmap_sg_attrs() when the binding is freed
   after dma_dev.

2. net: devmem: detach TX bindings on NETDEV_UNREGISTER
   Extend net_devmem_dev_unregister() to fully detach and unmap TX
   bindings when binding->dev unregisters, or clear binding->vdev alone
   when only a virtual device unregisters while binding->dev lives.

3. netdevsim: support devmem RX and TX bindings
   Enable PP_FLAG_DMA_MAP | PP_FLAG_ALLOW_UNREADABLE_NETMEM, 64-bit DMA mask
   on nsim_bus_dev, and NETMEM_TX_DMA on netdevsim so devmem RX/TX bindings
   can be exercised in software selftests.

4. selftests: net: add devmem RX and TX netdev unregister tests
   Add devmem_bind_rx_unregister_check and devmem_bind_tx_unregister_check
   to tools/testing/selftests/net/nl_netdev.py.

Cc: Tariq Toukan <tariqt@nvidia.com>
Cc: Kaifeng Wang <kaifengw@google.com>
Cc: Dragos Tatulea <dtatulea@nvidia.com>

---
v4:
- 1/4: Unmap RX bindings in net_devmem_unbind_dmabuf() once their queues
  are closed, and make netif_mp_close_rxq() report queue restart
  failures, so the NETDEV_UNREGISTER walk only handles bindings whose
  queue failed to restart. Drop net_devmem_lock: walk the bindings under
  the xarray lock and take the instance lock of binding->dev in the
  final free. (Stanislav) Record a queue in bound_rxqs before opening
  it. (Sashiko)
- 2/4: Detach and unmap TX bindings directly from the walk, without a
  grace period or a second pass. (Stanislav)
- Link to v3: https://lore.kernel.org/netdev/20261014161754.3782346-1-almasrymina@google.com/

v3:
- 1/4: Run the NETDEV_UNREGISTER walk under the netdev instance lock,
  and skip it when the device only moves to another netns. (Sashiko)
- 2/4: Detach all matching TX bindings, then synchronize_net() once
  before unmapping them. (Sashiko)
- 3/4: Only request page pool DMA mapping with CONFIG_HAS_DMA, refuse
  pp_hold on memory provider pools, and drop unreadable skbs in the
  software PSP checksum path. (Sashiko)
- 4/4: Check only the test's dma-buf in debugfs, skip instead of passing
  silently, clean up on errors, compute UDMABUF_CREATE per arch, and
  add the needed config options. (Sashiko, Dragos)
- Link to v2: https://lore.kernel.org/netdev/20261012174300.2338784-1-almasrymina@google.com/

v2:
- 1/4: Don't unmap when a leased queue is unleased. Keep bindings in
  net_devmem_dmabuf_bindings until freed, hidden from TX lookups by an
  xarray mark, and unmap unbound RX bindings on NETDEV_UNREGISTER. Update
  the binding->lock and binding->ref comments. (Sashiko)
- 2/4: Detach TX bindings from the same net_devmem_lock-protected walk,
  which now also covers unbound TX bindings, and clear the LIVE mark
  instead of erasing the ID. The walk no longer uses RCU or takes
  binding references (Stanislav).
- Link to v1: https://lore.kernel.org/netdev/20261010025532.839559-1-almasrymina@google.com/

Mina Almasry (4):
  net: devmem: unmap dma_buf synchronously on queue uninstall
  net: devmem: detach TX bindings on NETDEV_UNREGISTER
  netdevsim: support devmem RX and TX bindings
  selftests: net: add devmem RX and TX netdev unregister tests

 drivers/net/netdevsim/bus.c              |   1 +
 drivers/net/netdevsim/netdev.c           |  27 +++-
 drivers/net/netdevsim/psp.c              |   5 +
 include/net/page_pool/memory_provider.h  |   4 +-
 net/core/devmem.c                        | 157 +++++++++++++++++++----
 net/core/devmem.h                        |  11 +-
 net/core/netdev-genl.c                   |   1 +
 net/core/netdev_rx_queue.c               |  26 ++--
 tools/testing/selftests/net/config       |   4 +
 tools/testing/selftests/net/nl_netdev.py | 125 +++++++++++++++++-
 10 files changed, 314 insertions(+), 47 deletions(-)


base-commit: aa07285705f8d5bac464eb1c5b7412f8e88b91e1
-- 
2.56.0.176.g3575e65682-goog

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

[Mina Almasry]

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.

Because dma_buf_attach() does not take a reference on dma_dev, device
removal frees dma_dev while the attachment is still mapped. Closing the
netlink socket afterwards triggers a slab use-after-free in
dma_unmap_sg_attrs():

  BUG: KASAN: slab-use-after-free in dma_unmap_sg_attrs+0x76/0x2b0
  Read of size 8 at addr ffff88810fae64b0 by task kworker/3:1/53
  Workqueue: events __net_devmem_dmabuf_binding_free
  Call Trace:
   dma_unmap_sg_attrs+0x76/0x2b0
   unmap_udmabuf+0x8e/0x100
   dma_buf_unmap_attachment+0x145/0x330
   dma_buf_unmap_attachment_unlocked+0x96/0x130
   __net_devmem_dmabuf_binding_free+0x156/0x2e0
  Freed by task 9:
   kfree+0x1b8/0x550
   nsim_bus_dev_release+0xe/0x60
   device_release+0xcf/0x250

Add net_devmem_dmabuf_binding_unmap() serialized under binding->lock
and null out binding->sgt and binding->attachment after unmapping so
unmapping is idempotent across queue uninstall and deferred binding
destruction. Call it synchronously when the last bound RX queue is
uninstalled in mp_dmabuf_devmem_uninstall(), except when a leased
queue is unleased: the physical device is still registered and, if
restarting its queue failed, may still be using the binding.

The same use-after-free happens when the netlink socket is closed
before the device is removed but RX tokens keep the binding alive. Once
its queues are closed, nothing DMAs into an RX binding anymore and the
outstanding tokens only need the net_iovs, so unmap it in
net_devmem_unbind_dmabuf(). Make netif_mp_close_rxq() return an error
when a queue failed to restart and may still be using the binding. Such
a binding stays mapped and keeps binding->dev. Record a queue in
bound_rxqs before opening it, so that a failed bind never leaves a
queue using the binding without unbind knowing about it.

Keep bindings in net_devmem_dmabuf_bindings until they are freed and
hide unbound ones from TX lookups with an xarray mark, so that on
NETDEV_UNREGISTER the remaining unbound RX bindings of the device can be
found, detached and unmapped, unless the device is a virtual device
leasing queues, for the same reason as above. The walk runs under the
device's instance lock and is skipped when the device only moves to
another netns. Like netdev_nl_sock_priv_destroy(),
__net_devmem_dmabuf_binding_free() takes the instance lock of
binding->dev before unmapping and erasing the binding, so a binding the
walk finds under the xarray lock stays alive while it is detached.

Fixes: f8350a4358fc ("net: page_pool: add a mp hook to unregister_netdevice*")
Fixes: 0afc44d8cdf6 ("net: devmem: fix kernel panic when netlink socket close after module unload")
Suggested-by: Stanislav Fomichev <sdf@fomichev.me>
Cc: Taehee Yoo <ap420073@gmail.com>
Cc: Tariq Toukan <tariqt@nvidia.com>
Cc: Kaifeng Wang <kaifengw@google.com>
Assisted-by: LLM
Signed-off-by: Mina Almasry <almasrymina@google.com>
Reviewed-by: Dragos Tatulea <dtatulea@nvidia.com>
---
v4:
- Unmap RX bindings in net_devmem_unbind_dmabuf() once their queues are
  closed, and make netif_mp_close_rxq() report queue restart failures.
  The NETDEV_UNREGISTER walk only handles bindings whose queue failed
  to restart (Stanislav).
- Drop net_devmem_lock. Walk the bindings under the xarray lock, and
  take the instance lock of binding->dev in the final free (Stanislav).
- Record a queue in bound_rxqs before opening it, so unbind knows about
  every queue using the binding (Sashiko).

v3:
- Run the NETDEV_UNREGISTER walk under the netdev instance lock so it
  cannot race with net_devmem_unbind_dmabuf() (Sashiko).
- Skip the walk when the device only moves to another netns (Sashiko).

v2:
- Don't unmap when a leased queue is unleased (Sashiko).
- Unmap unbound RX bindings on NETDEV_UNREGISTER; keep bindings in
  net_devmem_dmabuf_bindings until freed, hidden from TX lookups by
  NET_DEVMEM_BINDING_LIVE (Sashiko).
- Update the binding->lock and binding->ref comments (Sashiko).
---
 include/net/page_pool/memory_provider.h |   4 +-
 net/core/devmem.c                       | 137 +++++++++++++++++++-----
 net/core/devmem.h                       |  10 +-
 net/core/netdev-genl.c                  |   1 +
 net/core/netdev_rx_queue.c              |  26 +++--
 5 files changed, 140 insertions(+), 38 deletions(-)

diff --git a/include/net/page_pool/memory_provider.h b/include/net/page_pool/memory_provider.h
index 255ce4cfd9755..a999a9b28bc70 100644
--- a/include/net/page_pool/memory_provider.h
+++ b/include/net/page_pool/memory_provider.h
@@ -26,8 +26,8 @@ void net_mp_niov_clear_page_pool(struct net_iov *niov);
 int netif_mp_open_rxq(struct net_device *dev, unsigned int rxq_idx,
 		      const struct pp_memory_provider_params *p,
 		      struct netlink_ext_ack *extack);
-void netif_mp_close_rxq(struct net_device *dev, unsigned int rxq_idx,
-			const struct pp_memory_provider_params *old_p);
+int netif_mp_close_rxq(struct net_device *dev, unsigned int rxq_idx,
+		       const struct pp_memory_provider_params *old_p);
 
 /**
   * net_mp_netmem_place_in_cache() - give a netmem to a page pool
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
@@ -12,6 +12,7 @@
 #include <linux/mm.h>
 #include <linux/netdevice.h>
 #include <linux/types.h>
+#include <net/netdev_lock.h>
 #include <net/netdev_queues.h>
 #include <net/netdev_rx_queue.h>
 #include <net/page_pool/helpers.h>
@@ -20,6 +21,7 @@
 #include <net/tcp.h>
 #include <trace/events/page_pool.h>
 
+#include "dev.h"
 #include "devmem.h"
 #include "mp_dmabuf_devmem.h"
 #include "page_pool_priv.h"
@@ -27,6 +29,7 @@
 /* Device memory support */
 
 static DEFINE_XARRAY_FLAGS(net_devmem_dmabuf_bindings, XA_FLAGS_ALLOC1);
+#define NET_DEVMEM_BINDING_LIVE XA_MARK_1
 
 static const struct memory_provider_ops dmabuf_devmem_ops;
 
@@ -58,9 +61,26 @@ static void net_devmem_dmabuf_binding_release(struct percpu_ref *ref)
 	schedule_work(&binding->unbind_w);
 }
 
+static void
+net_devmem_dmabuf_binding_unmap(struct net_devmem_dmabuf_binding *binding)
+{
+	lockdep_assert_held(&binding->lock);
+
+	if (!binding->sgt)
+		return;
+
+	dma_buf_unmap_attachment_unlocked(binding->attachment, binding->sgt,
+					  binding->direction);
+	dma_buf_detach(binding->dmabuf, binding->attachment);
+	binding->sgt = NULL;
+	binding->attachment = NULL;
+}
+
 void __net_devmem_dmabuf_binding_free(struct work_struct *wq)
 {
 	struct net_devmem_dmabuf_binding *binding = container_of(wq, typeof(*binding), unbind_w);
+	netdevice_tracker dev_tracker;
+	struct net_device *dev;
 
 	size_t size, avail;
 
@@ -74,9 +94,23 @@ void __net_devmem_dmabuf_binding_free(struct work_struct *wq)
 		  size, avail))
 		gen_pool_destroy(binding->chunk_pool);
 
-	dma_buf_unmap_attachment_unlocked(binding->attachment, binding->sgt,
-					  binding->direction);
-	dma_buf_detach(binding->dmabuf, binding->attachment);
+	mutex_lock(&binding->lock);
+	dev = binding->dev;
+	if (dev)
+		netdev_hold(dev, &dev_tracker, GFP_KERNEL);
+	mutex_unlock(&binding->lock);
+
+	if (dev)
+		netdev_lock(dev);
+	mutex_lock(&binding->lock);
+	net_devmem_dmabuf_binding_unmap(binding);
+	mutex_unlock(&binding->lock);
+	xa_erase(&net_devmem_dmabuf_bindings, binding->id);
+	if (dev) {
+		netdev_unlock(dev);
+		netdev_put(dev, &dev_tracker);
+	}
+
 	dma_buf_put(binding->dmabuf);
 	xa_destroy(&binding->bound_rxqs);
 	percpu_ref_exit(&binding->ref);
@@ -128,11 +162,13 @@ void net_devmem_unbind_dmabuf(struct net_devmem_dmabuf_binding *binding)
 	struct netdev_rx_queue *rxq;
 	unsigned long xa_idx;
 	unsigned int rxq_idx;
+	bool busy = false;
 
-	xa_erase(&net_devmem_dmabuf_bindings, binding->id);
+	xa_clear_mark(&net_devmem_dmabuf_bindings, binding->id,
+		      NET_DEVMEM_BINDING_LIVE);
 
 	/* Ensure no tx net_devmem_lookup_dmabuf() are in flight after the
-	 * erase.
+	 * mark is cleared.
 	 */
 	synchronize_net();
 
@@ -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;
+	}
+
+	if (binding->direction == DMA_FROM_DEVICE && binding->dev && !busy) {
+		mutex_lock(&binding->lock);
+		WRITE_ONCE(binding->dev, NULL);
+		net_devmem_dmabuf_binding_unmap(binding);
+		mutex_unlock(&binding->lock);
 	}
 
 	percpu_ref_kill(&binding->ref);
 }
 
+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);
+}
+
+static void net_devmem_dev_detach(struct net_devmem_dmabuf_binding *binding,
+				  struct net_device *dev)
+{
+	mutex_lock(&binding->lock);
+	WRITE_ONCE(binding->dev, NULL);
+	if (!netif_is_queue_leasee(dev))
+		net_devmem_dmabuf_binding_unmap(binding);
+	mutex_unlock(&binding->lock);
+}
+
+void net_devmem_dev_unregister(struct net_device *dev)
+{
+	struct net_devmem_dmabuf_binding *binding;
+	unsigned long xa_idx;
+
+	netdev_assert_locked(dev);
+
+	if (dev->reg_state != NETREG_UNREGISTERING)
+		return;
+
+	xa_lock(&net_devmem_dmabuf_bindings);
+	xa_for_each(&net_devmem_dmabuf_bindings, xa_idx, binding) {
+		if (!net_devmem_dev_unregister_rx(binding, xa_idx, dev))
+			continue;
+
+		xa_unlock(&net_devmem_dmabuf_bindings);
+		net_devmem_dev_detach(binding, dev);
+		xa_lock(&net_devmem_dmabuf_bindings);
+	}
+	xa_unlock(&net_devmem_dmabuf_bindings);
+}
+
 int net_devmem_bind_dmabuf_to_queue(struct net_device *dev, u32 rxq_idx,
 				    struct net_devmem_dmabuf_binding *binding,
 				    struct netlink_ext_ack *extack)
@@ -168,20 +254,16 @@ int net_devmem_bind_dmabuf_to_queue(struct net_device *dev, u32 rxq_idx,
 	if (binding->niov_shift != PAGE_SHIFT)
 		mp_params.rx_page_size = 1U << binding->niov_shift;
 
-	err = netif_mp_open_rxq(dev, rxq_idx, &mp_params, extack);
-	if (err)
-		return err;
-
 	rxq = __netif_get_rx_queue(dev, rxq_idx);
 	err = xa_alloc(&binding->bound_rxqs, &xa_idx, rxq, xa_limit_32b,
 		       GFP_KERNEL);
 	if (err)
-		goto err_close_rxq;
+		return err;
 
-	return 0;
+	err = netif_mp_open_rxq(dev, rxq_idx, &mp_params, extack);
+	if (err)
+		xa_erase(&binding->bound_rxqs, xa_idx);
 
-err_close_rxq:
-	netif_mp_close_rxq(dev, rxq_idx, &mp_params);
 	return err;
 }
 
@@ -331,6 +413,8 @@ net_devmem_bind_dmabuf(struct net_device *dev, void *vdev,
 			      GFP_KERNEL);
 	if (err < 0)
 		goto err_free_chunks;
+	xa_set_mark(&net_devmem_dmabuf_bindings, binding->id,
+		    NET_DEVMEM_BINDING_LIVE);
 
 	list_add(&binding->list, &priv->bindings);
 
@@ -359,9 +443,11 @@ net_devmem_bind_dmabuf(struct net_device *dev, void *vdev,
 struct net_devmem_dmabuf_binding *net_devmem_lookup_dmabuf(u32 id)
 {
 	struct net_devmem_dmabuf_binding *binding;
+	unsigned long xa_idx = id;
 
 	rcu_read_lock();
-	binding = xa_load(&net_devmem_dmabuf_bindings, id);
+	binding = xa_find(&net_devmem_dmabuf_bindings, &xa_idx, id,
+			  NET_DEVMEM_BINDING_LIVE);
 	if (binding) {
 		if (!net_devmem_dmabuf_binding_get(binding))
 			binding = NULL;
@@ -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);
+		if (xa_empty(&binding->bound_rxqs)) {
+			mutex_lock(&binding->lock);
+			ASSERT_EXCLUSIVE_WRITER(binding->dev);
+			WRITE_ONCE(binding->dev, NULL);
+			if (!rxq->lease)
+				net_devmem_dmabuf_binding_unmap(binding);
+			mutex_unlock(&binding->lock);
 		}
+		break;
 	}
 }
 
diff --git a/net/core/devmem.h b/net/core/devmem.h
index 4a293a7d1149c..a834e0f698c7e 100644
--- a/net/core/devmem.h
+++ b/net/core/devmem.h
@@ -27,7 +27,7 @@ struct net_devmem_dmabuf_binding {
 	 */
 	void *vdev;
 	struct gen_pool *chunk_pool;
-	/* Protect dev */
+	/* Protect dev, attachment and sgt */
 	struct mutex lock;
 
 	/* The user holds a ref (via the netlink API) for as long as they want
@@ -38,7 +38,8 @@ struct net_devmem_dmabuf_binding {
 	 *
 	 * The binding undos itself and unmaps the underlying dmabuf once all
 	 * those refs are dropped and the binding is no longer desired or in
-	 * use.
+	 * use. RX bindings are unmapped earlier, once they are no longer
+	 * installed on any queue.
 	 *
 	 * net_devmem_get_net_iov() on dmabuf net_iovs will increment this
 	 * reference, making sure that the binding remains alive until all the
@@ -100,6 +101,7 @@ net_devmem_bind_dmabuf(struct net_device *dev, void *vdev,
 		       struct netlink_ext_ack *extack);
 struct net_devmem_dmabuf_binding *net_devmem_lookup_dmabuf(u32 id);
 void net_devmem_unbind_dmabuf(struct net_devmem_dmabuf_binding *binding);
+void net_devmem_dev_unregister(struct net_device *dev);
 int net_devmem_bind_dmabuf_to_queue(struct net_device *dev, u32 rxq_idx,
 				    struct net_devmem_dmabuf_binding *binding,
 				    struct netlink_ext_ack *extack);
@@ -196,6 +198,10 @@ net_devmem_unbind_dmabuf(struct net_devmem_dmabuf_binding *binding)
 {
 }
 
+static inline void net_devmem_dev_unregister(struct net_device *dev)
+{
+}
+
 static inline int
 net_devmem_bind_dmabuf_to_queue(struct net_device *dev, u32 rxq_idx,
 				struct net_devmem_dmabuf_binding *binding,
diff --git a/net/core/netdev-genl.c b/net/core/netdev-genl.c
index 33b9f4eb9565a..b04e22937b789 100644
--- a/net/core/netdev-genl.c
+++ b/net/core/netdev-genl.c
@@ -1488,6 +1488,7 @@ static int netdev_genl_netdevice_event(struct notifier_block *nb,
 		break;
 	case NETDEV_UNREGISTER:
 		netdev_lock(netdev);
+		net_devmem_dev_unregister(netdev);
 		netdev_genl_dev_notify(netdev, NETDEV_CMD_DEV_DEL_NTF);
 		netdev_unlock(netdev);
 		break;
diff --git a/net/core/netdev_rx_queue.c b/net/core/netdev_rx_queue.c
index 00a7011eb4d57..a7261fedd5e76 100644
--- a/net/core/netdev_rx_queue.c
+++ b/net/core/netdev_rx_queue.c
@@ -275,8 +275,8 @@ int netif_mp_open_rxq(struct net_device *dev, unsigned int rxq_idx,
 	return ret;
 }
 
-static void __netif_mp_close_rxq(struct net_device *dev, unsigned int rxq_idx,
-				 const struct pp_memory_provider_params *old_p)
+static int __netif_mp_close_rxq(struct net_device *dev, unsigned int rxq_idx,
+				const struct pp_memory_provider_params *old_p)
 {
 	struct netdev_queue_config qcfg[2];
 	struct netdev_rx_queue *rxq;
@@ -289,35 +289,41 @@ static void __netif_mp_close_rxq(struct net_device *dev, unsigned int rxq_idx,
 	 */
 	if (dev->reg_state > NETREG_REGISTERED &&
 	    !rxq->mp_params.mp_ops)
-		return;
+		return 0;
 
 	if (WARN_ON_ONCE(rxq->mp_params.mp_ops != old_p->mp_ops ||
 			 rxq->mp_params.mp_priv != old_p->mp_priv))
-		return;
+		return -EINVAL;
 
 	netdev_queue_config(dev, rxq_idx, &qcfg[0]);
 	memset(&rxq->mp_params, 0, sizeof(rxq->mp_params));
 	netdev_queue_config(dev, rxq_idx, &qcfg[1]);
 
 	err = netdev_rx_queue_reconfig(dev, rxq_idx, &qcfg[0], &qcfg[1]);
-	WARN_ON(err && err != -ENETDOWN);
+	if (err == -ENETDOWN)
+		return 0;
+	WARN_ON(err);
+	return err;
 }
 
-void netif_mp_close_rxq(struct net_device *dev, unsigned int rxq_idx,
-			const struct pp_memory_provider_params *old_p)
+int netif_mp_close_rxq(struct net_device *dev, unsigned int rxq_idx,
+		       const struct pp_memory_provider_params *old_p)
 {
+	int ret;
+
 	if (WARN_ON_ONCE(rxq_idx >= dev->real_num_rx_queues))
-		return;
+		return -EINVAL;
 	if (!netif_rxq_is_leased(dev, rxq_idx))
 		return __netif_mp_close_rxq(dev, rxq_idx, old_p);
 
 	if (!__netif_get_rx_queue_lease(&dev, &rxq_idx, NETIF_VIRT_TO_PHYS)) {
 		WARN_ON_ONCE(1);
-		return;
+		return -EINVAL;
 	}
 	netdev_lock(dev);
-	__netif_mp_close_rxq(dev, rxq_idx, old_p);
+	ret = __netif_mp_close_rxq(dev, rxq_idx, old_p);
 	netdev_unlock(dev);
+	return ret;
 }
 
 void __netif_mp_uninstall_rxq(struct netdev_rx_queue *rxq,
-- 
2.56.0.176.g3575e65682-goog

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

[Stanislav Fomichev]

On 10/15, Mina Almasry wrote:
> 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.
> 
> Because dma_buf_attach() does not take a reference on dma_dev, device
> removal frees dma_dev while the attachment is still mapped. Closing the
> netlink socket afterwards triggers a slab use-after-free in
> dma_unmap_sg_attrs():
> 
>   BUG: KASAN: slab-use-after-free in dma_unmap_sg_attrs+0x76/0x2b0
>   Read of size 8 at addr ffff88810fae64b0 by task kworker/3:1/53
>   Workqueue: events __net_devmem_dmabuf_binding_free
>   Call Trace:
>    dma_unmap_sg_attrs+0x76/0x2b0
>    unmap_udmabuf+0x8e/0x100
>    dma_buf_unmap_attachment+0x145/0x330
>    dma_buf_unmap_attachment_unlocked+0x96/0x130
>    __net_devmem_dmabuf_binding_free+0x156/0x2e0
>   Freed by task 9:
>    kfree+0x1b8/0x550
>    nsim_bus_dev_release+0xe/0x60
>    device_release+0xcf/0x250
> 
> Add net_devmem_dmabuf_binding_unmap() serialized under binding->lock
> and null out binding->sgt and binding->attachment after unmapping so
> unmapping is idempotent across queue uninstall and deferred binding
> destruction. Call it synchronously when the last bound RX queue is
> uninstalled in mp_dmabuf_devmem_uninstall(), except when a leased
> queue is unleased: the physical device is still registered and, if
> restarting its queue failed, may still be using the binding.
> 
> The same use-after-free happens when the netlink socket is closed
> before the device is removed but RX tokens keep the binding alive. Once
> its queues are closed, nothing DMAs into an RX binding anymore and the
> outstanding tokens only need the net_iovs, so unmap it in
> net_devmem_unbind_dmabuf(). Make netif_mp_close_rxq() return an error
> when a queue failed to restart and may still be using the binding. Such
> a binding stays mapped and keeps binding->dev. Record a queue in
> bound_rxqs before opening it, so that a failed bind never leaves a
> queue using the binding without unbind knowing about it.
> 
> Keep bindings in net_devmem_dmabuf_bindings until they are freed and
> hide unbound ones from TX lookups with an xarray mark, so that on
> NETDEV_UNREGISTER the remaining unbound RX bindings of the device can be
> found, detached and unmapped, unless the device is a virtual device
> leasing queues, for the same reason as above. The walk runs under the
> device's instance lock and is skipped when the device only moves to
> another netns. Like netdev_nl_sock_priv_destroy(),
> __net_devmem_dmabuf_binding_free() takes the instance lock of
> binding->dev before unmapping and erasing the binding, so a binding the
> walk finds under the xarray lock stays alive while it is detached.
> 
> Fixes: f8350a4358fc ("net: page_pool: add a mp hook to unregister_netdevice*")
> Fixes: 0afc44d8cdf6 ("net: devmem: fix kernel panic when netlink socket close after module unload")
> Suggested-by: Stanislav Fomichev <sdf@fomichev.me>
> Cc: Taehee Yoo <ap420073@gmail.com>
> Cc: Tariq Toukan <tariqt@nvidia.com>
> Cc: Kaifeng Wang <kaifengw@google.com>
> Assisted-by: LLM
> Signed-off-by: Mina Almasry <almasrymina@google.com>
> Reviewed-by: Dragos Tatulea <dtatulea@nvidia.com>
> ---
> v4:
> - Unmap RX bindings in net_devmem_unbind_dmabuf() once their queues are
>   closed, and make netif_mp_close_rxq() report queue restart failures.
>   The NETDEV_UNREGISTER walk only handles bindings whose queue failed
>   to restart (Stanislav).
> - Drop net_devmem_lock. Walk the bindings under the xarray lock, and
>   take the instance lock of binding->dev in the final free (Stanislav).
> - Record a queue in bound_rxqs before opening it, so unbind knows about
>   every queue using the binding (Sashiko).
> 
> v3:
> - Run the NETDEV_UNREGISTER walk under the netdev instance lock so it
>   cannot race with net_devmem_unbind_dmabuf() (Sashiko).
> - Skip the walk when the device only moves to another netns (Sashiko).
> 
> v2:
> - Don't unmap when a leased queue is unleased (Sashiko).
> - Unmap unbound RX bindings on NETDEV_UNREGISTER; keep bindings in
>   net_devmem_dmabuf_bindings until freed, hidden from TX lookups by
>   NET_DEVMEM_BINDING_LIVE (Sashiko).
> - Update the binding->lock and binding->ref comments (Sashiko).
> ---
>  include/net/page_pool/memory_provider.h |   4 +-
>  net/core/devmem.c                       | 137 +++++++++++++++++++-----
>  net/core/devmem.h                       |  10 +-
>  net/core/netdev-genl.c                  |   1 +
>  net/core/netdev_rx_queue.c              |  26 +++--
>  5 files changed, 140 insertions(+), 38 deletions(-)
> 
> diff --git a/include/net/page_pool/memory_provider.h b/include/net/page_pool/memory_provider.h
> index 255ce4cfd9755..a999a9b28bc70 100644
> --- a/include/net/page_pool/memory_provider.h
> +++ b/include/net/page_pool/memory_provider.h
> @@ -26,8 +26,8 @@ void net_mp_niov_clear_page_pool(struct net_iov *niov);
>  int netif_mp_open_rxq(struct net_device *dev, unsigned int rxq_idx,
>  		      const struct pp_memory_provider_params *p,
>  		      struct netlink_ext_ack *extack);
> -void netif_mp_close_rxq(struct net_device *dev, unsigned int rxq_idx,
> -			const struct pp_memory_provider_params *old_p);
> +int netif_mp_close_rxq(struct net_device *dev, unsigned int rxq_idx,
> +		       const struct pp_memory_provider_params *old_p);
>  
>  /**
>    * net_mp_netmem_place_in_cache() - give a netmem to a page pool
> 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
> @@ -12,6 +12,7 @@
>  #include <linux/mm.h>
>  #include <linux/netdevice.h>
>  #include <linux/types.h>
> +#include <net/netdev_lock.h>
>  #include <net/netdev_queues.h>
>  #include <net/netdev_rx_queue.h>
>  #include <net/page_pool/helpers.h>
> @@ -20,6 +21,7 @@
>  #include <net/tcp.h>
>  #include <trace/events/page_pool.h>
>  
> +#include "dev.h"
>  #include "devmem.h"
>  #include "mp_dmabuf_devmem.h"
>  #include "page_pool_priv.h"
> @@ -27,6 +29,7 @@
>  /* Device memory support */
>  
>  static DEFINE_XARRAY_FLAGS(net_devmem_dmabuf_bindings, XA_FLAGS_ALLOC1);
> +#define NET_DEVMEM_BINDING_LIVE XA_MARK_1
>  
>  static const struct memory_provider_ops dmabuf_devmem_ops;
>  
> @@ -58,9 +61,26 @@ static void net_devmem_dmabuf_binding_release(struct percpu_ref *ref)
>  	schedule_work(&binding->unbind_w);
>  }
>  
> +static void
> +net_devmem_dmabuf_binding_unmap(struct net_devmem_dmabuf_binding *binding)
> +{
> +	lockdep_assert_held(&binding->lock);
> +
> +	if (!binding->sgt)
> +		return;
> +
> +	dma_buf_unmap_attachment_unlocked(binding->attachment, binding->sgt,
> +					  binding->direction);
> +	dma_buf_detach(binding->dmabuf, binding->attachment);
> +	binding->sgt = NULL;
> +	binding->attachment = NULL;
> +}
> +
>  void __net_devmem_dmabuf_binding_free(struct work_struct *wq)
>  {
>  	struct net_devmem_dmabuf_binding *binding = container_of(wq, typeof(*binding), unbind_w);
> +	netdevice_tracker dev_tracker;
> +	struct net_device *dev;
>  
>  	size_t size, avail;
>  
> @@ -74,9 +94,23 @@ void __net_devmem_dmabuf_binding_free(struct work_struct *wq)
>  		  size, avail))
>  		gen_pool_destroy(binding->chunk_pool);
>  
> -	dma_buf_unmap_attachment_unlocked(binding->attachment, binding->sgt,
> -					  binding->direction);
> -	dma_buf_detach(binding->dmabuf, binding->attachment);
> +	mutex_lock(&binding->lock);
> +	dev = binding->dev;
> +	if (dev)
> +		netdev_hold(dev, &dev_tracker, GFP_KERNEL);
> +	mutex_unlock(&binding->lock);
> +
> +	if (dev)
> +		netdev_lock(dev);
> +	mutex_lock(&binding->lock);
> +	net_devmem_dmabuf_binding_unmap(binding);
> +	mutex_unlock(&binding->lock);
> +	xa_erase(&net_devmem_dmabuf_bindings, binding->id);
> +	if (dev) {
> +		netdev_unlock(dev);
> +		netdev_put(dev, &dev_tracker);
> +	}
> +
>  	dma_buf_put(binding->dmabuf);
>  	xa_destroy(&binding->bound_rxqs);
>  	percpu_ref_exit(&binding->ref);
> @@ -128,11 +162,13 @@ void net_devmem_unbind_dmabuf(struct net_devmem_dmabuf_binding *binding)
>  	struct netdev_rx_queue *rxq;
>  	unsigned long xa_idx;
>  	unsigned int rxq_idx;
> +	bool busy = false;
>  
> -	xa_erase(&net_devmem_dmabuf_bindings, binding->id);
> +	xa_clear_mark(&net_devmem_dmabuf_bindings, binding->id,
> +		      NET_DEVMEM_BINDING_LIVE);
>  
>  	/* Ensure no tx net_devmem_lookup_dmabuf() are in flight after the
> -	 * erase.
> +	 * mark is cleared.
>  	 */
>  	synchronize_net();
>  
> @@ -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;
> +	}
> +
> +	if (binding->direction == DMA_FROM_DEVICE && binding->dev && !busy) {
> +		mutex_lock(&binding->lock);
> +		WRITE_ONCE(binding->dev, NULL);
> +		net_devmem_dmabuf_binding_unmap(binding);
> +		mutex_unlock(&binding->lock);
>  	}
>  
>  	percpu_ref_kill(&binding->ref);
>  }
>  

[..]

> +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);
> +}

super nit: if you happen to respin, I'd just open code this part in
net_devmem_dev_unregister, it reads funny to me idk, but maybe it's just me.
But even as is LGTM, thanks!

Acked-by: Stanislav Fomichev <sdf@fomichev.me>

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

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 4 potential
issue(s) to consider.

Critical: 0 · High: 3 · Medium: 0 (1) · Low: 1

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [High] This patch leaves TX bindings (NETDEV_CMD_BIND_TX,
  DMA_TO_DEVICE) uncovered.
- [High] Leased-queue RX bindings still hit the dma_unmap_sg_attrs() UAF
  that the commit says it fixes.
- [High] There is a use-after-free race between the NETDEV_UNREGISTER
  walk and the binding free worker.
- [Low] The updated refcount comment in struct net_devmem_dmabuf_binding
  (net/core/devmem.h:41-42) says 'RX bindings are unmapped earlier, once…

Pre-existing issues:
- [Medium] This patch was not the cause, but netif_mp_close_rxq() now
  returns non-zero when queue reconfiguration fails (the old queue may…

--

net: devmem: unmap dma_buf synchronously on queue uninstall

This patch unmaps devmem RX dma_buf attachments early, at two points: when
the last bound RX queue is uninstalled, and in net_devmem_unbind_dmabuf()
once the binding's queues are closed. This avoids a use-after-free of
dma_dev in the deferred unmap. Bindings now stay in the global xarray until
they are freed, unbound ones are hidden from TX lookup by an xarray mark,
and leftover RX bindings are detached and unmapped on NETDEV_UNREGISTER.

> 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

[ ... ]

> @@ -74,9 +94,23 @@ void __net_devmem_dmabuf_binding_free(struct work_struct *wq)
>  		  size, avail))
>  		gen_pool_destroy(binding->chunk_pool);
>  
> -	dma_buf_unmap_attachment_unlocked(binding->attachment, binding->sgt,
> -					  binding->direction);
> -	dma_buf_detach(binding->dmabuf, binding->attachment);
> +	mutex_lock(&binding->lock);
> +	dev = binding->dev;
> +	if (dev)
> +		netdev_hold(dev, &dev_tracker, GFP_KERNEL);
> +	mutex_unlock(&binding->lock);
> +
> +	if (dev)
> +		netdev_lock(dev);
> +	mutex_lock(&binding->lock);
> +	net_devmem_dmabuf_binding_unmap(binding);
> +	mutex_unlock(&binding->lock);
> +	xa_erase(&net_devmem_dmabuf_bindings, binding->id);
> +	if (dev) {
> +		netdev_unlock(dev);
> +		netdev_put(dev, &dev_tracker);
> +	}

[Severity: High]
For a TX binding (NETDEV_CMD_BIND_TX, DMA_TO_DEVICE), can this code touch
a net_device that has already been freed?

At this commit nothing clears binding->dev or unmaps a TX binding when its
device is unregistered:

- The unmap in net_devmem_unbind_dmabuf() only runs for DMA_FROM_DEVICE.
- net_devmem_dev_unregister_rx() only matches DMA_FROM_DEVICE.
- mp_dmabuf_devmem_uninstall() only runs for RX queues.

Here is the sequence. The netlink socket is closed while skbs waiting for
retransmit still hold binding refs. The device is then unregistered and
freed. When the last skb is freed, this worker is scheduled.

netdev_hold() and netdev_lock() would then run on the freed net_device.
After that comes the original dma_unmap_sg_attrs() use-after-free on the
freed dma_dev.

The later commit "net: devmem: detach TX bindings on NETDEV_UNREGISTER" in
this series adds net_devmem_dev_unregister_tx() and seems to fix this, so
the problem only exists in the tree state at this commit.

> +
>  	dma_buf_put(binding->dmabuf);
>  	xa_destroy(&binding->bound_rxqs);
>  	percpu_ref_exit(&binding->ref);

[ ... ]

> @@ -147,12 +183,62 @@ void net_devmem_unbind_dmabuf(struct net_devmem_dmabuf_binding *binding)

[ ... ]

> +static void net_devmem_dev_detach(struct net_devmem_dmabuf_binding *binding,
> +				  struct net_device *dev)
> +{
> +	mutex_lock(&binding->lock);
> +	WRITE_ONCE(binding->dev, NULL);
> +	if (!netif_is_queue_leasee(dev))
> +		net_devmem_dmabuf_binding_unmap(binding);
> +	mutex_unlock(&binding->lock);
> +}
> +
> +void net_devmem_dev_unregister(struct net_device *dev)
> +{
> +	struct net_devmem_dmabuf_binding *binding;
> +	unsigned long xa_idx;
> +
> +	netdev_assert_locked(dev);
> +
> +	if (dev->reg_state != NETREG_UNREGISTERING)
> +		return;
> +
> +	xa_lock(&net_devmem_dmabuf_bindings);
> +	xa_for_each(&net_devmem_dmabuf_bindings, xa_idx, binding) {
> +		if (!net_devmem_dev_unregister_rx(binding, xa_idx, dev))
> +			continue;
> +
> +		xa_unlock(&net_devmem_dmabuf_bindings);
> +		net_devmem_dev_detach(binding, dev);
> +		xa_lock(&net_devmem_dmabuf_bindings);

[Severity: High]
Can the free worker kfree() the binding while this thread is still inside
mutex_unlock(&binding->lock) in net_devmem_dev_detach()?

After xa_lock is dropped, nothing pins the binding. The commit message
relies on __net_devmem_dmabuf_binding_free() blocking on netdev_lock(dev).
That only happens if the worker reads binding->dev before the walk clears
it.

Take a binding that is already unbound (percpu_ref_kill() has run) but is
still referenced by page pools, RX tokens or skbs on other CPUs:

CPU1 (NETDEV_UNREGISTER, holds netdev_lock(dev))
net_devmem_dev_unregister()
  xa_unlock(&net_devmem_dmabuf_bindings);
  net_devmem_dev_detach()
    mutex_lock(&binding->lock);
    WRITE_ONCE(binding->dev, NULL);
    net_devmem_dmabuf_binding_unmap(binding);
    mutex_unlock(&binding->lock);
      (still inside __mutex_unlock_slowpath())

CPU2 (last ref dropped, worker)
__net_devmem_dmabuf_binding_free()
  mutex_lock(&binding->lock);   /* acquired once CPU1 releases owner */
  dev = binding->dev;           /* NULL, so netdev_lock() is skipped */
  mutex_unlock(&binding->lock);
  ...
  xa_erase(&net_devmem_dmabuf_bindings, binding->id);
  ...
  kfree(binding);

Documentation/locking/mutex-design.rst notes that mutex_unlock() may still
access the mutex structure after it has internally released the lock. So
another context that acquires the mutex cannot assume the unlocker is done
with it.

Should the walk take a reference on the binding (for example with
percpu_ref_tryget()) before it drops the xarray lock?

This is still present at the end of the series. After "net: devmem: detach
TX bindings on NETDEV_UNREGISTER", the walk also detaches TX bindings that
only in-flight skbs keep alive, so the window gets wider.

> +	}
> +	xa_unlock(&net_devmem_dmabuf_bindings);
> +}

[ ... ]

> @@ -529,16 +615,19 @@ static void mp_dmabuf_devmem_uninstall(void *mp_priv,

[ ... ]

> +		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);
> +			if (!rxq->lease)
> +				net_devmem_dmabuf_binding_unmap(binding);
> +			mutex_unlock(&binding->lock);

[Severity: High]
Does this leave the dma_buf attachment mapped, with no path left that
unmaps it before the physical NIC goes away?

When bind-rx targets a leased queue on a virtual device such as netkit,
binding->dev is the virtual device. The attachment, however, is mapped for
the physical NIC's dma_dev, which comes from netdev_nl_get_dma_dev().

On netkit teardown, netkit_uninit()->netdev_rx_queue_unlease()->
netif_rxq_cleanup_unlease() does:

	__netif_mp_close_rxq(phys_rxq->dev, rxq_idx, &p);
	__netif_mp_uninstall_rxq(virt_rxq, &p);

There, the new return value of __netif_mp_close_rxq() is discarded. Also,
virt_rxq->lease is only cleared after netif_rxq_cleanup_unlease() returns.
So on this path binding->dev is set to NULL and the unmap is always
skipped. net_devmem_dev_detach() does the same when
netif_is_queue_leasee(dev) is true.

After that, net_devmem_dev_unregister() for the physical NIC cannot match
the binding, because binding->dev is NULL. net_devmem_unbind_dmabuf() also
skips the unmap for the same reason.

Suppose the physical NIC is then removed and the netlink socket is closed.
Wouldn't __net_devmem_dmabuf_binding_free() unmap the attachment against
the freed physical dma_dev? That is the dma_unmap_sg_attrs()
use-after-free described in the commit message.

Could netif_rxq_cleanup_unlease() use the result of __netif_mp_close_rxq()
to decide whether it is safe to unmap here? This looks unchanged at the end
of the series.

> +		}
> +		break;
>  	}
>  }

> diff --git a/net/core/devmem.h b/net/core/devmem.h
> index 4a293a7d1149c..a834e0f698c7e 100644
> --- a/net/core/devmem.h
> +++ b/net/core/devmem.h

[ ... ]

> @@ -38,7 +38,8 @@ struct net_devmem_dmabuf_binding {
>  	 *
>  	 * The binding undos itself and unmaps the underlying dmabuf once all
>  	 * those refs are dropped and the binding is no longer desired or in
> -	 * use.
> +	 * use. RX bindings are unmapped earlier, once they are no longer
> +	 * installed on any queue.

[Severity: Low]
This isn't a bug, but is this comment accurate? Several paths leave an RX
binding mapped after it is no longer installed on any queue:

- mp_dmabuf_devmem_uninstall() skips the unmap when rxq->lease is set.
- net_devmem_unbind_dmabuf() skips it when a queue close failed (busy) or
  when binding->dev is already NULL.
- net_devmem_dev_detach() skips it for queue leasees.

In those cases the unmap only happens in __net_devmem_dmabuf_binding_free().
Should the comment mention these exceptions?

[ ... ]

> diff --git a/net/core/netdev_rx_queue.c b/net/core/netdev_rx_queue.c
> index 00a7011eb4d57..a7261fedd5e76 100644
> --- a/net/core/netdev_rx_queue.c
> +++ b/net/core/netdev_rx_queue.c

[ ... ]

> @@ -289,35 +289,41 @@ static void __netif_mp_close_rxq(struct net_device *dev, unsigned int rxq_idx,

[ ... ]

>  	err = netdev_rx_queue_reconfig(dev, rxq_idx, &qcfg[0], &qcfg[1]);
> -	WARN_ON(err && err != -ENETDOWN);
> +	if (err == -ENETDOWN)
> +		return 0;
> +	WARN_ON(err);
> +	return err;
>  }

[Severity: Medium]
This isn't a bug introduced by this patch, but netif_mp_close_rxq() now
reports a failed queue restart, and only devmem acts on that. In
io_uring/zcrx.c, io_close_queue() still ignores the result and unmaps
unconditionally:

		if (ifq->if_rxq != -1)
			netif_mp_close_rxq(netdev, ifq->if_rxq, &p);

		scoped_guard(mutex, &ifq->pp_lock)
			io_zcrx_unmap_areas(ifq);

The reconfiguration can fail, for example with -ENOMEM from
ndo_queue_mem_alloc, and the old queue can keep running on the zcrx areas.
Could the device then DMA into areas that were just unmapped? io_uring
behaved the same way before this patch.

[ ... ]

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261015163915.1166043-1-almasrymina%40google.com

[PATCH net v4 2/4] net: devmem: detach TX bindings on NETDEV_UNREGISTER

[Mina Almasry]

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

Extend net_devmem_dev_unregister() to TX bindings, including ones
whose netlink socket is already closed but which TCP retransmits keep
alive:
- When binding->dev (the physical DMA-capable device) unregisters,
  clear the binding's NET_DEVMEM_BINDING_LIVE mark and both
  binding->dev and binding->vdev, and unmap the dma_buf under
  binding->lock. No grace period is needed: the device is already
  closed, so nothing hands the DMA addresses to the hardware anymore,
  and concurrent senders only compare against binding->dev.
- 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")
Suggested-by: Stanislav Fomichev <sdf@fomichev.me>
Cc: Kaiyuan Zhang <kaiyuanz@google.com>
Cc: Bobby Eshleman <bobbyeshleman@meta.com>
Cc: Tariq Toukan <tariqt@nvidia.com>
Cc: Kaifeng Wang <kaifengw@google.com>
Assisted-by: LLM
Signed-off-by: Mina Almasry <almasrymina@google.com>
Reviewed-by: Dragos Tatulea <dtatulea@nvidia.com>
---
v4:
- Detach and unmap TX bindings directly from the xarray walk, without a
  grace period or a second pass (Stanislav).

v3:
- Detach all matching TX bindings first, then call synchronize_net()
  once before unmapping them, instead of once per binding. Don't wait
  for a grace period when only binding->vdev is cleared, since vdev is
  never dereferenced (Sashiko).
- Don't detach TX bindings when the device only moves to another netns
  (Sashiko).

v2:
- Build on the net_devmem_dev_unregister() walk from the previous patch,
  so unbound TX bindings are also detached (Sashiko). Clear the
  binding's LIVE mark instead of erasing its ID.
- The walk holds net_devmem_lock, which the final free also takes
  before erasing the binding, instead of using RCU and
  percpu_ref_tryget(), so it no longer takes a binding reference
  (Stanislav).
---
 net/core/devmem.c | 22 +++++++++++++++++++++-
 net/core/devmem.h |  3 ++-
 2 files changed, 23 insertions(+), 2 deletions(-)

diff --git a/net/core/devmem.c b/net/core/devmem.c
index 20d95b24d5951..d33a092c6e4bb 100644
--- a/net/core/devmem.c
+++ b/net/core/devmem.c
@@ -207,11 +207,30 @@ net_devmem_dev_unregister_rx(struct net_devmem_dmabuf_binding *binding,
 			    NET_DEVMEM_BINDING_LIVE);
 }
 
+static bool
+net_devmem_dev_unregister_tx(struct net_devmem_dmabuf_binding *binding,
+			     unsigned long xa_idx, struct net_device *dev)
+{
+	if (binding->direction != DMA_TO_DEVICE)
+		return false;
+
+	if (READ_ONCE(binding->vdev) == dev)
+		WRITE_ONCE(binding->vdev, NULL);
+
+	if (READ_ONCE(binding->dev) != dev)
+		return false;
+
+	__xa_clear_mark(&net_devmem_dmabuf_bindings, xa_idx,
+			NET_DEVMEM_BINDING_LIVE);
+	return true;
+}
+
 static void net_devmem_dev_detach(struct net_devmem_dmabuf_binding *binding,
 				  struct net_device *dev)
 {
 	mutex_lock(&binding->lock);
 	WRITE_ONCE(binding->dev, NULL);
+	WRITE_ONCE(binding->vdev, NULL);
 	if (!netif_is_queue_leasee(dev))
 		net_devmem_dmabuf_binding_unmap(binding);
 	mutex_unlock(&binding->lock);
@@ -229,7 +248,8 @@ void net_devmem_dev_unregister(struct net_device *dev)
 
 	xa_lock(&net_devmem_dmabuf_bindings);
 	xa_for_each(&net_devmem_dmabuf_bindings, xa_idx, binding) {
-		if (!net_devmem_dev_unregister_rx(binding, xa_idx, dev))
+		if (!net_devmem_dev_unregister_rx(binding, xa_idx, dev) &&
+		    !net_devmem_dev_unregister_tx(binding, xa_idx, dev))
 			continue;
 
 		xa_unlock(&net_devmem_dmabuf_bindings);
diff --git a/net/core/devmem.h b/net/core/devmem.h
index a834e0f698c7e..ce069e1736416 100644
--- a/net/core/devmem.h
+++ b/net/core/devmem.h
@@ -39,7 +39,8 @@ struct net_devmem_dmabuf_binding {
 	 * The binding undos itself and unmaps the underlying dmabuf once all
 	 * those refs are dropped and the binding is no longer desired or in
 	 * use. RX bindings are unmapped earlier, once they are no longer
-	 * installed on any queue.
+	 * installed on any queue. TX bindings are unmapped earlier if their
+	 * device is unregistered.
 	 *
 	 * net_devmem_get_net_iov() on dmabuf net_iovs will increment this
 	 * reference, making sure that the binding remains alive until all the
-- 
2.56.0.176.g3575e65682-goog

Re: [PATCH net v4 2/4] net: devmem: detach TX bindings on NETDEV_UNREGISTER

[Stanislav Fomichev]

On 10/15, 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
> 
> Extend net_devmem_dev_unregister() to TX bindings, including ones
> whose netlink socket is already closed but which TCP retransmits keep
> alive:
> - When binding->dev (the physical DMA-capable device) unregisters,
>   clear the binding's NET_DEVMEM_BINDING_LIVE mark and both
>   binding->dev and binding->vdev, and unmap the dma_buf under
>   binding->lock. No grace period is needed: the device is already
>   closed, so nothing hands the DMA addresses to the hardware anymore,
>   and concurrent senders only compare against binding->dev.
> - 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")
> Suggested-by: Stanislav Fomichev <sdf@fomichev.me>
> Cc: Kaiyuan Zhang <kaiyuanz@google.com>
> Cc: Bobby Eshleman <bobbyeshleman@meta.com>
> Cc: Tariq Toukan <tariqt@nvidia.com>
> Cc: Kaifeng Wang <kaifengw@google.com>
> Assisted-by: LLM
> Signed-off-by: Mina Almasry <almasrymina@google.com>
> Reviewed-by: Dragos Tatulea <dtatulea@nvidia.com>
> ---
> v4:
> - Detach and unmap TX bindings directly from the xarray walk, without a
>   grace period or a second pass (Stanislav).

Acked-by: Stanislav Fomichev <sdf@fomichev.me>

Re: [PATCH net v4 2/4] net: devmem: detach TX bindings on NETDEV_UNREGISTER

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 3 potential
issue(s) to consider.

Critical: 0 · High: 2 · Medium: 0 · Low: 1

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [High] The fix leaves a race open on the leased path (NETMEM_TX_NO_DMA,
  e.g. netkit), which is the path added by 1abe839b34ae and named in…
- [High] net_devmem_dev_unregister() now also visits TX bindings whose
  percpu_ref has already reached zero, i.e. bindings whose…
- [Low] This patch makes binding->vdev writable after the binding is
  created, but the locking comment in struct net_devmem_dmabuf_binding…

--

net: devmem: detach TX bindings on NETDEV_UNREGISTER

This patch extends net_devmem_dev_unregister() to cover TX bindings.
When binding->dev unregisters, the binding loses its LIVE mark, dev and
vdev are cleared, and the dma_buf is unmapped. When only binding->vdev
unregisters, only vdev is cleared.

> diff --git a/net/core/devmem.c b/net/core/devmem.c
> index 20d95b24d5951..d33a092c6e4bb 100644
> --- a/net/core/devmem.c
> +++ b/net/core/devmem.c
> @@ -207,11 +207,30 @@ net_devmem_dev_unregister_rx(struct net_devmem_dmabuf_binding *binding,
>  			    NET_DEVMEM_BINDING_LIVE);
>  }
>  
> +static bool
> +net_devmem_dev_unregister_tx(struct net_devmem_dmabuf_binding *binding,
> +			     unsigned long xa_idx, struct net_device *dev)
> +{
> +	if (binding->direction != DMA_TO_DEVICE)
> +		return false;
> +
> +	if (READ_ONCE(binding->vdev) == dev)
> +		WRITE_ONCE(binding->vdev, NULL);

[Severity: Low]
This isn't a bug, but vdev can now change after the binding is
created. The lock comment in struct net_devmem_dmabuf_binding in
net/core/devmem.h still says:

	/* Protect dev, attachment and sgt */
	struct mutex lock;

Here vdev is written with only the net_devmem_dmabuf_bindings xa_lock
held. net_devmem_dev_detach() writes it under binding->lock. Readers
such as net_devmem_get_binding() read it with no lock at all.

Could the comment be updated to describe how vdev is synchronized? As
far as I can tell it is written with WRITE_ONCE, only ever set to
NULL, and only used for comparisons.

> +
> +	if (READ_ONCE(binding->dev) != dev)
> +		return false;

[Severity: High]
On the leased path, can a TX binding still be created on the physical
device after that device's unregister walk has finished, leaving
binding->dev dangling?

The setup is a physical device P (NETMEM_TX_DMA) with a queue leased
to a netkit device N, and P being unregistered:

unregister_netdevice_many_notify(P)
  P->reg_state = NETREG_UNREGISTERING
  NETDEV_UNREGISTER notifiers:
    netdev_genl_netdevice_event()     (subsys_initcall, runs first)
      netdev_lock(P)
      net_devmem_dev_unregister(P)
      netdev_unlock(P)
    ... other notifiers ...
    netkit notifier                   (module_init, runs later)
      netkit_check_lease_unregister()
        unregister_netdevice_many(N)

During this window N is still NETREG_REGISTERED. An unprivileged
NETDEV_CMD_BIND_TX on N therefore passes netdev_get_by_index_lock().
For P, netdev_find_netmem_tx_dev() checks only this:

		if (netif_device_present(phys_dev) &&
		    phys_dev->netmem_tx == NETMEM_TX_DMA)
			return phys_dev;

Unregistration does not clear __LINK_STATE_PRESENT. After that,
netdev_nl_bind_tx_doit() does:

	if (bind_dev != netdev)
		netdev_lock(bind_dev);
	...
	binding = net_devmem_bind_dmabuf(bind_dev,
					 bind_dev != netdev ? netdev : NULL,
					 ...

Nothing re-checks bind_dev->reg_state, so the code inserts a LIVE
binding with binding->dev == P and maps the dma_buf.

When N unregisters later, this function clears vdev and then returns
false at this check, because binding->dev is P and not N. P is freed
in netdev_run_todo() once netkit_uninit() drops the lease reference.
Any later NETDEV_UNREGISTER rebroadcast happens with
NETREG_UNREGISTERED, so net_devmem_dev_unregister() returns early and
never sees the binding.

When the socket closes, netdev_nl_sock_priv_destroy() and
__net_devmem_dmabuf_binding_free() call netdev_hold() and
netdev_lock() on the freed P. That matches the oops in the commit
message, but on the leased path named in the second Fixes: tag. The
dma_buf also stays mapped against the removed device. Nothing later
in the series seems to touch net/core.

The direct case looks safe, because netdev_get_by_index_lock() rejects
a device that is no longer registered. Would it make sense to reject
bind_dev->reg_state != NETREG_REGISTERED (or bind_dev->moving_ns)
after netdev_lock(bind_dev) in netdev_nl_bind_tx_doit()?

> +
> +	__xa_clear_mark(&net_devmem_dmabuf_bindings, xa_idx,
> +			NET_DEVMEM_BINDING_LIVE);
> +	return true;
> +}
> +
>  static void net_devmem_dev_detach(struct net_devmem_dmabuf_binding *binding,
>  				  struct net_device *dev)
>  {
>  	mutex_lock(&binding->lock);
>  	WRITE_ONCE(binding->dev, NULL);
> +	WRITE_ONCE(binding->vdev, NULL);
>  	if (!netif_is_queue_leasee(dev))
>  		net_devmem_dmabuf_binding_unmap(binding);
>  	mutex_unlock(&binding->lock);

[ ... ]

> @@ -229,7 +248,8 @@ void net_devmem_dev_unregister(struct net_device *dev)
>  
>  	xa_lock(&net_devmem_dmabuf_bindings);
>  	xa_for_each(&net_devmem_dmabuf_bindings, xa_idx, binding) {
> -		if (!net_devmem_dev_unregister_rx(binding, xa_idx, dev))
> +		if (!net_devmem_dev_unregister_rx(binding, xa_idx, dev) &&
> +		    !net_devmem_dev_unregister_tx(binding, xa_idx, dev))
>  			continue;
>  
>  		xa_unlock(&net_devmem_dmabuf_bindings);

[Severity: High]
Can the binding be freed while net_devmem_dev_detach() is still inside
mutex_unlock(&binding->lock)?

The walk now also selects TX bindings whose percpu_ref may already
have reached zero. An example is a binding whose netlink socket is
closed and whose last net_iov reference was dropped when P's
dev_close() or dev_shutdown() freed the TX skbs. For such a binding,
__net_devmem_dmabuf_binding_free() may be pending or running. The
walk takes no reference before xa_unlock() and the call to
net_devmem_dev_detach().

The free worker decides whether to serialize with the walk by looking
at binding->dev:

	mutex_lock(&binding->lock);
	dev = binding->dev;
	if (dev)
		netdev_hold(dev, &dev_tracker, GFP_KERNEL);
	mutex_unlock(&binding->lock);

	if (dev)
		netdev_lock(dev);
	...
	xa_erase(&net_devmem_dmabuf_bindings, binding->id);
	...
	kfree(binding);

Suppose the worker blocks on that first mutex_lock() while detach
holds the lock (dma_buf_unmap_attachment_unlocked() can sleep). When
it gets the lock it reads dev == NULL, skips netdev_lock(), and goes
on to kfree(binding).

__mutex_unlock_slowpath() releases the owner field before it takes
lock->wait_lock. If the worker acquires the mutex in that gap, the
unlocking thread then takes wait_lock inside freed memory. The
mutex_unlock() kerneldoc forbids this:

  mutex_unlock() can NOT directly be used to release an object such
  that another concurrent task can free it.

The previous patch in the series ("net: devmem: unmap dma_buf
synchronously on queue uninstall") relies on the worker taking
binding->dev's instance lock to keep a walked binding alive. That
protection is gone once detach clears dev and releases the mutex.

Should the walk and the final free be serialized by something that
does not depend on binding->dev, for example a global mutex held by
both across the whole detach?

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261015163915.1166043-1-almasrymina%40google.com

[PATCH net v4 3/4] netdevsim: support devmem RX and TX bindings

[Mina Almasry]

Set a 64-bit DMA mask on nsim_bus_dev, enable NETMEM_TX_DMA on the
netdevsim net_device, and configure page_pool instances with
PP_FLAG_ALLOW_UNREADABLE_NETMEM and queue_idx, plus PP_FLAG_DMA_MAP |
PP_FLAG_DMA_SYNC_DEV when CONFIG_HAS_DMA is set, so devmem bind-rx and
bind-tx netlink operations can be tested against netdevsim in software
selftests.

The pp_hold debugfs file allocates pages from the queue 0 page pool, so
refuse it when that pool is backed by a memory provider, and take the
netdev instance lock there so a concurrent bind-rx cannot swap the pool
in between. The software PSP path checksums the payload when the peer
does not decapsulate, so drop unreadable skbs there.

Cc: Tariq Toukan <tariqt@nvidia.com>
Cc: Kaifeng Wang <kaifengw@google.com>
Assisted-by: LLM
Signed-off-by: Mina Almasry <almasrymina@google.com>
Reviewed-by: Dragos Tatulea <dtatulea@nvidia.com>
---
v3:
- Only request page pool DMA mapping when CONFIG_HAS_DMA is set
  (Sashiko).
- Refuse pp_hold on a page pool backed by a memory provider, under the
  netdev instance lock (Sashiko).
- Drop unreadable skbs in the software PSP path when it would have to
  checksum the payload (Sashiko).
---
 drivers/net/netdevsim/bus.c    |  1 +
 drivers/net/netdevsim/netdev.c | 27 +++++++++++++++++++++------
 drivers/net/netdevsim/psp.c    |  5 +++++
 3 files changed, 27 insertions(+), 6 deletions(-)

diff --git a/drivers/net/netdevsim/bus.c b/drivers/net/netdevsim/bus.c
index 5c55c308487b4..f87f5e037b6fb 100644
--- a/drivers/net/netdevsim/bus.c
+++ b/drivers/net/netdevsim/bus.c
@@ -462,6 +462,7 @@ nsim_bus_dev_new(unsigned int id, unsigned int port_count, unsigned int num_queu
 	nsim_bus_dev->port_count = port_count;
 	nsim_bus_dev->num_queues = num_queues;
 	nsim_bus_dev->initial_net = current->nsproxy->net_ns;
+	dma_coerce_mask_and_coherent(&nsim_bus_dev->dev, DMA_BIT_MASK(64));
 	/* Disallow using nsim_bus_dev */
 	smp_store_release(&nsim_bus_dev->init, false);
 
diff --git a/drivers/net/netdevsim/netdev.c b/drivers/net/netdevsim/netdev.c
index b4a99f3ceac60..357ea9e5623b3 100644
--- a/drivers/net/netdevsim/netdev.c
+++ b/drivers/net/netdevsim/netdev.c
@@ -414,19 +414,27 @@ static int nsim_poll(struct napi_struct *napi, int budget)
 	return done;
 }
 
-static int nsim_create_page_pool(struct page_pool **p, struct napi_struct *napi)
+static int nsim_create_page_pool(struct net_device *dev, struct page_pool **p,
+				 struct napi_struct *napi, int idx)
 {
+	struct netdevsim *ns = netdev_priv(dev);
 	struct page_pool_params params = {
 		.order = 0,
 		.pool_size = NSIM_RING_SIZE,
+		.flags = PP_FLAG_ALLOW_UNREADABLE_NETMEM,
+		.max_len = PAGE_SIZE,
 		.nid = NUMA_NO_NODE,
-		.dev = &napi->dev->dev,
+		.dev = &ns->nsim_bus_dev->dev,
 		.napi = napi,
 		.dma_dir = DMA_BIDIRECTIONAL,
-		.netdev = napi->dev,
+		.netdev = dev,
+		.queue_idx = idx,
 	};
 	struct page_pool *pool;
 
+	if (IS_ENABLED(CONFIG_HAS_DMA))
+		params.flags |= PP_FLAG_DMA_MAP | PP_FLAG_DMA_SYNC_DEV;
+
 	pool = page_pool_create(&params);
 	if (IS_ERR(pool))
 		return PTR_ERR(pool);
@@ -450,7 +458,7 @@ static int nsim_init_napi(struct netdevsim *ns)
 	for (i = 0; i < dev->num_rx_queues; i++) {
 		rq = ns->rq[i];
 
-		err = nsim_create_page_pool(&rq->page_pool, &rq->napi);
+		err = nsim_create_page_pool(dev, &rq->page_pool, &rq->napi, i);
 		if (err)
 			goto err_pp_destroy;
 	}
@@ -762,14 +770,16 @@ nsim_queue_mem_alloc(struct net_device *dev,
 	if (ns->rq_reset_mode == 1) {
 		if (!netif_running(ns->netdev))
 			return -ENETDOWN;
-		return nsim_create_page_pool(&qmem->pp, &ns->rq[idx]->napi);
+		return nsim_create_page_pool(dev, &qmem->pp,
+					     &ns->rq[idx]->napi, idx);
 	}
 
 	qmem->rq = nsim_queue_alloc();
 	if (!qmem->rq)
 		return -ENOMEM;
 
-	err = nsim_create_page_pool(&qmem->rq->page_pool, &qmem->rq->napi);
+	err = nsim_create_page_pool(dev, &qmem->rq->page_pool, &qmem->rq->napi,
+				    idx);
 	if (err)
 		goto err_free;
 
@@ -929,12 +939,15 @@ nsim_pp_hold_write(struct file *file, const char __user *data,
 		return ret;
 
 	rtnl_lock();
+	netdev_lock(ns->netdev);
 	ret = count;
 	if (val == !!ns->page)
 		goto exit;
 
 	if (!netif_running(ns->netdev) && val) {
 		ret = -ENETDOWN;
+	} else if (val && page_pool_is_unreadable(ns->rq[0]->page_pool)) {
+		ret = -EOPNOTSUPP;
 	} else if (val) {
 		ns->page = page_pool_dev_alloc_pages(ns->rq[0]->page_pool);
 		if (!ns->page)
@@ -946,6 +959,7 @@ nsim_pp_hold_write(struct file *file, const char __user *data,
 	}
 
 exit:
+	netdev_unlock(ns->netdev);
 	rtnl_unlock();
 	return ret;
 }
@@ -1051,6 +1065,7 @@ static int nsim_init_netdevsim(struct netdevsim *ns)
 	ns->netdev->netdev_ops = &nsim_netdev_ops;
 	ns->netdev->stat_ops = &nsim_stat_ops;
 	ns->netdev->queue_mgmt_ops = &nsim_queue_mgmt_ops;
+	ns->netdev->netmem_tx = NETMEM_TX_DMA;
 	netdev_lockdep_set_classes(ns->netdev);
 
 	err = nsim_udp_tunnels_info_create(ns->nsim_dev, ns->netdev);
diff --git a/drivers/net/netdevsim/psp.c b/drivers/net/netdevsim/psp.c
index 6b3532b5e360d..908cc0d391dc6 100644
--- a/drivers/net/netdevsim/psp.c
+++ b/drivers/net/netdevsim/psp.c
@@ -86,6 +86,11 @@ nsim_do_psp(struct sk_buff *skb, struct netdevsim *ns,
 		__wsum csum;
 		int udplen;
 
+		if (!skb_frags_readable(skb)) {
+			rc = SKB_DROP_REASON_PSP_OUTPUT;
+			goto out_unlock;
+		}
+
 		/* Do not decapsulate. Receive the skb with the udp and psp
 		 * headers still there as if this is a normal udp packet.
 		 * psp_dev_encapsulate() sets udp checksum to 0, so we need to
-- 
2.56.0.176.g3575e65682-goog

Re: [PATCH net v4 3/4] netdevsim: support devmem RX and TX bindings

[Stanislav Fomichev]

On 10/15, Mina Almasry wrote:
> Set a 64-bit DMA mask on nsim_bus_dev, enable NETMEM_TX_DMA on the
> netdevsim net_device, and configure page_pool instances with
> PP_FLAG_ALLOW_UNREADABLE_NETMEM and queue_idx, plus PP_FLAG_DMA_MAP |
> PP_FLAG_DMA_SYNC_DEV when CONFIG_HAS_DMA is set, so devmem bind-rx and
> bind-tx netlink operations can be tested against netdevsim in software
> selftests.
> 
> The pp_hold debugfs file allocates pages from the queue 0 page pool, so
> refuse it when that pool is backed by a memory provider, and take the
> netdev instance lock there so a concurrent bind-rx cannot swap the pool
> in between. The software PSP path checksums the payload when the peer
> does not decapsulate, so drop unreadable skbs there.
> 
> Cc: Tariq Toukan <tariqt@nvidia.com>
> Cc: Kaifeng Wang <kaifengw@google.com>
> Assisted-by: LLM
> Signed-off-by: Mina Almasry <almasrymina@google.com>
> Reviewed-by: Dragos Tatulea <dtatulea@nvidia.com>

Acked-by: Stanislav Fomichev <sdf@fomichev.me>

Re: [PATCH net v4 3/4] netdevsim: support devmem RX and TX bindings

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 1 potential
issue(s) to consider.

Critical: 0 · High: 1 · Medium: 0 (1) · Low: 0

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [High] On builds where PP_DMA_INDEX_BITS is 0, this patch leads to a
  use-after-free of the netdevsim bus device.

Pre-existing issues:
- [Medium] This problem wasn't introduced by this patch, but writing true
  to the pp_hold debugfs file of a running netdevsim VF port…

--

netdevsim: support devmem RX and TX bindings

This commit sets a 64-bit DMA mask on the netdevsim bus device, enables
NETMEM_TX_DMA, and creates netdevsim page pools with
PP_FLAG_ALLOW_UNREADABLE_NETMEM and queue_idx, plus DMA mapping when
CONFIG_HAS_DMA is set, so devmem bind-rx and bind-tx can be tested. It
also refuses pp_hold on memory-provider-backed pools under the netdev
instance lock, and drops unreadable skbs in the software PSP checksum
path.

> diff --git a/drivers/net/netdevsim/netdev.c b/drivers/net/netdevsim/netdev.c
> index b4a99f3ceac60..357ea9e5623b3 100644
> --- a/drivers/net/netdevsim/netdev.c
> +++ b/drivers/net/netdevsim/netdev.c
> @@ -414,19 +414,27 @@ static int nsim_poll(struct napi_struct *napi, int budget)
>  	return done;
>  }
>  
> -static int nsim_create_page_pool(struct page_pool **p, struct napi_struct *napi)
> +static int nsim_create_page_pool(struct net_device *dev, struct page_pool **p,
> +				 struct napi_struct *napi, int idx)
>  {
> +	struct netdevsim *ns = netdev_priv(dev);
>  	struct page_pool_params params = {
>  		.order = 0,
>  		.pool_size = NSIM_RING_SIZE,
> +		.flags = PP_FLAG_ALLOW_UNREADABLE_NETMEM,
> +		.max_len = PAGE_SIZE,
>  		.nid = NUMA_NO_NODE,
> -		.dev = &napi->dev->dev,
> +		.dev = &ns->nsim_bus_dev->dev,
>  		.napi = napi,
>  		.dma_dir = DMA_BIDIRECTIONAL,
> -		.netdev = napi->dev,
> +		.netdev = dev,
> +		.queue_idx = idx,
>  	};
>  	struct page_pool *pool;
>  
> +	if (IS_ENABLED(CONFIG_HAS_DMA))
> +		params.flags |= PP_FLAG_DMA_MAP | PP_FLAG_DMA_SYNC_DEV;
> +

[Severity: High]

Can this cause a use-after-free of the nsim_bus_dev struct device on
builds where PP_DMA_INDEX_BITS is 0?

The page pool doesn't take a reference on pool->p.dev. With
PP_FLAG_DMA_MAP now set, pages from these pools carry a DMA mapping
against &ns->nsim_bus_dev->dev.

PP_DMA_INDEX_BITS is 0 when POISON_POINTER_DELTA is 0 and PAGE_OFFSET is
not a suitably aligned compile-time constant. Some 32-bit and nommu
builds hit this, riscv nommu for example, where PAGE_OFFSET is
phys_ram_base. In that case the mapping is never recorded:

net/core/page_pool.c:page_pool_register_dma_index() {
    ...
	if (unlikely(!PP_DMA_INDEX_BITS))
		goto out;
    ...
}

so page_pool_scrub() can't unmap outstanding pages while the device is
still alive.

Suppose a page is held via pp_hold and the device is then deleted.
nsim_destroy() returns ns->page late on purpose ("Put this intentionally
late to exercise the orphaning path"), and the sequence looks like this:

nsim_destroy()
  nsim_queue_uninit()
    page_pool_destroy()
      page_pool_release()
        page_pool_scrub()     <- held page not tracked, stays mapped
      inflight == 1, release_dw scheduled
  page_pool_put_full_page(pp, ns->page, false)  <- page lands in pool->ring
nsim_bus_dev_del()
  device_unregister()
    nsim_bus_dev_release()    <- struct device freed

page_pool_release_retry()
  page_pool_release()
    ...
      page_pool_empty_ring()
        page_pool_return_netmem()
          __page_pool_release_netmem_dma()
            __page_pool_unmap_netmem_dma()
              dma_unmap_page_attrs(pool->p.dev, ...)  <- freed device

The existing nl_netdev.py page_pool test, which holds a page and then
removes the device, follows this sequence. Before this patch the pools
did no DMA mapping, so the late put never touched the device.

>  	pool = page_pool_create(&params);
>  	if (IS_ERR(pool))
>  		return PTR_ERR(pool);

[ ... ]

> @@ -929,12 +939,15 @@ nsim_pp_hold_write(struct file *file, const char __user *data,
>  		return ret;
>  
>  	rtnl_lock();
> +	netdev_lock(ns->netdev);
>  	ret = count;
>  	if (val == !!ns->page)
>  		goto exit;
>  
>  	if (!netif_running(ns->netdev) && val) {
>  		ret = -ENETDOWN;
> +	} else if (val && page_pool_is_unreadable(ns->rq[0]->page_pool)) {

[Severity: Medium]

This isn't a bug introduced by this patch, but can this dereference a
NULL ns->rq on VF ports?

nsim_create() creates the pp_hold file for both PF and VF ports. However,
nsim_init_netdevsim_vf() only sets nsim_vf_netdev_ops and registers the
netdev, so ns->rq stays NULL.

nsim_vf_netdev_ops has no ndo_open. __dev_open() still sets
__LINK_STATE_START, so after "ip link set up" on the VF the
netif_running() check above passes.

Writing 1 to pp_hold on a running VF port with no page held would then
evaluate ns->rq[0] with ns->rq == NULL. The previous
page_pool_dev_alloc_pages(ns->rq[0]->page_pool) call had the same
dereference.

> +		ret = -EOPNOTSUPP;
>  	} else if (val) {
>  		ns->page = page_pool_dev_alloc_pages(ns->rq[0]->page_pool);
>  		if (!ns->page)

[ ... ]

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261015163915.1166043-1-almasrymina%40google.com

Re: [PATCH net v4 3/4] netdevsim: support devmem RX and TX bindings

[Mina Almasry]

On Sun, Oct 18, 2026 at 9:01 AM <netdev-bot+sashiko@kernel.org> wrote:
>
> Thank you for your contribution! Sashiko AI review found 1 potential
> issue(s) to consider.
>
> Critical: 0 · High: 1 · Medium: 0 (1) · Low: 0
>
> If you believe none of the issues are legitimate, please reply to each
> issue of Medium severity or higher and explain why the AI is wrong.
> If any of them are legitimate and you plan to respin, please let patchwork
> know by including "pw-bot: cr" as a separate line at the end of your reply
> (one such reply per series is enough).
>
> - [High] On builds where PP_DMA_INDEX_BITS is 0, this patch leads to a
>   use-after-free of the netdevsim bus device.
>

Bro this is a pre-existing issue I think. My local sashiko also caught
this but it has to do with us not doing a get_dev or something when
PP_DMA_INDEX_BITS is 0. That's a very rare condition and unrelated to
this patch series I think.

-- 
Thanks,
Mina

[PATCH net v4 4/4] selftests: net: add devmem RX and TX netdev unregister tests

[Mina Almasry]

Add devmem_bind_rx_unregister_check and devmem_bind_tx_unregister_check
to nl_netdev.py to verify that unregistering a netdevsim device while a
netlink socket holds an active RX or TX devmem binding synchronously
detaches the dma_buf attachment and cleanly closes the netlink socket
without use-after-free or page faults.

Add the dma-buf, udmabuf and devmem options the tests need to
tools/testing/selftests/net/config.

Cc: Tariq Toukan <tariqt@nvidia.com>
Cc: Kaifeng Wang <kaifengw@google.com>
Assisted-by: LLM
Signed-off-by: Mina Almasry <almasrymina@google.com>
Reviewed-by: Dragos Tatulea <dtatulea@nvidia.com>
---
v3:
- Count attachments of the test's own dma-buf only, and skip instead of
  passing when debugfs bufinfo is not readable (Sashiko).
- Close the netlink socket after removing the device, clean up with
  defer(), and skip on EOPNOTSUPP from bind-rx/bind-tx (Sashiko).
- Replace the hardcoded UDMABUF_CREATE value with a helper that computes
  it for the running architecture (Sashiko, Dragos), and close the memfd
  and /dev/udmabuf fds on errors (Sashiko).
- Add the needed options to tools/testing/selftests/net/config (Sashiko).
---
 tools/testing/selftests/net/config       |   4 +
 tools/testing/selftests/net/nl_netdev.py | 125 ++++++++++++++++++++++-
 2 files changed, 126 insertions(+), 3 deletions(-)

diff --git a/tools/testing/selftests/net/config b/tools/testing/selftests/net/config
index 737e7e6327b37..1e21f0055a2c0 100644
--- a/tools/testing/selftests/net/config
+++ b/tools/testing/selftests/net/config
@@ -11,6 +11,7 @@ CONFIG_CRYPTO_ARIA=y
 CONFIG_CRYPTO_CHACHA20POLY1305=m
 CONFIG_CRYPTO_SHA1=y
 CONFIG_CRYPTO_SM4_GENERIC=y
+CONFIG_DEBUG_FS=y
 CONFIG_DEBUG_INFO_BTF=y
 CONFIG_DEBUG_INFO_BTF_MODULES=n
 CONFIG_DUMMY=y
@@ -76,6 +77,7 @@ CONFIG_NET_CLS_BPF=m
 CONFIG_NET_CLS_FLOWER=m
 CONFIG_NET_CLS_MATCHALL=m
 CONFIG_NET_CLS_U32=m
+CONFIG_NET_DEVMEM=y
 CONFIG_NETDEVSIM=m
 CONFIG_NET_DROP_MONITOR=m
 CONFIG_NETFILTER=y
@@ -120,6 +122,7 @@ CONFIG_OPENVSWITCH=m
 CONFIG_PAGE_POOL_STATS=y
 CONFIG_PSAMPLE=m
 CONFIG_RPS=y
+CONFIG_SYNC_FILE=y
 CONFIG_SYN_COOKIES=y
 CONFIG_SYSCTL=y
 CONFIG_SYSFS=y
@@ -131,6 +134,7 @@ CONFIG_TEST_BPF=m
 CONFIG_TLS=m
 CONFIG_TRACEPOINTS=y
 CONFIG_TUN=y
+CONFIG_UDMABUF=y
 CONFIG_USER_NS=y
 CONFIG_VETH=y
 CONFIG_VLAN_8021Q=y
diff --git a/tools/testing/selftests/net/nl_netdev.py b/tools/testing/selftests/net/nl_netdev.py
index ceb44c8e1fec5..988878b71358c 100755
--- a/tools/testing/selftests/net/nl_netdev.py
+++ b/tools/testing/selftests/net/nl_netdev.py
@@ -6,10 +6,14 @@ Tests for the netdev netlink family.
 """
 
 import errno
+import fcntl
+import mmap
+import os
+import struct
 from os import system
-from lib.py import ksft_run, ksft_exit
+from lib.py import ksft_run, ksft_exit, KsftFailEx, KsftSkipEx
 from lib.py import ksft_eq, ksft_ge, ksft_ne, ksft_raises, ksft_busy_wait
-from lib.py import NetdevFamily, NetdevSimDev, NlError, defer, ip
+from lib.py import EthtoolFamily, NetdevFamily, NetdevSimDev, NlError, defer, ip
 
 
 def empty_check(nf) -> None:
@@ -366,6 +370,119 @@ def page_pool_stats_ifindex_check(nf) -> None:
     ksft_eq(cm.exception.nl_msg.extack['bad-attr'], '.info.id')
 
 
+def _udmabuf_create_ioctl() -> int:
+    """Return UDMABUF_CREATE, _IOW('u', 0x42, struct udmabuf_create)."""
+    machine = os.uname().machine
+    if machine.startswith(("alpha", "mips", "ppc", "powerpc", "sparc")):
+        ioc_write = 4 << 29
+    elif machine.startswith("parisc"):
+        ioc_write = 2 << 30
+    else:
+        ioc_write = 1 << 30
+    return ioc_write | 24 << 16 | ord("u") << 8 | 0x42
+
+
+def _create_udmabuf(num_pages=64) -> int:
+    """Create a sealed memfd-backed udmabuf fd for devmem tests."""
+    if not os.path.exists("/dev/udmabuf"):
+        raise KsftSkipEx("/dev/udmabuf is not available")
+
+    size = num_pages * mmap.PAGESIZE
+    memfd = os.memfd_create("devmem-ksft", os.MFD_ALLOW_SEALING)
+    try:
+        os.ftruncate(memfd, size)
+        fcntl.fcntl(memfd, fcntl.F_ADD_SEALS, fcntl.F_SEAL_SHRINK)
+        devfd = os.open("/dev/udmabuf", os.O_RDWR)
+        try:
+            req = bytearray(struct.pack("IIQQ", memfd, 0, 0, size))
+            return fcntl.ioctl(devfd, _udmabuf_create_ioctl(), req)
+        finally:
+            os.close(devfd)
+    finally:
+        os.close(memfd)
+
+
+def _dmabuf_attach_count(dmabuf_fd) -> int:
+    """Return the number of devices attached to dmabuf_fd, from debugfs."""
+    path = "/sys/kernel/debug/dma_buf/bufinfo"
+    try:
+        with open(path, "r", encoding="utf-8") as f:
+            lines = f.read().splitlines()
+    except OSError as e:
+        raise KsftSkipEx(f"{path} is not readable: {e}") from e
+
+    ino = os.fstat(dmabuf_fd).st_ino
+    found = False
+    for line in lines:
+        fields = line.split("\t")
+        if not found:
+            found = (len(fields) == 7 and fields[5].isdigit() and
+                     int(fields[5]) == ino)
+        elif line.startswith("Total "):
+            return int(line.split()[1])
+    raise KsftFailEx(f"dma-buf {ino} not found in {path}")
+
+
+def _devmem_bind(bind, attrs) -> None:
+    """Run a devmem bind op, skip the test if the device can't do devmem."""
+    try:
+        bind(attrs)
+    except NlError as e:
+        if e.error == errno.EOPNOTSUPP:
+            raise KsftSkipEx(f"devmem binding not supported: {e}") from e
+        raise
+
+
+def devmem_bind_rx_unregister_check(_nf) -> None:
+    """Verify RX devmem bindings detach dma_buf synchronously on netdev unregister."""
+    dmabuf_fd = _create_udmabuf()
+    defer(os.close, dmabuf_fd)
+    nf_priv = NetdevFamily()
+    defer(nf_priv.close)
+    nsimdev = NetdevSimDev(queue_count=2)
+    defer(nsimdev.remove)
+    nsim = nsimdev.nsims[0]
+
+    ip(f"link set dev {nsim.ifname} up")
+    with EthtoolFamily() as ef:
+        ef.rings_set({"header": {"dev-index": nsim.ifindex},
+                      "tcp-data-split": "enabled"})
+    _devmem_bind(nf_priv.bind_rx, {
+        "ifindex": nsim.ifindex,
+        "fd": dmabuf_fd,
+        "queues": [{"id": 1, "type": "rx"}],
+    })
+    ksft_ge(_dmabuf_attach_count(dmabuf_fd), 1)
+
+    nsimdev.remove()
+    ksft_eq(_dmabuf_attach_count(dmabuf_fd), 0)
+
+    nf_priv.close()
+
+
+def devmem_bind_tx_unregister_check(_nf) -> None:
+    """Verify TX devmem bindings detach cleanly on netdev unregister without UAF."""
+    dmabuf_fd = _create_udmabuf()
+    defer(os.close, dmabuf_fd)
+    nf_priv = NetdevFamily()
+    defer(nf_priv.close)
+    nsimdev = NetdevSimDev(queue_count=2)
+    defer(nsimdev.remove)
+    nsim = nsimdev.nsims[0]
+
+    ip(f"link set dev {nsim.ifname} up")
+    _devmem_bind(nf_priv.bind_tx, {
+        "ifindex": nsim.ifindex,
+        "fd": dmabuf_fd,
+    })
+    ksft_ge(_dmabuf_attach_count(dmabuf_fd), 1)
+
+    nsimdev.remove()
+    ksft_eq(_dmabuf_attach_count(dmabuf_fd), 0)
+
+    nf_priv.close()
+
+
 def main() -> None:
     """ Ksft boiler plate main """
     nf = NetdevFamily()
@@ -379,7 +496,9 @@ def main() -> None:
               page_pool_check,
               page_pool_dump_ifindex,
               page_pool_ifindex_leak_check,
-              page_pool_stats_ifindex_check
+              page_pool_stats_ifindex_check,
+              devmem_bind_rx_unregister_check,
+              devmem_bind_tx_unregister_check
               ],
              args=(nf, ))
     ksft_exit()
-- 
2.56.0.176.g3575e65682-goog

Re: [PATCH net v4 4/4] selftests: net: add devmem RX and TX netdev unregister tests

[Stanislav Fomichev]

On 10/15, Mina Almasry wrote:
> Add devmem_bind_rx_unregister_check and devmem_bind_tx_unregister_check
> to nl_netdev.py to verify that unregistering a netdevsim device while a
> netlink socket holds an active RX or TX devmem binding synchronously
> detaches the dma_buf attachment and cleanly closes the netlink socket
> without use-after-free or page faults.
> 
> Add the dma-buf, udmabuf and devmem options the tests need to
> tools/testing/selftests/net/config.
> 
> Cc: Tariq Toukan <tariqt@nvidia.com>
> Cc: Kaifeng Wang <kaifengw@google.com>
> Assisted-by: LLM
> Signed-off-by: Mina Almasry <almasrymina@google.com>
> Reviewed-by: Dragos Tatulea <dtatulea@nvidia.com>

Acked-by: Stanislav Fomichev <sdf@fomichev.me>