On Fri, Jul 10, 2026 at 07:27:12AM +0000, [email protected] 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 <[email protected]> > > 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?
