[PATCH net-next 0/3] net: consolidate net_iov freelist and DMA handling

[PATCH net-next 0/3] net: consolidate net_iov freelist and DMA handling

[Stanislav Fomichev]

Follow up on the recently merged devmem gen_pool removal by consolidating
the remaining net_iov freelist and DMA-address handling.

- move shared freelist state and operations into net_iov_area
- switch devmem to public memory-provider helpers and reject DMA
  addresses that cannot be represented
- decode stored DMA addresses for TX on 32-bit systems with 64-bit
  dma_addr_t

Stanislav Fomichev (3):
  net: netmem: add net_iov_area freelist helpers
  net: devmem: use memory provider helpers for net_iovs
  net: devmem: decode DMA addresses for TX

 include/linux/skbuff.h          |  7 ++-
 include/net/netmem.h            | 55 ++++++++++++++++++++++-
 include/net/page_pool/helpers.h | 10 +----
 io_uring/zcrx.c                 | 25 ++++-------
 io_uring/zcrx.h                 |  4 --
 net/core/devmem.c               | 77 +++++++++++++++------------------
 net/core/devmem.h               |  7 ++-
 net/core/page_pool_priv.h       | 14 +-----
 8 files changed, 108 insertions(+), 91 deletions(-)

-- 
2.53.0-Meta

[PATCH net-next 1/3] net: netmem: add net_iov_area freelist helpers

[Stanislav Fomichev]

io_uring zero-copy receive and devmem both maintain a bounded LIFO for
net_iovs in a contiguous area. Store the freelist in struct net_iov_area
and provide common push and pop helpers.

Leave synchronization to area owners. Keep devmem's area adjacent to its
lock. Use u32 indices and counts, which halves devmem's freelist storage
on 64-bit systems. Reject devmem areas with more than U32_MAX entries
before narrowing the count. With 4 KiB chunks, the limit is almost 16 TiB.

Signed-off-by: Stanislav Fomichev <sdf@fomichev.me>
---
 include/net/netmem.h | 28 ++++++++++++++++++++++++++-
 io_uring/zcrx.c      | 25 +++++++++---------------
 io_uring/zcrx.h      |  4 ----
 net/core/devmem.c    | 46 ++++++++++++++++++++++----------------------
 net/core/devmem.h    |  7 +++----
 5 files changed, 62 insertions(+), 48 deletions(-)

diff --git a/include/net/netmem.h b/include/net/netmem.h
index bccacd21b6c3..da885d95ea63 100644
--- a/include/net/netmem.h
+++ b/include/net/netmem.h
@@ -101,10 +101,15 @@ struct net_iov {
 struct net_iov_area {
 	/* Array of net_iovs for this area. */
 	struct net_iov *niovs;
-	size_t num_niovs;
+
+	/* Stack of free net_iov indices. */
+	u32 *freelist;
 
 	/* Offset into the dma-buf where this chunk starts.  */
 	unsigned long base_virtual;
+
+	u32 num_niovs;
+	u32 free_count;
 };
 
 static inline struct net_iov_area *net_iov_owner(const struct net_iov *niov)
@@ -117,6 +122,27 @@ static inline unsigned int net_iov_idx(const struct net_iov *niov)
 	return niov - net_iov_owner(niov)->niovs;
 }
 
+static inline struct net_iov *net_iov_area_pop(struct net_iov_area *area)
+{
+	u32 idx;
+
+	if (unlikely(!area->free_count))
+		return NULL;
+
+	idx = area->freelist[--area->free_count];
+	return &area->niovs[idx];
+}
+
+static inline void net_iov_area_push(struct net_iov_area *area,
+				     struct net_iov *niov)
+{
+	if (WARN_ON_ONCE(net_iov_owner(niov) != area ||
+			 area->free_count >= area->num_niovs))
+		return;
+
+	area->freelist[area->free_count++] = net_iov_idx(niov);
+}
+
 /* Initialize a niov: stamp the owning area, the memory provider type.
  */
 static inline void net_iov_init(struct net_iov *niov,
diff --git a/io_uring/zcrx.c b/io_uring/zcrx.c
index 86d580d4410d..fa6061127b9d 100644
--- a/io_uring/zcrx.c
+++ b/io_uring/zcrx.c
@@ -462,7 +462,7 @@ static void io_zcrx_free_area(struct io_zcrx_ifq *ifq,
 		io_unaccount_mem(ifq->user, ifq->mm_account,
 				 area->mem.account_pages);
 
-	kvfree(area->freelist);
+	kvfree(area->nia.freelist);
 	kvfree(area->nia.niovs);
 	kvfree(area->user_refs);
 	kfree(area);
@@ -548,9 +548,10 @@ static int __zcrx_create_area(struct io_zcrx_ifq *ifq,
 	if (!area->nia.niovs)
 		goto err;
 
-	area->freelist = kvmalloc_array(nr_iovs, sizeof(area->freelist[0]),
-					GFP_KERNEL_ACCOUNT | __GFP_ZERO);
-	if (!area->freelist)
+	area->nia.freelist = kvmalloc_array(nr_iovs,
+					    sizeof(area->nia.freelist[0]),
+					    GFP_KERNEL_ACCOUNT | __GFP_ZERO);
+	if (!area->nia.freelist)
 		goto err;
 
 	area->user_refs = kvmalloc_objs(area->user_refs[0], nr_iovs,
@@ -562,7 +563,7 @@ static int __zcrx_create_area(struct io_zcrx_ifq *ifq,
 		struct net_iov *niov = &area->nia.niovs[i];
 
 		net_iov_init(niov, &area->nia, NET_IOV_IOURING);
-		area->freelist[i] = i;
+		area->nia.freelist[i] = i;
 		atomic_set(&area->user_refs[i], 0);
 	}
 
@@ -572,7 +573,7 @@ static int __zcrx_create_area(struct io_zcrx_ifq *ifq,
 			goto err;
 	}
 
-	area->free_count = nr_iovs;
+	area->nia.free_count = nr_iovs;
 	/* we're only supporting one area per ifq for now */
 	area->area_id = zcrx_next_area_id(ifq);
 	area_reg->rq_area_token = zcrx_area_id_to_token(area->area_id);
@@ -689,22 +690,14 @@ static void io_zcrx_return_niov_freelist(struct net_iov *niov)
 	struct io_zcrx_ifq *ifq = area->ifq;
 
 	guard(spinlock_bh)(&ifq->alloc_lock);
-	if (WARN_ON_ONCE(area->free_count >= area->nia.num_niovs))
-		return;
-	area->freelist[area->free_count++] = net_iov_idx(niov);
+	net_iov_area_push(&area->nia, niov);
 }
 
 static struct net_iov *zcrx_get_free_niov(struct io_zcrx_area *area)
 {
-	unsigned niov_idx;
-
 	lockdep_assert_held(&area->ifq->alloc_lock);
 
-	if (unlikely(!area->free_count))
-		return NULL;
-
-	niov_idx = area->freelist[--area->free_count];
-	return &area->nia.niovs[niov_idx];
+	return net_iov_area_pop(&area->nia);
 }
 
 static void io_zcrx_return_niov(struct net_iov *niov)
diff --git a/io_uring/zcrx.h b/io_uring/zcrx.h
index d4a54b4e17fd..a923291defde 100644
--- a/io_uring/zcrx.h
+++ b/io_uring/zcrx.h
@@ -36,10 +36,6 @@ struct io_zcrx_area {
 	bool			is_mapped;
 	u16			area_id;
 
-	/* freelist */
-	u32			free_count;
-	u32			*freelist;
-
 	struct io_zcrx_mem	mem;
 };
 
diff --git a/net/core/devmem.c b/net/core/devmem.c
index a9d86b5a5588..c1c1872b88de 100644
--- a/net/core/devmem.c
+++ b/net/core/devmem.c
@@ -42,10 +42,10 @@ void __net_devmem_dmabuf_binding_free(struct work_struct *wq)
 {
 	struct net_devmem_dmabuf_binding *binding = container_of(wq, typeof(*binding), unbind_w);
 
-	if (binding->freelist)
-		WARN(binding->free_count != binding->area.num_niovs,
-		     "destroying dmabuf binding with outstanding net_iovs: total=%zu, free=%zu",
-		     binding->area.num_niovs, binding->free_count);
+	if (binding->area.freelist)
+		WARN(binding->area.free_count != binding->area.num_niovs,
+		     "destroying dmabuf binding with outstanding net_iovs: total=%u, free=%u",
+		     binding->area.num_niovs, binding->area.free_count);
 
 	kvfree(binding->area.niovs);
 	dma_buf_unmap_attachment_unlocked(binding->attachment, binding->sgt,
@@ -54,7 +54,7 @@ void __net_devmem_dmabuf_binding_free(struct work_struct *wq)
 	dma_buf_put(binding->dmabuf);
 	xa_destroy(&binding->bound_rxqs);
 	percpu_ref_exit(&binding->ref);
-	kvfree(binding->freelist);
+	kvfree(binding->area.freelist);
 	kvfree(binding->tx_vec);
 	kfree(binding);
 }
@@ -67,12 +67,10 @@ net_devmem_alloc_dmabuf_bulk(struct net_devmem_dmabuf_binding *binding,
 
 	spin_lock_bh(&binding->freelist_lock);
 
-	count = min_t(size_t, count, binding->free_count);
-	for (i = 0; i < count; i++) {
-		struct net_iov *niov = binding->freelist[--binding->free_count];
-
-		netmems[i] = net_iov_to_netmem(niov);
-	}
+	count = min(count, binding->area.free_count);
+	for (i = 0; i < count; i++)
+		netmems[i] =
+			net_iov_to_netmem(net_iov_area_pop(&binding->area));
 
 	spin_unlock_bh(&binding->freelist_lock);
 
@@ -84,12 +82,7 @@ void net_devmem_free_dmabuf(struct net_iov *niov)
 	struct net_devmem_dmabuf_binding *binding = net_devmem_iov_binding(niov);
 
 	spin_lock_bh(&binding->freelist_lock);
-	if (WARN_ON_ONCE(binding->free_count >= binding->area.num_niovs)) {
-		spin_unlock_bh(&binding->freelist_lock);
-		return;
-	}
-
-	binding->freelist[binding->free_count++] = niov;
+	net_iov_area_push(&binding->area, niov);
 	spin_unlock_bh(&binding->freelist_lock);
 }
 
@@ -228,6 +221,12 @@ net_devmem_bind_dmabuf(struct net_device *dev, void *vdev,
 		goto err_unmap;
 	}
 
+	if ((dmabuf->size >> niov_shift) > U32_MAX) {
+		err = -E2BIG;
+		NL_SET_ERR_MSG(extack, "dmabuf contains too many net_iovs");
+		goto err_unmap;
+	}
+
 	binding->area.base_virtual = 0;
 	binding->area.num_niovs = dmabuf->size >> niov_shift;
 	if (direction == DMA_TO_DEVICE) {
@@ -239,10 +238,11 @@ net_devmem_bind_dmabuf(struct net_device *dev, void *vdev,
 		}
 	} else {
 		spin_lock_init(&binding->freelist_lock);
-		binding->freelist = kvmalloc_array(binding->area.num_niovs,
-						   sizeof(binding->freelist[0]),
-						   GFP_KERNEL);
-		if (!binding->freelist) {
+		binding->area.freelist =
+			kvmalloc_array(binding->area.num_niovs,
+				       sizeof(binding->area.freelist[0]),
+				       GFP_KERNEL);
+		if (!binding->area.freelist) {
 			err = -ENOMEM;
 			goto err_unmap;
 		}
@@ -279,7 +279,7 @@ net_devmem_bind_dmabuf(struct net_device *dev, void *vdev,
 			if (direction == DMA_TO_DEVICE)
 				binding->tx_vec[niov_idx] = niov;
 			else
-				binding->freelist[binding->free_count++] = niov;
+				net_iov_area_push(&binding->area, niov);
 			dma_addr += niov_size;
 		}
 	}
@@ -297,7 +297,7 @@ net_devmem_bind_dmabuf(struct net_device *dev, void *vdev,
 err_free_niovs:
 	kvfree(binding->area.niovs);
 err_free_freelist:
-	kvfree(binding->freelist);
+	kvfree(binding->area.freelist);
 	kvfree(binding->tx_vec);
 err_unmap:
 	dma_buf_unmap_attachment_unlocked(binding->attachment, binding->sgt,
diff --git a/net/core/devmem.h b/net/core/devmem.h
index f71d9a2bcb3d..5edb763b89b2 100644
--- a/net/core/devmem.h
+++ b/net/core/devmem.h
@@ -17,6 +17,9 @@ struct netlink_ext_ack;
 
 struct net_devmem_dmabuf_binding {
 	struct net_iov_area area;
+	/* Protects area.freelist and area.free_count. */
+	spinlock_t freelist_lock;
+
 	struct dma_buf *dmabuf;
 	struct dma_buf_attachment *attachment;
 	struct sg_table *sgt;
@@ -57,10 +60,6 @@ struct net_devmem_dmabuf_binding {
 	/* rxq's this binding is active on. */
 	struct xarray bound_rxqs;
 
-	spinlock_t freelist_lock ____cacheline_aligned_in_smp;
-	size_t free_count;
-	struct net_iov **freelist;
-
 	/* ID of this binding. Globally unique to all bindings currently
 	 * active.
 	 */
-- 
2.53.0-Meta

Re: [PATCH net-next 1/3] net: netmem: add net_iov_area freelist helpers

[netdev-bot+sashiko]

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

Critical: 0 · High: 0 · Medium: 1 (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).

- [Medium] net/core/devmem.h: the refactor moves `spinlock_t
  freelist_lock` from the tail of `struct net_devmem_dmabuf_binding`…

Pre-existing issues:
- [Medium] In `net_devmem_bind_dmabuf()` (net/core/devmem.c) the per-sg-
  entry population loop advances `niov_idx` over the whole mapped…

--

net: netmem: add net_iov_area freelist helpers

This moves the bounded LIFO freelist that io_uring zcrx and devmem each
maintained into struct net_iov_area, and adds net_iov_area_pop() and
net_iov_area_push() helpers. The freelist now stores u32 indices instead of
pointers, and devmem rejects areas with more than U32_MAX entries before
narrowing num_niovs to u32.

> diff --git a/include/net/netmem.h b/include/net/netmem.h
> index bccacd21b6c37..da885d95ea63b 100644
> --- a/include/net/netmem.h
> +++ b/include/net/netmem.h
> @@ -101,10 +101,15 @@ struct net_iov {
>  struct net_iov_area {
>  	/* Array of net_iovs for this area. */
>  	struct net_iov *niovs;
> -	size_t num_niovs;
> +
> +	/* Stack of free net_iov indices. */
> +	u32 *freelist;
>  
>  	/* Offset into the dma-buf where this chunk starts.  */
>  	unsigned long base_virtual;
> +
> +	u32 num_niovs;
> +	u32 free_count;
>  };
>  
>  static inline struct net_iov_area *net_iov_owner(const struct net_iov *niov)
> @@ -117,6 +122,27 @@ static inline unsigned int net_iov_idx(const struct net_iov *niov)
>  	return niov - net_iov_owner(niov)->niovs;
>  }
>  
> +static inline struct net_iov *net_iov_area_pop(struct net_iov_area *area)
> +{
> +	u32 idx;
> +
> +	if (unlikely(!area->free_count))
> +		return NULL;
> +
> +	idx = area->freelist[--area->free_count];
> +	return &area->niovs[idx];
> +}
> +
> +static inline void net_iov_area_push(struct net_iov_area *area,
> +				     struct net_iov *niov)
> +{
> +	if (WARN_ON_ONCE(net_iov_owner(niov) != area ||
> +			 area->free_count >= area->num_niovs))
> +		return;
> +
> +	area->freelist[area->free_count++] = net_iov_idx(niov);
> +}
> +

[ ... io_uring/zcrx.c, io_uring/zcrx.h snipped ... ]

> diff --git a/net/core/devmem.c b/net/core/devmem.c
> index a9d86b5a55885..c1c1872b88de2 100644
> --- a/net/core/devmem.c
> +++ b/net/core/devmem.c

[ ... ]

> @@ -67,12 +67,10 @@ net_devmem_alloc_dmabuf_bulk(struct net_devmem_dmabuf_binding *binding,
>  
>  	spin_lock_bh(&binding->freelist_lock);
>  
> -	count = min_t(size_t, count, binding->free_count);
> -	for (i = 0; i < count; i++) {
> -		struct net_iov *niov = binding->freelist[--binding->free_count];
> -
> -		netmems[i] = net_iov_to_netmem(niov);
> -	}
> +	count = min(count, binding->area.free_count);
> +	for (i = 0; i < count; i++)
> +		netmems[i] =
> +			net_iov_to_netmem(net_iov_area_pop(&binding->area));
>  
>  	spin_unlock_bh(&binding->freelist_lock);
>  
> @@ -84,12 +82,7 @@ void net_devmem_free_dmabuf(struct net_iov *niov)
>  	struct net_devmem_dmabuf_binding *binding = net_devmem_iov_binding(niov);
>  
>  	spin_lock_bh(&binding->freelist_lock);
> -	if (WARN_ON_ONCE(binding->free_count >= binding->area.num_niovs)) {
> -		spin_unlock_bh(&binding->freelist_lock);
> -		return;
> -	}
> -
> -	binding->freelist[binding->free_count++] = niov;
> +	net_iov_area_push(&binding->area, niov);
>  	spin_unlock_bh(&binding->freelist_lock);
>  }

[ ... ]

> @@ -279,7 +279,7 @@ net_devmem_bind_dmabuf(struct net_device *dev, void *vdev,
>  			if (direction == DMA_TO_DEVICE)
>  				binding->tx_vec[niov_idx] = niov;
>  			else
> -				binding->freelist[binding->free_count++] = niov;
> +				net_iov_area_push(&binding->area, niov);
>  			dma_addr += niov_size;
>  		}
>  	}

[Severity: Medium]
This isn't a bug introduced by this patch, but the conversion makes the
population loop in net_devmem_bind_dmabuf() look better validated than it
actually is.

The freelist store is now bounds-checked inside net_iov_area_push(), which
warns and drops the entry when free_count reaches num_niovs. The two
sibling writes in the very same iteration are still unbounded:

	nr_niovs = len >> niov_shift;
	for (i = 0; i < nr_niovs; i++, niov_idx++) {
		niov = &binding->area.niovs[niov_idx];
		net_iov_init(niov, &binding->area, NET_IOV_DMABUF);
		if (net_mp_niov_set_dma_addr(niov, dma_addr)) {
			...
		}
		if (direction == DMA_TO_DEVICE)
			binding->tx_vec[niov_idx] = niov;
		else
			net_iov_area_push(&binding->area, niov);

niov_idx accumulates across every sg entry and is never compared against
binding->area.num_niovs, which is derived only from
dmabuf->size >> niov_shift. The loop checks per-entry alignment
(IS_ALIGNED(dma_addr, niov_size), IS_ALIGNED(len, niov_size)) but never that
the summed sg length equals dmabuf->size.

Can area.niovs[] and tx_vec[] be written past their ends if an exporter's
mapped sg_table covers more bytes than dmabuf->size?

In the other direction, if the mapping is shorter than dmabuf->size, can the
tail of tx_vec be dereferenced as struct net_iov *? It is allocated with
kvmalloc_objs() and never zeroed:

	binding->tx_vec = kvmalloc_objs(struct net_iov *,
					binding->area.num_niovs);

I could not construct this with an in-tree exporter (udmabuf, dma-heaps and
DRM PRIME all map exactly the object size), and I did not find an explicit
dma-buf API guarantee that the mapped sg length equals dmabuf->size either.
Would it be worth bounding niov_idx against area.num_niovs, or zeroing
tx_vec, while the push side is being touched here?

This is still the case at the end of the series (3ee6ae62e2d2): the later
patch "net: devmem: use memory provider helpers for net_iovs" zeroes
area.niovs but leaves tx_vec unzeroed and niov_idx unbounded.

> diff --git a/net/core/devmem.h b/net/core/devmem.h
> index f71d9a2bcb3d3..5edb763b89b26 100644
> --- a/net/core/devmem.h
> +++ b/net/core/devmem.h
> @@ -17,6 +17,9 @@ struct netlink_ext_ack;
>  
>  struct net_devmem_dmabuf_binding {
>  	struct net_iov_area area;
> +	/* Protects area.freelist and area.free_count. */
> +	spinlock_t freelist_lock;
> +
>  	struct dma_buf *dmabuf;
>  	struct dma_buf_attachment *attachment;
>  	struct sg_table *sgt;
> @@ -57,10 +60,6 @@ struct net_devmem_dmabuf_binding {
>  	/* rxq's this binding is active on. */
>  	struct xarray bound_rxqs;
>  
> -	spinlock_t freelist_lock ____cacheline_aligned_in_smp;
> -	size_t free_count;
> -	struct net_iov **freelist;
> -

[Severity: Medium]
The ____cacheline_aligned_in_smp annotation on freelist_lock is dropped
here, and the lock moves from the tail of the binding to right after the
embedded struct net_iov_area. Was that intentional?

With the new area layout (niovs at 0, freelist at 8, base_virtual at 16,
num_niovs at 24, free_count at 28) the write-hot lock word and free_count
end up in the same 64-byte line as the read-mostly area.niovs and
area.base_virtual.

The writers dirty that line on every allocation and release:

net/core/devmem.c:net_devmem_alloc_dmabuf_bulk() {
	spin_lock_bh(&binding->freelist_lock);
	count = min(count, binding->area.free_count);
	...
}

net/core/devmem.c:net_devmem_free_dmabuf() {
	spin_lock_bh(&binding->freelist_lock);
	net_iov_area_push(&binding->area, niov);
	...
}

while the receive path reads the same line per fragment, via
net_iov_idx() -> net_iov_owner(niov)->niovs and
net_devmem_iov_virtual_addr() -> net_iov_owner(niov)->base_virtual.

In the common devmem setup, where page pool refill/release runs on a
different core than the recvmsg() consumer, does this re-introduce the false
sharing the removed annotation was there to prevent?

The commit message says:

    Keep devmem's area adjacent to its lock.

but it does not mention removing the SMP cacheline isolation, and no numbers
are given. Annotating freelist_lock (or the lock plus free_count pair)
____cacheline_aligned_in_smp again would keep the refactor intact.

I checked the end of the series (3ee6ae62e2d2) and net/core/devmem.h still
declares the lock right after the area with no ____cacheline_aligned_in_smp,
so nothing later restores it.

>  	/* ID of this binding. Globally unique to all bindings currently
>  	 * active.
>  	 */

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260922204348.717198-1-sdf%40fomichev.me

Re: [PATCH net-next 1/3] net: netmem: add net_iov_area freelist helpers

[Stanislav Fomichev]

On 09/23, netdev-bot+sashiko@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential
> issue(s) to consider.
> 
> Critical: 0 · High: 0 · Medium: 1 (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).
> 
> - [Medium] net/core/devmem.h: the refactor moves `spinlock_t
>   freelist_lock` from the tail of `struct net_devmem_dmabuf_binding`…
> 
> Pre-existing issues:
> - [Medium] In `net_devmem_bind_dmabuf()` (net/core/devmem.c) the per-sg-
>   entry population loop advances `niov_idx` over the whole mapped…
> 
> --
> 
> net: netmem: add net_iov_area freelist helpers
> 
> This moves the bounded LIFO freelist that io_uring zcrx and devmem each
> maintained into struct net_iov_area, and adds net_iov_area_pop() and
> net_iov_area_push() helpers. The freelist now stores u32 indices instead of
> pointers, and devmem rejects areas with more than U32_MAX entries before
> narrowing num_niovs to u32.
> 
> > diff --git a/include/net/netmem.h b/include/net/netmem.h
> > index bccacd21b6c37..da885d95ea63b 100644
> > --- a/include/net/netmem.h
> > +++ b/include/net/netmem.h
> > @@ -101,10 +101,15 @@ struct net_iov {
> >  struct net_iov_area {
> >  	/* Array of net_iovs for this area. */
> >  	struct net_iov *niovs;
> > -	size_t num_niovs;
> > +
> > +	/* Stack of free net_iov indices. */
> > +	u32 *freelist;
> >  
> >  	/* Offset into the dma-buf where this chunk starts.  */
> >  	unsigned long base_virtual;
> > +
> > +	u32 num_niovs;
> > +	u32 free_count;
> >  };
> >  
> >  static inline struct net_iov_area *net_iov_owner(const struct net_iov *niov)
> > @@ -117,6 +122,27 @@ static inline unsigned int net_iov_idx(const struct net_iov *niov)
> >  	return niov - net_iov_owner(niov)->niovs;
> >  }
> >  
> > +static inline struct net_iov *net_iov_area_pop(struct net_iov_area *area)
> > +{
> > +	u32 idx;
> > +
> > +	if (unlikely(!area->free_count))
> > +		return NULL;
> > +
> > +	idx = area->freelist[--area->free_count];
> > +	return &area->niovs[idx];
> > +}
> > +
> > +static inline void net_iov_area_push(struct net_iov_area *area,
> > +				     struct net_iov *niov)
> > +{
> > +	if (WARN_ON_ONCE(net_iov_owner(niov) != area ||
> > +			 area->free_count >= area->num_niovs))
> > +		return;
> > +
> > +	area->freelist[area->free_count++] = net_iov_idx(niov);
> > +}
> > +
> 
> [ ... io_uring/zcrx.c, io_uring/zcrx.h snipped ... ]
> 
> > diff --git a/net/core/devmem.c b/net/core/devmem.c
> > index a9d86b5a55885..c1c1872b88de2 100644
> > --- a/net/core/devmem.c
> > +++ b/net/core/devmem.c
> 
> [ ... ]
> 
> > @@ -67,12 +67,10 @@ net_devmem_alloc_dmabuf_bulk(struct net_devmem_dmabuf_binding *binding,
> >  
> >  	spin_lock_bh(&binding->freelist_lock);
> >  
> > -	count = min_t(size_t, count, binding->free_count);
> > -	for (i = 0; i < count; i++) {
> > -		struct net_iov *niov = binding->freelist[--binding->free_count];
> > -
> > -		netmems[i] = net_iov_to_netmem(niov);
> > -	}
> > +	count = min(count, binding->area.free_count);
> > +	for (i = 0; i < count; i++)
> > +		netmems[i] =
> > +			net_iov_to_netmem(net_iov_area_pop(&binding->area));
> >  
> >  	spin_unlock_bh(&binding->freelist_lock);
> >  
> > @@ -84,12 +82,7 @@ void net_devmem_free_dmabuf(struct net_iov *niov)
> >  	struct net_devmem_dmabuf_binding *binding = net_devmem_iov_binding(niov);
> >  
> >  	spin_lock_bh(&binding->freelist_lock);
> > -	if (WARN_ON_ONCE(binding->free_count >= binding->area.num_niovs)) {
> > -		spin_unlock_bh(&binding->freelist_lock);
> > -		return;
> > -	}
> > -
> > -	binding->freelist[binding->free_count++] = niov;
> > +	net_iov_area_push(&binding->area, niov);
> >  	spin_unlock_bh(&binding->freelist_lock);
> >  }
> 
> [ ... ]
> 
> > @@ -279,7 +279,7 @@ net_devmem_bind_dmabuf(struct net_device *dev, void *vdev,
> >  			if (direction == DMA_TO_DEVICE)
> >  				binding->tx_vec[niov_idx] = niov;
> >  			else
> > -				binding->freelist[binding->free_count++] = niov;
> > +				net_iov_area_push(&binding->area, niov);
> >  			dma_addr += niov_size;
> >  		}
> >  	}
> 
> [Severity: Medium]
> This isn't a bug introduced by this patch, but the conversion makes the
> population loop in net_devmem_bind_dmabuf() look better validated than it
> actually is.
> 
> The freelist store is now bounds-checked inside net_iov_area_push(), which
> warns and drops the entry when free_count reaches num_niovs. The two
> sibling writes in the very same iteration are still unbounded:
> 
> 	nr_niovs = len >> niov_shift;
> 	for (i = 0; i < nr_niovs; i++, niov_idx++) {
> 		niov = &binding->area.niovs[niov_idx];
> 		net_iov_init(niov, &binding->area, NET_IOV_DMABUF);
> 		if (net_mp_niov_set_dma_addr(niov, dma_addr)) {
> 			...
> 		}
> 		if (direction == DMA_TO_DEVICE)
> 			binding->tx_vec[niov_idx] = niov;
> 		else
> 			net_iov_area_push(&binding->area, niov);
> 
> niov_idx accumulates across every sg entry and is never compared against
> binding->area.num_niovs, which is derived only from
> dmabuf->size >> niov_shift. The loop checks per-entry alignment
> (IS_ALIGNED(dma_addr, niov_size), IS_ALIGNED(len, niov_size)) but never that
> the summed sg length equals dmabuf->size.

[..]

> Can area.niovs[] and tx_vec[] be written past their ends if an exporter's
> mapped sg_table covers more bytes than dmabuf->size?
> 
> In the other direction, if the mapping is shorter than dmabuf->size, can the
> tail of tx_vec be dereferenced as struct net_iov *? It is allocated with
> kvmalloc_objs() and never zeroed:
> 
> 	binding->tx_vec = kvmalloc_objs(struct net_iov *,
> 					binding->area.num_niovs);
> 
> I could not construct this with an in-tree exporter (udmabuf, dma-heaps and
> DRM PRIME all map exactly the object size), and I did not find an explicit
> dma-buf API guarantee that the mapped sg length equals dmabuf->size either.
> Would it be worth bounding niov_idx against area.num_niovs, or zeroing
> tx_vec, while the push side is being touched here?

"I could not construct this with an in-tree exporter" - the arrays
are sized such that the niovs should fit.

> This is still the case at the end of the series (3ee6ae62e2d2): the later
> patch "net: devmem: use memory provider helpers for net_iovs" zeroes
> area.niovs but leaves tx_vec unzeroed and niov_idx unbounded.
> 
> > diff --git a/net/core/devmem.h b/net/core/devmem.h
> > index f71d9a2bcb3d3..5edb763b89b26 100644
> > --- a/net/core/devmem.h
> > +++ b/net/core/devmem.h
> > @@ -17,6 +17,9 @@ struct netlink_ext_ack;
> >  
> >  struct net_devmem_dmabuf_binding {
> >  	struct net_iov_area area;
> > +	/* Protects area.freelist and area.free_count. */
> > +	spinlock_t freelist_lock;
> > +
> >  	struct dma_buf *dmabuf;
> >  	struct dma_buf_attachment *attachment;
> >  	struct sg_table *sgt;
> > @@ -57,10 +60,6 @@ struct net_devmem_dmabuf_binding {
> >  	/* rxq's this binding is active on. */
> >  	struct xarray bound_rxqs;
> >  
> > -	spinlock_t freelist_lock ____cacheline_aligned_in_smp;
> > -	size_t free_count;
> > -	struct net_iov **freelist;
> > -
> 
> [Severity: Medium]
> The ____cacheline_aligned_in_smp annotation on freelist_lock is dropped
> here, and the lock moves from the tail of the binding to right after the
> embedded struct net_iov_area. Was that intentional?
> 
> With the new area layout (niovs at 0, freelist at 8, base_virtual at 16,
> num_niovs at 24, free_count at 28) the write-hot lock word and free_count
> end up in the same 64-byte line as the read-mostly area.niovs and
> area.base_virtual.
> 
> The writers dirty that line on every allocation and release:
> 
> net/core/devmem.c:net_devmem_alloc_dmabuf_bulk() {
> 	spin_lock_bh(&binding->freelist_lock);
> 	count = min(count, binding->area.free_count);
> 	...
> }
> 
> net/core/devmem.c:net_devmem_free_dmabuf() {
> 	spin_lock_bh(&binding->freelist_lock);
> 	net_iov_area_push(&binding->area, niov);
> 	...
> }
> 
> while the receive path reads the same line per fragment, via
> net_iov_idx() -> net_iov_owner(niov)->niovs and
> net_devmem_iov_virtual_addr() -> net_iov_owner(niov)->base_virtual.
> 
> In the common devmem setup, where page pool refill/release runs on a
> different core than the recvmsg() consumer, does this re-introduce the false
> sharing the removed annotation was there to prevent?
> 
> The commit message says:
> 
>     Keep devmem's area adjacent to its lock.
> 
> but it does not mention removing the SMP cacheline isolation, and no numbers
> are given. Annotating freelist_lock (or the lock plus free_count pair)
> ____cacheline_aligned_in_smp again would keep the refactor intact.
> 
> I checked the end of the series (3ee6ae62e2d2) and net/core/devmem.h still
> declares the lock right after the area with no ____cacheline_aligned_in_smp,
> so nothing later restores it.

Yes, this is all intentional. The lock and the freelist are used together.

Re: [PATCH net-next 1/3] net: netmem: add net_iov_area freelist helpers

[Mina Almasry]

On Tue, Sep 22, 2026 at 1:43 PM Stanislav Fomichev <sdf.kernel@gmail.com> wrote:
>
> io_uring zero-copy receive and devmem both maintain a bounded LIFO for
> net_iovs in a contiguous area. Store the freelist in struct net_iov_area
> and provide common push and pop helpers.
>
> Leave synchronization to area owners. Keep devmem's area adjacent to its

This could be a follow up change, but I think synchronization should
be provided by the netmem/niov infra, rather than the area owners. TBH
the infra providing an unsynchronized data structure and letting the
area owner use it and shoot themselves in the foot feels error prone.
For now we could use a comment.

I also think we should not provide _push and_pop functions, rather we
should provide _push_bulk() and _pop_bulk(), and the caller can decide
to only push and pop 1 at a time if they need to. The reason is that
in allocation paths, we almost always want to alloc in bulk. If the
infra had the lock it could lock once and alloc a bulk.

The free path is more nuanced. I think _push_bulk() is actually not
currently that useful, because page_pool_put_netmem_bulk() ends up
internally looping over individual calls to __page_pool_put_page(). It
seems like there is a very low hanging fruit optimization possible
here where we change things such that page_pool_put_netmem_bulk()
actually does free the entire bulk at once, and if the pp is using a
memory provider, it uses push _push_bulk() to free the entire stack of
netmems with 1 lock acquire.

But these can be future optimizations, so,

Reviewed-by: Mina Almasry <almasrymina@google.com>

> lock. Use u32 indices and counts, which halves devmem's freelist storage
> on 64-bit systems. Reject devmem areas with more than U32_MAX entries
> before narrowing the count. With 4 KiB chunks, the limit is almost 16 TiB.
>
> Signed-off-by: Stanislav Fomichev <sdf@fomichev.me>
> ---
>  include/net/netmem.h | 28 ++++++++++++++++++++++++++-
>  io_uring/zcrx.c      | 25 +++++++++---------------
>  io_uring/zcrx.h      |  4 ----
>  net/core/devmem.c    | 46 ++++++++++++++++++++++----------------------
>  net/core/devmem.h    |  7 +++----
>  5 files changed, 62 insertions(+), 48 deletions(-)
>
> diff --git a/include/net/netmem.h b/include/net/netmem.h
> index bccacd21b6c3..da885d95ea63 100644
> --- a/include/net/netmem.h
> +++ b/include/net/netmem.h
> @@ -101,10 +101,15 @@ struct net_iov {
>  struct net_iov_area {
>         /* Array of net_iovs for this area. */
>         struct net_iov *niovs;
> -       size_t num_niovs;
> +
> +       /* Stack of free net_iov indices. */
> +       u32 *freelist;
>
>         /* Offset into the dma-buf where this chunk starts.  */
>         unsigned long base_virtual;
> +
> +       u32 num_niovs;
> +       u32 free_count;

Why not keep num_niovs a size_t and make free_count a size_t as well
(so that essentially we can support anything up to SIZE_T_MAX - if
that exists - num entries)? Do we gain anything by converting these to
u32? I know in practice in really doens't make a difference, but since
struct dma_buf->size is a size_t and that's usually the type we use
for memory sizes we should use it unless we see a reason not to. Tbh I
don't think the size of the freelist array is a big deal?


-- 
Thanks,
Mina

Re: [PATCH net-next 1/3] net: netmem: add net_iov_area freelist helpers

[Pavel Begunkov]

On 9/24/26 16:02, Mina Almasry wrote:
> On Tue, Sep 22, 2026 at 1:43 PM Stanislav Fomichev <sdf.kernel@gmail.com> wrote:
>>
>> io_uring zero-copy receive and devmem both maintain a bounded LIFO for
>> net_iovs in a contiguous area. Store the freelist in struct net_iov_area
>> and provide common push and pop helpers.
>>
>> Leave synchronization to area owners. Keep devmem's area adjacent to its
> 
> This could be a follow up change, but I think synchronization should
> be provided by the netmem/niov infra, rather than the area owners. TBH
> the infra providing an unsynchronized data structure and letting the
> area owner use it and shoot themselves in the foot feels error prone.
> For now we could use a comment.

I don't think we want it. The duplication is minor, but I'm not set
on the per area index array approach, and it'd make changing it
more difficult.

> I also think we should not provide _push and_pop functions, rather we
> should provide _push_bulk() and _pop_bulk(), and the caller can decide
> to only push and pop 1 at a time if they need to. The reason is that
> in allocation paths, we almost always want to alloc in bulk. If the
> infra had the lock it could lock once and alloc a bulk.
-- 
Pavel Begunkov

Re: [PATCH net-next 1/3] net: netmem: add net_iov_area freelist helpers

[Mina Almasry]

On Thu, Sep 24, 2026 at 8:41 AM Pavel Begunkov <asml.silence@gmail.com> wrote:
>
> On 9/24/26 16:02, Mina Almasry wrote:
> > On Tue, Sep 22, 2026 at 1:43 PM Stanislav Fomichev <sdf.kernel@gmail.com> wrote:
> >>
> >> io_uring zero-copy receive and devmem both maintain a bounded LIFO for
> >> net_iovs in a contiguous area. Store the freelist in struct net_iov_area
> >> and provide common push and pop helpers.
> >>
> >> Leave synchronization to area owners. Keep devmem's area adjacent to its
> >
> > This could be a follow up change, but I think synchronization should
> > be provided by the netmem/niov infra, rather than the area owners. TBH
> > the infra providing an unsynchronized data structure and letting the
> > area owner use it and shoot themselves in the foot feels error prone.
> > For now we could use a comment.
>
> I don't think we want it. The duplication is minor, but I'm not set
> on the per area index array approach, and it'd make changing it
> more difficult.
>

What is 'it'? I'm guessing you don't want the freelist array put in
the net_iov_area at all then? Because even that change will make
replacing the freelist array hard to swap? Or are you objecting to
just letting the netmem functions do the locking.


--
Thanks,
Mina

Re: [PATCH net-next 1/3] net: netmem: add net_iov_area freelist helpers

[Stanislav Fomichev]

On 09/24, Pavel Begunkov wrote:
> On 9/24/26 16:02, Mina Almasry wrote:
> > On Tue, Sep 22, 2026 at 1:43 PM Stanislav Fomichev <sdf.kernel@gmail.com> wrote:
> > > 
> > > io_uring zero-copy receive and devmem both maintain a bounded LIFO for
> > > net_iovs in a contiguous area. Store the freelist in struct net_iov_area
> > > and provide common push and pop helpers.
> > > 
> > > Leave synchronization to area owners. Keep devmem's area adjacent to its
> > 
> > This could be a follow up change, but I think synchronization should
> > be provided by the netmem/niov infra, rather than the area owners. TBH
> > the infra providing an unsynchronized data structure and letting the
> > area owner use it and shoot themselves in the foot feels error prone.
> > For now we could use a comment.
> 
> I don't think we want it. The duplication is minor, but I'm not set
> on the per area index array approach, and it'd make changing it
> more difficult.

Do you want me to not touch iou in this patch at all? Or are you talking
about potential future synchronization part?

I don't see how this is making changing it more difficult, the freelist is
now behind push/pop which you can freely change, and both UAPIs benefit
from a faster/better freelist.

Re: [PATCH net-next 1/3] net: netmem: add net_iov_area freelist helpers

[Stanislav Fomichev]

On 09/24, Mina Almasry wrote:
> On Tue, Sep 22, 2026 at 1:43 PM Stanislav Fomichev <sdf.kernel@gmail.com> wrote:
> >
> > io_uring zero-copy receive and devmem both maintain a bounded LIFO for
> > net_iovs in a contiguous area. Store the freelist in struct net_iov_area
> > and provide common push and pop helpers.
> >
> > Leave synchronization to area owners. Keep devmem's area adjacent to its
> 
> This could be a follow up change, but I think synchronization should
> be provided by the netmem/niov infra, rather than the area owners. TBH
> the infra providing an unsynchronized data structure and letting the
> area owner use it and shoot themselves in the foot feels error prone.
> For now we could use a comment.
> 
> I also think we should not provide _push and_pop functions, rather we
> should provide _push_bulk() and _pop_bulk(), and the caller can decide
> to only push and pop 1 at a time if they need to. The reason is that
> in allocation paths, we almost always want to alloc in bulk. If the
> infra had the lock it could lock once and alloc a bulk.
> 
> The free path is more nuanced. I think _push_bulk() is actually not
> currently that useful, because page_pool_put_netmem_bulk() ends up
> internally looping over individual calls to __page_pool_put_page(). It
> seems like there is a very low hanging fruit optimization possible
> here where we change things such that page_pool_put_netmem_bulk()
> actually does free the entire bulk at once, and if the pp is using a
> memory provider, it uses push _push_bulk() to free the entire stack of
> netmems with 1 lock acquire.
> 
> But these can be future optimizations, so,
> 
> Reviewed-by: Mina Almasry <almasrymina@google.com>

Ack, let's discuss separately, this is probably a larger refactor.

> > lock. Use u32 indices and counts, which halves devmem's freelist storage
> > on 64-bit systems. Reject devmem areas with more than U32_MAX entries
> > before narrowing the count. With 4 KiB chunks, the limit is almost 16 TiB.
> >
> > Signed-off-by: Stanislav Fomichev <sdf@fomichev.me>
> > ---
> >  include/net/netmem.h | 28 ++++++++++++++++++++++++++-
> >  io_uring/zcrx.c      | 25 +++++++++---------------
> >  io_uring/zcrx.h      |  4 ----
> >  net/core/devmem.c    | 46 ++++++++++++++++++++++----------------------
> >  net/core/devmem.h    |  7 +++----
> >  5 files changed, 62 insertions(+), 48 deletions(-)
> >
> > diff --git a/include/net/netmem.h b/include/net/netmem.h
> > index bccacd21b6c3..da885d95ea63 100644
> > --- a/include/net/netmem.h
> > +++ b/include/net/netmem.h
> > @@ -101,10 +101,15 @@ struct net_iov {
> >  struct net_iov_area {
> >         /* Array of net_iovs for this area. */
> >         struct net_iov *niovs;
> > -       size_t num_niovs;
> > +
> > +       /* Stack of free net_iov indices. */
> > +       u32 *freelist;
> >
> >         /* Offset into the dma-buf where this chunk starts.  */
> >         unsigned long base_virtual;
> > +
> > +       u32 num_niovs;
> > +       u32 free_count;
> 
> Why not keep num_niovs a size_t and make free_count a size_t as well
> (so that essentially we can support anything up to SIZE_T_MAX - if
> that exists - num entries)? Do we gain anything by converting these to
> u32? I know in practice in really doens't make a difference, but since
> struct dma_buf->size is a size_t and that's usually the type we use
> for memory sizes we should use it unless we see a reason not to. Tbh I
> don't think the size of the freelist array is a big deal?

My reasoning: 4K * UINT_MAX is ~18TB which should be enough for a
foreseeable future. So just saving a few bytes, free_count was already
u32 on iour side.

[PATCH net-next 2/3] net: devmem: use memory provider helpers for net_iovs

[Stanislav Fomichev]

Use public memory-provider helpers for DMA address storage and page-pool
association instead of open-coding page_pool state and accounting.

Zero the net_iov array because net_mp_niov_set_page_pool() ORs
PP_SIGNATURE into pp_magic. Reject DMA addresses that cannot be
represented in net_iov metadata.

Signed-off-by: Stanislav Fomichev <sdf@fomichev.me>
---
 net/core/devmem.c | 31 +++++++++++--------------------
 1 file changed, 11 insertions(+), 20 deletions(-)

diff --git a/net/core/devmem.c b/net/core/devmem.c
index c1c1872b88de..0fc77c1a2956 100644
--- a/net/core/devmem.c
+++ b/net/core/devmem.c
@@ -17,11 +17,9 @@
 #include <net/page_pool/memory_provider.h>
 #include <net/sock.h>
 #include <net/tcp.h>
-#include <trace/events/page_pool.h>
 
 #include "devmem.h"
 #include "mp_dmabuf_devmem.h"
-#include "page_pool_priv.h"
 
 /* Device memory support */
 
@@ -247,7 +245,7 @@ net_devmem_bind_dmabuf(struct net_device *dev, void *vdev,
 			goto err_unmap;
 		}
 	}
-	binding->area.niovs = kvmalloc_objs(*binding->area.niovs,
+	binding->area.niovs = kvzalloc_objs(*binding->area.niovs,
 					    binding->area.num_niovs);
 	if (!binding->area.niovs) {
 		err = -ENOMEM;
@@ -274,8 +272,12 @@ net_devmem_bind_dmabuf(struct net_device *dev, void *vdev,
 		for (i = 0; i < nr_niovs; i++, niov_idx++) {
 			niov = &binding->area.niovs[niov_idx];
 			net_iov_init(niov, &binding->area, NET_IOV_DMABUF);
-			page_pool_set_dma_addr_netmem(net_iov_to_netmem(niov),
-						      dma_addr);
+			if (net_mp_niov_set_dma_addr(niov, dma_addr)) {
+				err = -EFAULT;
+				NL_SET_ERR_MSG(extack,
+					       "dmabuf DMA address cannot be represented");
+				goto err_free_niovs;
+			}
 			if (direction == DMA_TO_DEVICE)
 				binding->tx_vec[niov_idx] = niov;
 			else
@@ -439,19 +441,9 @@ netmem_ref mp_dmabuf_devmem_alloc_netmems(struct page_pool *pool, gfp_t gfp)
 	if (unlikely(!allocated))
 		return 0;
 
-	for (i = 0; i < allocated; i++) {
-		struct net_iov *niov = netmem_to_net_iov(netmems[i]);
-
-		niov->desc.pp_magic = 0;
-		niov->desc.pp = NULL;
-		atomic_long_set(&niov->desc.pp_ref_count, 0);
-
-		page_pool_set_pp_info(pool, netmems[i]);
-
-		pool->pages_state_hold_cnt++;
-		trace_page_pool_state_hold(pool, netmems[i],
-					   pool->pages_state_hold_cnt);
-	}
+	for (i = 0; i < allocated; i++)
+		net_mp_niov_set_page_pool(pool,
+					  netmem_to_net_iov(netmems[i]));
 
 	/* Return the last one, the rest stay in the page_pool cache. */
 	allocated--;
@@ -476,8 +468,7 @@ bool mp_dmabuf_devmem_release_page(struct page_pool *pool, netmem_ref netmem)
 	if (WARN_ON_ONCE(refcount != 1))
 		return false;
 
-	page_pool_clear_pp_info(netmem);
-
+	net_mp_niov_clear_page_pool(netmem_to_net_iov(netmem));
 	net_devmem_free_dmabuf(netmem_to_net_iov(netmem));
 
 	/* We don't want the page pool put_page()ing our net_iovs. */
-- 
2.53.0-Meta

Re: [PATCH net-next 2/3] net: devmem: use memory provider helpers for net_iovs

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found more severe
issue(s) in other patches of this series. This mail is only an FYI, in case
you want to address the nit picks below when you respin.

Low priority / nit picks for your consideration:

- [Low] net: devmem: use memory provider helpers for net_iovs bundles a
  real bug fix into a patch whose commit message presents it as a…

--

net: devmem: use memory provider helpers for net_iovs

This converts net/core/devmem.c to the public memory-provider helpers
net_mp_niov_set_dma_addr(), net_mp_niov_set_page_pool() and
net_mp_niov_clear_page_pool() instead of open-coding page_pool state,
switches the net_iov array to kvzalloc_objs(), and now rejects DMA
addresses that cannot be represented in net_iov metadata.

>  	Zero the net_iov array because net_mp_niov_set_page_pool() ORs
>  	PP_SIGNATURE into pp_magic. Reject DMA addresses that cannot be
>  	represented in net_iov metadata.

[Severity: Low]
This isn't a bug in the new code, but should the "Reject DMA addresses
that cannot be represented in net_iov metadata" sentence spell out that
the old behaviour was a silent truncation, and should the patch carry a
Fixes: tag?

Before this patch net_devmem_bind_dmabuf() dropped the return value of
page_pool_set_dma_addr_netmem(), which is the "this address does not
round-trip through the compressed net_iov metadata" indicator:

net/core/page_pool_priv.h:page_pool_set_dma_addr_netmem() {
	if (PAGE_POOL_32BIT_ARCH_WITH_64BIT_DMA) {
		netmem_set_dma_addr(netmem, addr >> PAGE_SHIFT);

		/* We assume page alignment to shave off bottom bits,
		 * if this "compression" doesn't work we need to drop.
		 */
		return addr != (dma_addr_t)netmem_get_dma_addr(netmem)
				       << PAGE_SHIFT;
	}
	...
}

Every other caller treats a true return as fatal, e.g.
page_pool_dma_map():

	if (page_pool_set_dma_addr_netmem(netmem, dma)) {
		WARN_ONCE(1, "unexpected DMA address, please report to netdev@");
		goto unmap_failed;
	}

and io_uring/zcrx.c returns -EFAULT. Devmem was the only site that kept
the truncated address and handed it to the NIC for DMA.

The path looks reachable from user space on 32-bit arches with 64-bit
dma_addr_t: netdev_nl_bind_rx_doit() only checks is_power_of_2() on
NETDEV_A_DMABUF_RX_PAGE_SIZE before niov_shift = ilog2(rx_page_size), so
a sub-PAGE_SIZE niov size is accepted, and the bind loop then advances
dma_addr += niov_size producing non-page-aligned addresses whose
compression fails.

Given that the next patch in the series ("net: devmem: decode DMA
addresses for TX") does carry a Fixes: tag for the other half of the same
32-bit/64-bit-DMA problem, would it make sense to split this hunk out
with its own Fixes: tag so it can be picked up for stable?

> diff --git a/net/core/devmem.c b/net/core/devmem.c
> index c1c1872b88de2..0fc77c1a29569 100644
> --- a/net/core/devmem.c
> +++ b/net/core/devmem.c

[ ... ]

> @@ -274,8 +272,12 @@ net_devmem_bind_dmabuf(struct net_device *dev, void *vdev,
>  		for (i = 0; i < nr_niovs; i++, niov_idx++) {
>  			niov = &binding->area.niovs[niov_idx];
>  			net_iov_init(niov, &binding->area, NET_IOV_DMABUF);
> -			page_pool_set_dma_addr_netmem(net_iov_to_netmem(niov),
> -						      dma_addr);
> +			if (net_mp_niov_set_dma_addr(niov, dma_addr)) {
> +				err = -EFAULT;
> +				NL_SET_ERR_MSG(extack,
> +					       "dmabuf DMA address cannot be represented");
> +				goto err_free_niovs;
> +			}
>  			if (direction == DMA_TO_DEVICE)
>  				binding->tx_vec[niov_idx] = niov;
>  			else

[ ... ]

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260922204348.717198-1-sdf%40fomichev.me

Re: [PATCH net-next 2/3] net: devmem: use memory provider helpers for net_iovs

[Stanislav Fomichev]

On 09/23, netdev-bot+sashiko@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found more severe
> issue(s) in other patches of this series. This mail is only an FYI, in case
> you want to address the nit picks below when you respin.
> 
> Low priority / nit picks for your consideration:
> 
> - [Low] net: devmem: use memory provider helpers for net_iovs bundles a
>   real bug fix into a patch whose commit message presents it as a…
> 
> --
> 
> net: devmem: use memory provider helpers for net_iovs
> 
> This converts net/core/devmem.c to the public memory-provider helpers
> net_mp_niov_set_dma_addr(), net_mp_niov_set_page_pool() and
> net_mp_niov_clear_page_pool() instead of open-coding page_pool state,
> switches the net_iov array to kvzalloc_objs(), and now rejects DMA
> addresses that cannot be represented in net_iov metadata.
> 
> >  	Zero the net_iov array because net_mp_niov_set_page_pool() ORs
> >  	PP_SIGNATURE into pp_magic. Reject DMA addresses that cannot be
> >  	represented in net_iov metadata.
> 
> [Severity: Low]
> This isn't a bug in the new code, but should the "Reject DMA addresses
> that cannot be represented in net_iov metadata" sentence spell out that
> the old behaviour was a silent truncation, and should the patch carry a
> Fixes: tag?
> 
> Before this patch net_devmem_bind_dmabuf() dropped the return value of
> page_pool_set_dma_addr_netmem(), which is the "this address does not
> round-trip through the compressed net_iov metadata" indicator:
> 
> net/core/page_pool_priv.h:page_pool_set_dma_addr_netmem() {
> 	if (PAGE_POOL_32BIT_ARCH_WITH_64BIT_DMA) {
> 		netmem_set_dma_addr(netmem, addr >> PAGE_SHIFT);
> 
> 		/* We assume page alignment to shave off bottom bits,
> 		 * if this "compression" doesn't work we need to drop.
> 		 */
> 		return addr != (dma_addr_t)netmem_get_dma_addr(netmem)
> 				       << PAGE_SHIFT;
> 	}
> 	...
> }
> 
> Every other caller treats a true return as fatal, e.g.
> page_pool_dma_map():
> 
> 	if (page_pool_set_dma_addr_netmem(netmem, dma)) {
> 		WARN_ONCE(1, "unexpected DMA address, please report to netdev@");
> 		goto unmap_failed;
> 	}
> 
> and io_uring/zcrx.c returns -EFAULT. Devmem was the only site that kept
> the truncated address and handed it to the NIC for DMA.
> 
> The path looks reachable from user space on 32-bit arches with 64-bit
> dma_addr_t: netdev_nl_bind_rx_doit() only checks is_power_of_2() on
> NETDEV_A_DMABUF_RX_PAGE_SIZE before niov_shift = ilog2(rx_page_size), so
> a sub-PAGE_SIZE niov size is accepted, and the bind loop then advances
> dma_addr += niov_size producing non-page-aligned addresses whose
> compression fails.
> 
> Given that the next patch in the series ("net: devmem: decode DMA
> addresses for TX") does carry a Fixes: tag for the other half of the same
> 32-bit/64-bit-DMA problem, would it make sense to split this hunk out
> with its own Fixes: tag so it can be picked up for stable?

I do not think this suggestion makes any sense. We do need to check
the return value of page_pool_set_dma_addr_netmem. Whether it's this
patch (I think it fits better here), or next - does not matter much.

Re: [PATCH net-next 2/3] net: devmem: use memory provider helpers for net_iovs

[Mina Almasry]

On Tue, Sep 22, 2026 at 1:43 PM Stanislav Fomichev <sdf.kernel@gmail.com> wrote:
>
> Use public memory-provider helpers for DMA address storage and page-pool
> association instead of open-coding page_pool state and accounting.
>
> Zero the net_iov array because net_mp_niov_set_page_pool() ORs
> PP_SIGNATURE into pp_magic. Reject DMA addresses that cannot be
> represented in net_iov metadata.
>
> Signed-off-by: Stanislav Fomichev <sdf@fomichev.me>

Reviewed-by: Mina Almasry <almasrymina@google.com>

-- 
Thanks,
Mina

[PATCH net-next 3/3] net: devmem: decode DMA addresses for TX

[Stanislav Fomichev]

On 32-bit architectures where dma_addr_t is wider than unsigned long,
page_pool_set_dma_addr_netmem() stores page-aligned DMA addresses shifted
by PAGE_SHIFT. The net_iov branch of __skb_frag_dma_map() adds byte offsets
to the encoded value, so the NIC is programmed with an invalid DMA address.
This can trigger an IOMMU fault or DMA from unintended memory.

Consolidate DMA address encoding, decoding, and representability checks in
netmem helpers. Use the common decoder from the page pool and net_iov TX
paths so both interpret stored addresses consistently.

Fixes: bd61848900bf ("net: devmem: Implement TX path")
Signed-off-by: Stanislav Fomichev <sdf@fomichev.me>
---
 include/linux/skbuff.h          |  7 +++++--
 include/net/netmem.h            | 27 +++++++++++++++++++++++++++
 include/net/page_pool/helpers.h | 10 +---------
 net/core/page_pool_priv.h       | 14 ++------------
 4 files changed, 35 insertions(+), 23 deletions(-)

diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index 421f6fc45451..3740d4b15b70 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -3833,9 +3833,12 @@ static inline dma_addr_t __skb_frag_dma_map(struct device *dev,
 					    size_t offset, size_t size,
 					    enum dma_data_direction dir)
 {
+	dma_addr_t addr;
+
 	if (skb_frag_is_net_iov(frag)) {
-		return netmem_to_net_iov(frag->netmem)->desc.dma_addr +
-		       offset + frag->offset;
+		addr = netmem_dma_addr_decode(
+			netmem_get_dma_addr(frag->netmem));
+		return addr + offset + frag->offset;
 	}
 	return dma_map_page(dev, skb_frag_page(frag),
 			    skb_frag_off(frag) + offset, size, dir);
diff --git a/include/net/netmem.h b/include/net/netmem.h
index da885d95ea63..4e06a647620a 100644
--- a/include/net/netmem.h
+++ b/include/net/netmem.h
@@ -384,6 +384,33 @@ static inline bool netmem_is_pfmemalloc(netmem_ref netmem)
 	return page_is_pfmemalloc(netmem_to_page(netmem));
 }
 
+#define NETMEM_32BIT_ARCH_WITH_64BIT_DMA	\
+	(sizeof(dma_addr_t) > sizeof(unsigned long))
+
+static inline unsigned long netmem_dma_addr_encode(dma_addr_t addr)
+{
+	if (NETMEM_32BIT_ARCH_WITH_64BIT_DMA)
+		addr >>= PAGE_SHIFT;
+
+	return addr;
+}
+
+static inline dma_addr_t netmem_dma_addr_decode(unsigned long addr)
+{
+	if (NETMEM_32BIT_ARCH_WITH_64BIT_DMA)
+		return (dma_addr_t)addr << PAGE_SHIFT;
+
+	return addr;
+}
+
+static inline bool netmem_dma_addr_fits(dma_addr_t addr)
+{
+	/* We assume page alignment to shave off bottom bits,
+	 * if this "compression" doesn't work we need to drop.
+	 */
+	return addr == netmem_dma_addr_decode(netmem_dma_addr_encode(addr));
+}
+
 static inline unsigned long netmem_get_dma_addr(netmem_ref netmem)
 {
 	return netmem_to_nmdesc(netmem)->dma_addr;
diff --git a/include/net/page_pool/helpers.h b/include/net/page_pool/helpers.h
index 87a4e13886e1..cd021832c3fa 100644
--- a/include/net/page_pool/helpers.h
+++ b/include/net/page_pool/helpers.h
@@ -408,9 +408,6 @@ static inline void page_pool_recycle_direct_netmem(struct page_pool *pool,
 	page_pool_put_full_netmem(pool, netmem, true);
 }
 
-#define PAGE_POOL_32BIT_ARCH_WITH_64BIT_DMA	\
-		(sizeof(dma_addr_t) > sizeof(unsigned long))
-
 /**
  * page_pool_free_va() - free a va into the page_pool
  * @pool: pool from which va was allocated
@@ -427,12 +424,7 @@ static inline void page_pool_free_va(struct page_pool *pool, void *va,
 
 static inline dma_addr_t page_pool_get_dma_addr_netmem(netmem_ref netmem)
 {
-	dma_addr_t ret = netmem_get_dma_addr(netmem);
-
-	if (PAGE_POOL_32BIT_ARCH_WITH_64BIT_DMA)
-		ret <<= PAGE_SHIFT;
-
-	return ret;
+	return netmem_dma_addr_decode(netmem_get_dma_addr(netmem));
 }
 
 /**
diff --git a/net/core/page_pool_priv.h b/net/core/page_pool_priv.h
index 2fb06d5f6d55..430b97cd88da 100644
--- a/net/core/page_pool_priv.h
+++ b/net/core/page_pool_priv.h
@@ -18,18 +18,8 @@ void page_pool_unlist(struct page_pool *pool);
 static inline bool
 page_pool_set_dma_addr_netmem(netmem_ref netmem, dma_addr_t addr)
 {
-	if (PAGE_POOL_32BIT_ARCH_WITH_64BIT_DMA) {
-		netmem_set_dma_addr(netmem, addr >> PAGE_SHIFT);
-
-		/* We assume page alignment to shave off bottom bits,
-		 * if this "compression" doesn't work we need to drop.
-		 */
-		return addr != (dma_addr_t)netmem_get_dma_addr(netmem)
-				       << PAGE_SHIFT;
-	}
-
-	netmem_set_dma_addr(netmem, addr);
-	return false;
+	netmem_set_dma_addr(netmem, netmem_dma_addr_encode(addr));
+	return !netmem_dma_addr_fits(addr);
 }
 
 static inline bool page_pool_set_dma_addr(struct page *page, dma_addr_t addr)
-- 
2.53.0-Meta

Re: [PATCH net-next 3/3] net: devmem: decode DMA addresses for TX

[Mina Almasry]

On Tue, Sep 22, 2026 at 1:43 PM Stanislav Fomichev <sdf.kernel@gmail.com> wrote:
>
> On 32-bit architectures where dma_addr_t is wider than unsigned long,
> page_pool_set_dma_addr_netmem() stores page-aligned DMA addresses shifted
> by PAGE_SHIFT. The net_iov branch of __skb_frag_dma_map() adds byte offsets
> to the encoded value, so the NIC is programmed with an invalid DMA address.
> This can trigger an IOMMU fault or DMA from unintended memory.
>
> Consolidate DMA address encoding, decoding, and representability checks in
> netmem helpers. Use the common decoder from the page pool and net_iov TX
> paths so both interpret stored addresses consistently.
>
> Fixes: bd61848900bf ("net: devmem: Implement TX path")
> Signed-off-by: Stanislav Fomichev <sdf@fomichev.me>

Reviewed-by: Mina Almasry <almasrymina@google.com>

-- 
Thanks,
Mina