Re: [PATCH net v2] net: cpsw: fix netdev cleanup on probe failure

[Sverdlin, Alexander]

Hi!

On Mon, 2026-10-19 at 08:04 +0200, Karl Mehltretter wrote:
> The legacy CPSW driver registers its netdevs before requesting IRQs. If
> a later IRQ request fails in dual-EMAC mode, the probe error path
> unregisters only the primary netdev. Devres then calls free_netdev() on
> the still-registered secondary netdev, triggering the reg_state BUG_ON.
> 
> Once registered, either interface can queue rx_mode_work through
> dev_set_rx_mode(). The error path does not drain the work before devres
> releases the netdev and its private data, allowing the worker to access
> freed memory.
> 
> Use the same per-port cleanup for probe failure and remove. Unregister
> each netdev whose reg_state is NETREG_REGISTERED, then drain its
> rx_mode_work. The state check is needed because cpsw_probe_dual_emac()
> stores the secondary pointer before register_netdev() succeeds.
> 
> The workqueue lifetime issue was found during an LLM-assisted review of
> the 6.12.112-rc1 stable series. Inspecting the same path then exposed
> the dual-EMAC cleanup bug.
> 
> An instrumented KASAN ARM kernel and a QEMU CPSW probe stub reproduced
> both failures by forcing the TX IRQ request to return -EBUSY. Not tested
> on real hardware.
> 
> Fixes: 070f9c658a59 ("net: ethernet: ti: cpsw: Push the request_irq function to the end of probe")
> Fixes: 0b8c878d1173 ("net: cpsw: Execute ndo_set_rx_mode callback in a work queue")
> Cc: stable@xxxxxxxxxxxxxxx
> Assisted-by: LLM
> Signed-off-by: Karl Mehltretter <kmehltretter@xxxxxxxxx>

Reviewed-by: Alexander Sverdlin <alexander.sverdlin@xxxxxxxxxxx>

> ---
> v2:
>   - fold the two fixes into one patch (Jakub)
>   - share registered-port cleanup between probe unwind and remove (Jakub)
>   - check NETREG_REGISTERED instead of adding a local flag (Jakub)
>   - add the requested discovery context (netdev bot)
>   - rebase on net
> 
> v1: https://lore.kernel.org/netdev/cover.1791035789.git.kmehltretter@xxxxxxxxx/
> 
> Tested:
>   - ARM W=1 cpsw.o builds with TI_CPSW=y and TI_CPSW=m
>   - KASAN ARM runs with a QEMU CPSW probe stub. A test hook held
>     rx_mode_work pending while the TX IRQ request failed with -EBUSY.
>     Both failures reproduced without the combined fix; v2 completed
>     single- and dual-EMAC cleanup without KASAN or BUG reports in two
>     runs per mode.
> 
>  drivers/net/ethernet/ti/cpsw.c | 33 ++++++++++++++++++++-------------
>  1 file changed, 20 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/net/ethernet/ti/cpsw.c b/drivers/net/ethernet/ti/cpsw.c
> index aa3531e844e87..01296c0582bf0 100644
> --- a/drivers/net/ethernet/ti/cpsw.c
> +++ b/drivers/net/ethernet/ti/cpsw.c
> @@ -1524,6 +1524,23 @@ static int cpsw_probe_dual_emac(struct cpsw_priv *priv)
>  	return ret;
>  }
>  
> +static void cpsw_unregister_ports(struct cpsw_common *cpsw)
> +{
> +	struct net_device *ndev;
> +	struct cpsw_priv *priv;
> +	int i;
> +
> +	for (i = 0; i < cpsw->data.slaves; i++) {
> +		ndev = cpsw->slaves[i].ndev;
> +		if (!ndev || ndev->reg_state != NETREG_REGISTERED)
> +			continue;
> +
> +		priv = netdev_priv(ndev);
> +		unregister_netdev(ndev);
> +		disable_work_sync(&priv->rx_mode_work);
> +	}
> +}
> +
>  static const struct of_device_id cpsw_of_mtable[] = {
>  	{ .compatible = "ti,cpsw"},
>  	{ .compatible = "ti,am335x-cpsw"},
> @@ -1764,7 +1781,7 @@ static int cpsw_probe(struct platform_device *pdev)
>  	return 0;
>  
>  clean_unregister_netdev_ret:
> -	unregister_netdev(ndev);
> +	cpsw_unregister_ports(cpsw);
>  clean_cpts:
>  	cpts_release(cpsw->cpts);
>  	cpdma_ctlr_destroy(cpsw->dma);
> @@ -1779,9 +1796,7 @@ static int cpsw_probe(struct platform_device *pdev)
>  static void cpsw_remove(struct platform_device *pdev)
>  {
>  	struct cpsw_common *cpsw = platform_get_drvdata(pdev);
> -	struct net_device *ndev;
> -	struct cpsw_priv *priv;
> -	int i, ret;
> +	int ret;
>  
>  	ret = pm_runtime_resume_and_get(&pdev->dev);
>  	if (ret < 0) {
> @@ -1793,15 +1808,7 @@ static void cpsw_remove(struct platform_device *pdev)
>  		return;
>  	}
>  
> -	for (i = 0; i < cpsw->data.slaves; i++) {
> -		ndev = cpsw->slaves[i].ndev;
> -		if (!ndev)
> -			continue;
> -
> -		priv = netdev_priv(ndev);
> -		unregister_netdev(ndev);
> -		disable_work_sync(&priv->rx_mode_work);
> -	}
> +	cpsw_unregister_ports(cpsw);
>  
>  	cpts_release(cpsw->cpts);
>  	cpdma_ctlr_destroy(cpsw->dma);
> 
> base-commit: 7ec48f396a1d89532c0537bc38b408b8c31ee37f

-- 
Alexander Sverdlin
Siemens AG
www.siemens.com