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

[Bobby Eshleman <bobbyeshleman@gmail.com>]

On Fri, Jul 10, 2026 at 07:27:12AM +0000, sashiko-bot@kernel.org wrote:
> 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.

Let's defer pre-existing issues to future work?

> 
> > +				       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.

Ditto?