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