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. > + 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/[email protected]?part=1
