Re: [PATCH net v2 3/4] net: mvpp2: do not return retired RX buffers to BM

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Fixing the UAF introduces a permanent leak of hardware Buffer Manager (BM) pool slots on RX refill failure, leading to a permanent RX stall.
--

commit 0804789e54c7ef2fb92f8bc16ab030a2285b6149
Author: Til Kaiser <mail@tk154.de>

net: mvpp2: do not return retired RX buffers to BM

This patch splits the RX error handling path to avoid returning buffers
to the hardware Buffer Manager (BM) pool after they have been passed to
XDP or built into an skb.

> diff --git a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
> index 3372ed27cc8d6..397aa5ca4992c 100644
> --- a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
> +++ b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
> @@ -3991,7 +3991,7 @@ static int mvpp2_rx(struct mvpp2_port *port, struct napi_struct *napi,
>  				err = mvpp2_rx_refill(port, bm_pool, pp, pool);
>  				if (err) {
>  					netdev_err(port->dev, "failed to refill BM pools\n");
> -					goto err_drop_frame;
> +					goto err_drop_frame_retired;

[Severity: High]
Does skipping the buffer return here cause a permanent hardware pool leak?

By jumping to err_drop_frame_retired, we no longer put a replacement buffer
into the hardware Buffer Manager (BM) pool. If mvpp2_rx_refill() repeatedly
fails under memory pressure, will this drain the BM pool slots to zero and
permanently stall packet reception?

To avoid returning an already-consumed buffer while keeping the BM pool
full, would it be safer to perform the buffer allocation and refill
before passing the original buffer to XDP?

[ ... ]

> @@ -4023,13 +4025,16 @@ static int mvpp2_rx(struct mvpp2_port *port, struct napi_struct *napi,
>  		err = mvpp2_rx_refill(port, bm_pool, pp, pool);
>  		if (err) {
>  			netdev_err(port->dev, "failed to refill BM pools\n");
> +			if (!pp)
> +				dma_unmap_single_attrs(dev->dev.parent, dma_addr,
> +						       bm_pool->buf_size,
> +						       DMA_FROM_DEVICE,
> +						       DMA_ATTR_SKIP_CPU_SYNC);
>  			dev_kfree_skb_any(skb);
> -			goto err_drop_frame;
> +			goto err_drop_frame_retired;

[Severity: High]
Does this path also permanently lose a hardware buffer slot?

Since the successfully built skb is freed and we jump to
err_drop_frame_retired, the hardware BM pool does not get replenished.

Could the refill be done before calling build_skb() so the original buffer
can be safely returned to the hardware pool if allocation fails?

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260602164635.62517-1-mail@tk154.de?part=3