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

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [Critical] The `tx_vec` array is allocated without zero-initialization, leading to dereferencing uninitialized pointers if the mapped SG table is smaller than the dmabuf.
- [Critical] The `tx_vec` array can be overflowed if a dmabuf exporter returns an SG table whose total mapped length exceeds `dmabuf->size`.
--

commit 19418ca41b5b75f8c09b7526af4fcb2ea506b0fa
Author: Bobby Eshleman <bobbyeshleman@meta.com>
Subject: net: devmem: allow rx-buf-size > PAGE_SIZE per dmabuf binding

This patch adds a bind-time netlink attribute, NETDEV_A_DMABUF_RX_BUF_SIZE,
that lets userspace request a larger niov size for devmem dmabuf bindings,
allowing rx-buf-size > PAGE_SIZE.

> diff --git a/net/core/devmem.c b/net/core/devmem.c
> --- a/net/core/devmem.c
> +++ b/net/core/devmem.c
> @@ -248,18 +256,14 @@ net_devmem_bind_dmabuf(struct net_device *dev, void *vdev,
>  			goto err_unmap;
>  		}
>  		binding->tx_vec = kvmalloc_objs(struct net_iov *,
> -						dmabuf->size / PAGE_SIZE);
> +						dmabuf->size >> niov_shift);

[Severity: Critical]
This isn't a bug introduced by this patch, but since kvmalloc_objs() doesn't
zero-initialize the memory, is it possible for uninitialized pointers to be
returned later?

If a dmabuf exporter returns an SG table whose total mapped length is less
than dmabuf->size, the trailing elements of the tx_vec array would remain
uninitialized. When userspace requests transmission with an offset beyond
the mapped length but within dmabuf->size, net_devmem_get_niov_at() would
read from this uninitialized memory and return a garbage pointer to the
datapath.

[ ... ]

> @@ -313,7 +319,7 @@ net_devmem_bind_dmabuf(struct net_device *dev, void *vdev,
>  			page_pool_set_dma_addr_netmem(net_iov_to_netmem(niov),
>  						      net_devmem_get_dma_addr(niov));
>  			if (direction == DMA_TO_DEVICE)
> -				binding->tx_vec[owner->area.base_virtual / PAGE_SIZE + i] = niov;
> +				binding->tx_vec[(owner->area.base_virtual >> niov_shift) + i] = niov;

[Severity: Critical]
This is a pre-existing issue, but does this loop lack bounds checking
against the actual allocated size of tx_vec?

The tx_vec array is allocated to hold dmabuf->size >> niov_shift elements.
If a buggy or malicious dmabuf exporter returns an SG table that maps more
data than expected (exceeding dmabuf->size), this loop will write pointers
past the end of the dynamically allocated tx_vec array, corrupting adjacent
kernel memory.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260603-tcpdm-large-niovs-v1-0-f37a4ac6726c@meta.com?part=1