Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues: - [High] Truncation of 64-bit `dma_addr_t` to 32-bit `unsigned long` when interacting with `gen_pool` causes memory leaks and corruption on 32-bit systems with LPAE. -- commit 294321e299dc1978a803324d3c6d3e8d5fb93d2d Author: Bobby Eshleman <[email protected]> net: devmem: allow rx-buf-size > PAGE_SIZE per dmabuf binding This commit introduces a new netlink attribute to allow devmem dmabuf bindings to request a larger niov size. The value must be a power of two greater than or equal to PAGE_SIZE, aiming to reduce CPU overhead for large flows. > diff --git a/net/core/devmem.c b/net/core/devmem.c > index 957d6b96216b3..3d6cf35e50f30 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))) > + niov_size))) > return; > > - gen_pool_free(binding->chunk_pool, dma_addr, PAGE_SIZE); > + gen_pool_free(binding->chunk_pool, dma_addr, niov_size); > } [Severity: High] This isn't a bug introduced by this patch, but does truncating dma_addr_t to unsigned long in net_devmem_free_dmabuf() cause issues on 32-bit systems with LPAE? The net_devmem_get_dma_addr() function returns a 64-bit dma_addr_t, but it is cast to an unsigned long local variable. The gen_pool_has_addr() and gen_pool_free() functions also expect an unsigned long for the address. If a DMA-BUF is mapped with a 64-bit dma_addr_t that exceeds the 32-bit boundary, passing it as unsigned long will truncate it. Could this lead to gen_pool_has_addr() failing and causing an unrecoverable memory leak, or gen_pool_free() clearing bits in the wrong chunk and causing memory corruption? [ ... ] > @@ -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 also a pre-existing issue, but does passing dma_addr to gen_pool_add_owner() truncate the 64-bit DMA address here? The virt parameter of gen_pool_add_owner() takes an unsigned long. If a contiguous chunk crosses a 4GB boundary on a 32-bit LPAE system, would this truncation cause a start_addr > end_addr wrap-around or overlap in the gen_pool? -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=1
