Re: [PATCH net-next v9 1/7] net: bcmgenet: convert RX path to page_pool

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Out-of-bounds DMA sync in bcmgenet_desc_rx() due to unvalidated packet length
--

commit dcf6f3586789c6e4c09c02881b479202e9ad31ad
Author: Nicolai Buchwitz <nb@tipi-net.de>

net: bcmgenet: convert RX path to page_pool

This commit replaces the per-packet allocation and DMA mapping in the RX path
with a page_pool implementation. It updates the refill and descriptor routines
to allocate from the pool and build skbs using napi_build_skb.

> diff --git a/drivers/net/ethernet/broadcom/genet/bcmgenet.c b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
> index 54f71b1e85fc4..df11c4977e8f7 100644
> --- a/drivers/net/ethernet/broadcom/genet/bcmgenet.c
> +++ b/drivers/net/ethernet/broadcom/genet/bcmgenet.c

[ ... ]

> @@ -2367,15 +2355,23 @@ static unsigned int bcmgenet_desc_rx(struct bcmgenet_rx_ring *ring,
>  		dma_flag = dma_length_status & 0xffff;
>  		len = dma_length_status >> DMA_BUFLENGTH_SHIFT;
>  
> +		/* Sync the rest of the actual received frame */
> +		if (len > sizeof(struct status_64))
> +			page_pool_dma_sync_for_cpu(ring->page_pool, rx_page,
> +						   sizeof(struct status_64),
> +						   len - sizeof(struct status_64));

Can this cause an out-of-bounds DMA sync regression?

The length is read from the hardware-written Receive Status Block and then
immediately used to sync the remainder of the frame data.

However, the bounds check against RX_BUF_LENGTH is performed after this
page_pool_dma_sync_for_cpu call.

If the hardware provides a corrupted or abnormally large length, could the
driver attempt to sync memory outside of the mapped page pool buffer? On
systems using SWIOTLB, passing a sync size larger than the mapped region
might lead to out-of-bounds copies and memory corruption.

>  		netif_dbg(priv, rx_status, dev,
>  			  "%s:p_ind=%d c_ind=%d read_ptr=%d len_stat=0x%08x\n",
>  			  __func__, p_index, ring->c_index,
>  			  ring->read_ptr, dma_length_status);
>  
> -		if (unlikely(len > RX_BUF_LENGTH)) {
> -			netif_err(priv, rx_status, dev, "oversized packet\n");
> +		if (unlikely(len > RX_BUF_LENGTH || len < GENET_RSB_PAD)) {
> +			netif_err(priv, rx_status, dev,
> +				  "invalid packet length %d\n", len);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260506095553.55357-1-nb@tipi-net.de?part=1