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

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

[Karl Mehltretter]

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@vger.kernel.org
Assisted-by: LLM
Signed-off-by: Karl Mehltretter <kmehltretter@gmail.com>
---
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@gmail.com/

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
-- 
2.39.5 (Apple Git-154)

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

[Siddharth Vadapalli]

On 10/19/26 11:34 AM, 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@vger.kernel.org
> Assisted-by: LLM
> Signed-off-by: Karl Mehltretter <kmehltretter@gmail.com>
> ---
> 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@gmail.com/
> 
> 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);

Although the above code is a refactor, it appears to me that the correct 
sequence should be:
	disable_work_sync(&priv->rx_mode_work);
	unregister_netdev(ndev);
RX Mode work (cpsw_ndo_set_rx_mode_work) does have a 'netif_running' check 
so it might still be fine if the sequence isn't followed.

Please correct me if I am mistaken regarding the above. Rest of the patch 
looks good to me.

Regards,
Siddharth.

> +	}
> +}
> +
>   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

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

[Karl Mehltretter]

On Mon, Oct 19, 2026 at 12:19:11PM +0100, Siddharth Vadapalli wrote:
> > +		unregister_netdev(ndev);
> > +		disable_work_sync(&priv->rx_mode_work);
> 
> Although the above code is a refactor, it appears to me that the correct
> sequence should be:
> 	disable_work_sync(&priv->rx_mode_work);
> 	unregister_netdev(ndev);
> RX Mode work (cpsw_ndo_set_rx_mode_work) does have a 'netif_running' check
> so it might still be fine if the sequence isn't followed.
> 
> Please correct me if I am mistaken regarding the above. Rest of the patch
> looks good to me.
> 

Both orders are safe here. The worker and unregister_netdev() serialize
on RTNL, so a worker running after unregister exits at !netif_running().

This matches cpsw_new, so I'd keep the current order.

Thanks,
Karl

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@vger.kernel.org
> Assisted-by: LLM
> Signed-off-by: Karl Mehltretter <kmehltretter@gmail.com>

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

> ---
> 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@gmail.com/
> 
> 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