[PATCH DRAFT] net: cirrus: ep93xx: fix probe error unwind

[PATCH DRAFT] net: cirrus: ep93xx: fix probe error unwind

[박명훈]

From: Myeonghun Pak <mhun512@gmail.com>

ep93xx_eth_probe() uses ep93xx_eth_remove() as common error unwind
after setting driver data. When register_netdev() fails, this calls
unregister_netdev() for a net_device that was never registered. The
net core treats that as a driver bug and emits WARN_ON(1).

The shared remove path also fails to unmap the register mapping on
earlier allocation and resource-reservation failures, because
ep->base_addr is only assigned after request_mem_region() succeeds.

Unwind probe failures directly and publish driver data only after the
netdev is registered. This keeps .remove() for successful probes only,
releases the memory region on late failures, frees the net_device, and
unmaps the registers.

Fixes: 1d22e05df818 ("[PATCH] Cirrus Logic ep93xx ethernet driver")
Co-developed-by: Ijae Kim <ae878000@gmail.com>
Signed-off-by: Ijae Kim <ae878000@gmail.com>
Signed-off-by: Myeonghun Pak <mhun512@gmail.com>
---
diff --git a/drivers/net/ethernet/cirrus/ep93xx_eth.c b/drivers/net/ethernet/cirrus/ep93xx_eth.c
index a4972457ed..4e98b76ec7 100644
--- a/drivers/net/ethernet/cirrus/ep93xx_eth.c
+++ b/drivers/net/ethernet/cirrus/ep93xx_eth.c
@@ -800,7 +800,7 @@ static int ep93xx_eth_probe(struct platform_device *pdev)
 	dev = alloc_etherdev(sizeof(struct ep93xx_priv));
 	if (dev == NULL) {
 		err = -ENOMEM;
-		goto err_out;
+		goto err_iounmap;
 	}
 
 	memcpy_fromio(addr, base_addr + 0x50, ETH_ALEN);
@@ -814,14 +814,12 @@ static int ep93xx_eth_probe(struct platform_device *pdev)
 	SET_NETDEV_DEV(dev, &pdev->dev);
 	netif_napi_add(dev, &ep->napi, ep93xx_poll);
 
-	platform_set_drvdata(pdev, dev);
-
 	ep->res = request_mem_region(mem->start, resource_size(mem),
 				     dev_name(&pdev->dev));
 	if (ep->res == NULL) {
 		dev_err(&pdev->dev, "Could not reserve memory region\n");
 		err = -ENOMEM;
-		goto err_out;
+		goto err_free_netdev;
 	}
 
 	ep->base_addr = base_addr;
@@ -841,16 +839,22 @@ static int ep93xx_eth_probe(struct platform_device *pdev)
 	err = register_netdev(dev);
 	if (err) {
 		dev_err(&pdev->dev, "Failed to register netdev\n");
-		goto err_out;
+		goto err_release_mem;
 	}
 
+	platform_set_drvdata(pdev, dev);
+
 	printk(KERN_INFO "%s: ep93xx on-chip ethernet, IRQ %d, %pM\n",
 			dev->name, ep->irq, dev->dev_addr);
 
 	return 0;
 
-err_out:
-	ep93xx_eth_remove(pdev);
+err_release_mem:
+	release_mem_region(mem->start, resource_size(mem));
+err_free_netdev:
+	free_netdev(dev);
+err_iounmap:
+	iounmap(base_addr);
 	return err;
 }
 

Re: [PATCH DRAFT] net: cirrus: ep93xx: fix probe error unwind

[Andrew Lunn]

On Sun, Apr 26, 2026 at 11:10:06PM +0900, 박명훈 wrote:
> From: Myeonghun Pak <mhun512@gmail.com>
> 
> ep93xx_eth_probe() uses ep93xx_eth_remove() as common error unwind
> after setting driver data. When register_netdev() fails, this calls
> unregister_netdev() for a net_device that was never registered. The
> net core treats that as a driver bug and emits WARN_ON(1).

How did you trigger the failure of register_netdev()?

    Andrew

Re: [PATCH DRAFT] net: cirrus: ep93xx: fix probe error unwind

[Andrew Lunn]

On Mon, Apr 27, 2026 at 12:18:33PM +0900, Myeonghun Pak wrote:
> For the repro, I used an x86 COMPILE_TEST QEMU boot with ep93xx_eth.ko loaded
> and a synthetic ep93xx-eth platform device. I configured failslab with a
> stacktrace filter so the injected allocation failure happened inside
> register_netdevice(). That made register_netdev() fail from ep93xx_eth_probe().

So totally synthetic, not real world.

This driver was added in 2006. Can you point to reports of this being
a real problem any time in the last 20 years? Is this a problem worth
fixing because it does actually happen?

> This was originally found as part of the results from an ongoing research
> project with my fellow researchers.

Please redirect your effort to drivers which are still in use. Drivers
from the last 5-10 years. Drivers which are worth fixing, and have
Maintainers who cares about the driver. And problems which are likely
to happen in the real world, not an synthetic setting.

     Andrew