Re: [net-next v24 3/7] net: mtip: Add buffers management functions to the L2 switch driver

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [Critical] mtip_free_buffers() is invoked unconditionally on the mtip_open() error path, causing Use-After-Free for shared ring buffers when usage_count > 0.
- [High] mtip_free_buffers() frees DMA-mapped buffers without ensuring the hardware MAC/DMA engine is stopped.
- [Medium] Pending transmit sk_buffs are leaked in mtip_free_buffers().
--

commit 4b7d8928772619aad35c821850421906a5fec249
Author: Lukasz Majewski <lukasz.majewski@mailbox.org>
Date:   Mon Jun 1 11:24:32 2026 +0000

net: mtip: Add buffers management functions to the L2 switch driver
    
This patch provides buffers management functions' content for the MTIP
L2 switch driver. It adds implementations for allocating and freeing
shared ring buffers and page pools.

> diff --git a/drivers/net/ethernet/freescale/mtipsw/mtipl2sw.c b/drivers/net/ethernet/freescale/mtipsw/mtipl2sw.c
> index 1d1005a42306d..5b5229ee411df 100644
> --- a/drivers/net/ethernet/freescale/mtipsw/mtipl2sw.c
> +++ b/drivers/net/ethernet/freescale/mtipsw/mtipl2sw.c
> @@ -892,11 +892,101 @@ static void mtip_get_drvinfo(struct net_device *dev,
>  
>  static void mtip_free_buffers(struct net_device *dev)
>  {
> +	struct mtip_ndev_priv *priv = netdev_priv(dev);
> +	struct switch_enet_private *fep = priv->fep;
> +	int i;
> +

[Severity: High]
Does this function ensure the hardware MAC and DMA engines are stopped
before freeing the buffers?

Looking at its caller in mtip_close(), mtip_free_buffers() is invoked when
fep->usage_count == 1. However, mtip_close() does not appear to explicitly
stop the MAC, disable the DMA engine, or reset the hardware before calling
this. It only disables the clock after mtip_free_buffers() returns:

	if (fep->usage_count == 1) {
		napi_disable(&fep->napi);
		netif_napi_del(&fep->napi);
		mtip_free_buffers(dev);
		clk_disable(fep->clk_ipg);
	}

If the hardware is still processing queued TX packets or receiving RX packets,
could it continue to perform DMA operations on the memory that has just been
freed and reallocated to the system?

[Severity: Critical]
Is it safe for the mtip_open() error path to call this function
unconditionally?

When a subsequent port is brought up (fep->usage_count > 0) and
mtip_mii_probe() fails, the error path goes to mtip_mii_probe_err and calls
mtip_free_buffers(dev) regardless of the usage count:

 mtip_mii_probe_err:
	mtip_free_buffers(dev);

Since these buffers are shared across all switch ports, won't this destroy
the page pool and free the TX bounce buffers while the first port's NAPI
and hardware DMA are still actively using them?

> +	for (i = 0; i < RX_RING_SIZE; i++) {
> +		if (!fep->page[i])
> +			continue;
> +
> +		page_pool_put_full_page(fep->page_pool,
> +					fep->page[i], false);
> +		fep->page[i] = NULL;
> +	}
> +
> +	page_pool_destroy(fep->page_pool);
> +	fep->page_pool = NULL;
> +
> +	for (i = 0; i < TX_RING_SIZE; i++) {
> +		kfree(fep->tx_bounce[i]);
> +		fep->tx_bounce[i] = NULL;
> +	}

[Severity: Medium]
Are pending transmit sk_buffs leaked here?

This cleanup correctly frees the fep->tx_bounce buffers, but it does not
seem to iterate over the fep->tx_skbuff array to free any pending socket
buffers that were queued for transmission. Will any sk_buff still in
tx_skbuff[i] when the interface is closed be permanently leaked?

> +}

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260601112437.2216043-1-lukasz.majewski@mailbox.org?part=3