Re: [PATCH net-next v5 1/3] net: devmem: allow rx-buf-size > PAGE_SIZE per dmabuf binding

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] On 32-bit architectures with 64-bit DMA addresses (e.g., CONFIG_ARCH_DMA_ADDR_T_64BIT), passing `dma_addr_t` to `gen_pool` functions truncates the DMA address to 32 bits, leading to broken bit allocations, leaked chunks, and possible data corruption.
--

--- Patch [1]: [PATCH net-next v5 1/3] net: devmem: allow rx-buf-size > PAGE_SIZE per dmabuf binding ---
commit 700c87449a5348e02d33589504f632f46bed92e7
Author: Bobby Eshleman <bobbyeshleman@meta.com>

net: devmem: allow rx-buf-size > PAGE_SIZE per dmabuf binding

This commit introduces the NETDEV_A_DMABUF_RX_BUF_SIZE netlink attribute.
It allows userspace to request an RX buffer size larger than PAGE_SIZE
for devmem dmabuf bindings, avoiding CPU churn for large flows.

> diff --git a/net/core/devmem.c b/net/core/devmem.c
> index 957d6b96216b3..3ce3cc14bec07 100644
> --- a/net/core/devmem.c
> +++ b/net/core/devmem.c
[ ... ]
> @@ -113,12 +114,13 @@ void net_devmem_free_dmabuf(struct net_iov *niov)
>  {
>  	struct net_devmem_dmabuf_binding *binding = net_devmem_iov_binding(niov);
>  	unsigned long dma_addr = net_devmem_get_dma_addr(niov);
> +	size_t niov_size = 1UL << binding->niov_shift;
>  
>  	if (WARN_ON(!gen_pool_has_addr(binding->chunk_pool, dma_addr,
> -				       PAGE_SIZE)))

[Severity: High]
This is a pre-existing issue, but can this truncate the DMA address? 
Looking at net_devmem_free_dmabuf() in net/core/devmem.c, dma_addr is
derived as a dma_addr_t (which can be 64-bit), but is passed to
gen_pool_has_addr() and gen_pool_free() which expect an unsigned long. 

On 32-bit architectures with 64-bit DMA addresses (like
CONFIG_ARCH_DMA_ADDR_T_64BIT), this will silently truncate the upper 32
bits. If a chunk crosses a 4GB boundary in its lower 32 bits, the end
address could wrap around, causing gen_pool_has_addr() to fail the bounds
check and permanently leak memory via this WARN_ON path.

> +				       niov_size)))
>  		return;
>  
> -	gen_pool_free(binding->chunk_pool, dma_addr, PAGE_SIZE);
> +	gen_pool_free(binding->chunk_pool, dma_addr, niov_size);
>  }
[ ... ]
> @@ -288,7 +295,7 @@ net_devmem_bind_dmabuf(struct net_device *dev, void *vdev,
>  
>  		owner->area.base_virtual = virtual;
>  		owner->base_dma_addr = dma_addr;
> -		owner->area.num_niovs = len / PAGE_SIZE;
> +		owner->area.num_niovs = len >> niov_shift;
>  		owner->binding = binding;
>  
>  		err = gen_pool_add_owner(binding->chunk_pool, dma_addr,

[Severity: High]
This is a pre-existing issue, but does this suffer from the same 64-bit to
32-bit truncation? 

In net_devmem_bind_dmabuf() in net/core/devmem.c, gen_pool_add_owner()
takes an unsigned long for its virtual address parameter. Passing the
64-bit dma_addr here will truncate the upper 32 bits on 32-bit systems. 

If two dmabuf chunks happen to share the same lower 32 bits, this
truncation could cause overlapping chunk registrations. This might
eventually lead to double allocations of the same network buffers and
silent data corruption.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260708-tcpdm-large-niovs-v5-0-34bf6fac941b@meta.com?part=1