[PATCH] net: ethernet: mtk_eth_soc: allocate dummy netdev before registering netdevs

[PATCH] net: ethernet: mtk_eth_soc: allocate dummy netdev before registering netdevs

[Sandeep Haemoon]

From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Sandeep Haemoon <sandeeph@loljews.com>
Date: Tue, 15 Sep 2026 22:00:00 +0300
Subject: [PATCH] net: ethernet: mtk_eth_soc: allocate dummy netdev before registering netdevs

The DMA ring and NAPI are shared between the multiple MAC netdevs and are
carried by an internal "dummy" netdev (eth->dummy_dev). mtk_probe() creates
that dummy device only after the MAC netdevs have been registered, so for a
brief window the netdevs are visible to the network core while
eth->dummy_dev is still NULL.

If anything brings the first netdev up during that window (e.g. netifd
opening the WAN link the instant the interface is registered), mtk_open()
takes the first-open path and runs mtk_start_dma() -> mtk_dma_init() ->
mtk_rx_alloc(), which calls __xdp_rxq_info_reg() with eth->dummy_dev ==
NULL. That triggers

  WARNING: CPU: ... Missing net_device from driver

in net/core/xdp.c and returns -ENODEV, so mtk_open() fails and the
interface (e.g. the WAN port on MediaTek MT7988) stays down until it is
manually brought up after probe has completed.

Allocate the dummy netdev and add the shared tx/rx NAPI to it before
registering the MAC netdevs, and free it from the probe error path instead
of the now-unneeded err_unreg_netdev cleanup.

Signed-off-by: Sandeep Haemoon <sandeeph@loljews.com>
---
 drivers/net/ethernet/mediatek/mtk_eth_soc.c | 31 ++++++++++++++--------
 1 file changed, 18 insertions(+), 13 deletions(-)

diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.c b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
index fd7a49a..ae6a19b 100644
--- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c
+++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
@@ -5337,6 +5337,22 @@ static int mtk_probe(struct platform_device *pdev)
 		}
 	}
 
+	/* we run 2 devices on the same DMA ring so we need a dummy device
+	 * for NAPI to work. Allocate it before registering the netdevs so a
+	 * concurrent ndo_open (e.g. netifd bringing the first netdev up) never
+	 * observes eth->dummy_dev == NULL, which would make mtk_rx_alloc() ->
+	 * __xdp_rxq_info_reg() warn ("Missing net_device from driver",
+	 * net/core/xdp.c) and leave the interface down on boot.
+	 */
+	eth->dummy_dev = alloc_netdev_dummy(0);
+	if (!eth->dummy_dev) {
+		err = -ENOMEM;
+		dev_err(eth->dev, "failed to allocated dummy device\n");
+		goto err_deinit_ppe;
+	}
+	netif_napi_add(eth->dummy_dev, &eth->tx_napi, mtk_napi_tx);
+	netif_napi_add(eth->dummy_dev, &eth->rx_napi, mtk_napi_rx);
+
 	for (i = 0; i < MTK_MAX_DEVS; i++) {
 		if (!eth->netdev[i])
 			continue;
@@ -5351,17 +5367,6 @@ static int mtk_probe(struct platform_device *pdev)
 				   eth->netdev[i]->base_addr, eth->irq[MTK_FE_IRQ_SHARED]);
 	}
 
-	/* we run 2 devices on the same DMA ring so we need a dummy device
-	 * for NAPI to work
-	 */
-	eth->dummy_dev = alloc_netdev_dummy(0);
-	if (!eth->dummy_dev) {
-		err = -ENOMEM;
-		dev_err(eth->dev, "failed to allocated dummy device\n");
-		goto err_unreg_netdev;
-	}
-	netif_napi_add(eth->dummy_dev, &eth->tx_napi, mtk_napi_tx);
-	netif_napi_add(eth->dummy_dev, &eth->rx_napi, mtk_napi_rx);
 
 	platform_set_drvdata(pdev, eth);
 	schedule_delayed_work(&eth->reset.monitor_work,
@@ -5369,9 +5374,9 @@ static int mtk_probe(struct platform_device *pdev)
 
 	return 0;
 
-err_unreg_netdev:
-	mtk_unreg_dev(eth);
 err_deinit_ppe:
+	if (eth->dummy_dev)
+		free_netdev(eth->dummy_dev);
 	mtk_ppe_deinit(eth);
 	mtk_mdio_cleanup(eth);
 err_free_dev:
-- 
2.39.2

Re: [PATCH] net: ethernet: mtk_eth_soc: allocate dummy netdev before registering netdevs

[Lorenzo Bianconi]

[-- Attachment #1: Type: text/plain, Size: 4060 bytes --]

> From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
> From: Sandeep Haemoon <sandeeph@loljews.com>
> Date: Tue, 15 Sep 2026 22:00:00 +0300
> Subject: [PATCH] net: ethernet: mtk_eth_soc: allocate dummy netdev before registering netdevs
> 
> The DMA ring and NAPI are shared between the multiple MAC netdevs and are
> carried by an internal "dummy" netdev (eth->dummy_dev). mtk_probe() creates
> that dummy device only after the MAC netdevs have been registered, so for a
> brief window the netdevs are visible to the network core while
> eth->dummy_dev is still NULL.
> 
> If anything brings the first netdev up during that window (e.g. netifd
> opening the WAN link the instant the interface is registered), mtk_open()
> takes the first-open path and runs mtk_start_dma() -> mtk_dma_init() ->
> mtk_rx_alloc(), which calls __xdp_rxq_info_reg() with eth->dummy_dev ==
> NULL. That triggers
> 
>   WARNING: CPU: ... Missing net_device from driver
> 
> in net/core/xdp.c and returns -ENODEV, so mtk_open() fails and the
> interface (e.g. the WAN port on MediaTek MT7988) stays down until it is
> manually brought up after probe has completed.
> 
> Allocate the dummy netdev and add the shared tx/rx NAPI to it before
> registering the MAC netdevs, and free it from the probe error path instead
> of the now-unneeded err_unreg_netdev cleanup.

I think the Fix is right, but there is a preliminary bug to fix I guess.
Can you please rebase your patch on top of the following one? Thanks.

https://lore.kernel.org/netdev/20260916-mtk_eth_soc-netdev-fix-v1-1-5dac50eb65b1@oss.qualcomm.com/T/#u

Regards,
Lorenzo

> 
> Signed-off-by: Sandeep Haemoon <sandeeph@loljews.com>
> ---
>  drivers/net/ethernet/mediatek/mtk_eth_soc.c | 31 ++++++++++++++--------
>  1 file changed, 18 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.c b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
> index fd7a49a..ae6a19b 100644
> --- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c
> +++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
> @@ -5337,6 +5337,22 @@ static int mtk_probe(struct platform_device *pdev)
>  		}
>  	}
>  
> +	/* we run 2 devices on the same DMA ring so we need a dummy device
> +	 * for NAPI to work. Allocate it before registering the netdevs so a
> +	 * concurrent ndo_open (e.g. netifd bringing the first netdev up) never
> +	 * observes eth->dummy_dev == NULL, which would make mtk_rx_alloc() ->
> +	 * __xdp_rxq_info_reg() warn ("Missing net_device from driver",
> +	 * net/core/xdp.c) and leave the interface down on boot.
> +	 */
> +	eth->dummy_dev = alloc_netdev_dummy(0);
> +	if (!eth->dummy_dev) {
> +		err = -ENOMEM;
> +		dev_err(eth->dev, "failed to allocated dummy device\n");
> +		goto err_deinit_ppe;
> +	}
> +	netif_napi_add(eth->dummy_dev, &eth->tx_napi, mtk_napi_tx);
> +	netif_napi_add(eth->dummy_dev, &eth->rx_napi, mtk_napi_rx);
> +
>  	for (i = 0; i < MTK_MAX_DEVS; i++) {
>  		if (!eth->netdev[i])
>  			continue;
> @@ -5351,17 +5367,6 @@ static int mtk_probe(struct platform_device *pdev)
>  				   eth->netdev[i]->base_addr, eth->irq[MTK_FE_IRQ_SHARED]);
>  	}
>  
> -	/* we run 2 devices on the same DMA ring so we need a dummy device
> -	 * for NAPI to work
> -	 */
> -	eth->dummy_dev = alloc_netdev_dummy(0);
> -	if (!eth->dummy_dev) {
> -		err = -ENOMEM;
> -		dev_err(eth->dev, "failed to allocated dummy device\n");
> -		goto err_unreg_netdev;
> -	}
> -	netif_napi_add(eth->dummy_dev, &eth->tx_napi, mtk_napi_tx);
> -	netif_napi_add(eth->dummy_dev, &eth->rx_napi, mtk_napi_rx);
>  
>  	platform_set_drvdata(pdev, eth);
>  	schedule_delayed_work(&eth->reset.monitor_work,
> @@ -5369,9 +5374,9 @@ static int mtk_probe(struct platform_device *pdev)
>  
>  	return 0;
>  
> -err_unreg_netdev:
> -	mtk_unreg_dev(eth);
>  err_deinit_ppe:
> +	if (eth->dummy_dev)
> +		free_netdev(eth->dummy_dev);
>  	mtk_ppe_deinit(eth);
>  	mtk_mdio_cleanup(eth);
>  err_free_dev:
> -- 
> 2.39.2
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH] net: ethernet: mtk_eth_soc: allocate dummy netdev before registering netdevs

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 6 potential
issue(s) to consider:

Critical: 0 · High: 3 (1) · Medium: 1 · Low: 2

- [High] New use-after-free window on mtk_probe()'s error path:
  eth->dummy_dev now carries eth->tx_napi/eth->rx_napi *before* the…
- [High] New leak of a registered notifier into freed memory when
  alloc_netdev_dummy() fails.
- [High] Frame engine can be left armed while probe tears down clocks and
  power.
- [Medium] Commit message and new in-code comment do not match the
  implementation.
- [Low] mtk_probe(): the patch fixes a user-visible regression (first
  netdev cannot be brought up during the probe window because…
- [Low] Cosmetic artifact of the code motion: after removing the
  dummy_dev allocation block that used to sit after the…

Pre-existing issues:
- [High] mtk_probe(): if register_netdev() fails for the second or later
  MAC (mtk_eth_soc.c:5360-5363), control jumps to err_deinit_ppe and…

--

net: ethernet: mtk_eth_soc: allocate dummy netdev before registering netdevs

This moves the eth->dummy_dev allocation and the netif_napi_add() calls for
the shared tx/rx NAPI ahead of the register_netdev() loop in mtk_probe(), so
a concurrent ndo_open cannot observe eth->dummy_dev == NULL and fail in
__xdp_rxq_info_reg().  The err_unreg_netdev label and its mtk_unreg_dev()
call are removed, and the dummy netdev is freed from err_deinit_ppe instead.

A few questions about the reordering and the new unwind path below.

>     Allocate the dummy netdev and add the shared tx/rx NAPI to it before
>     registering the MAC netdevs, and free it from the probe error path instead
>     of the now-unneeded err_unreg_netdev cleanup.
>
>     Signed-off-by: Sandeep Haemoon <sandeeph@loljews.com>

[Severity: Low]
Should this carry a Fixes: tag?

The NULL window described in the changelog was created by commit
b209bd6d0bff ("net: mediatek: mtk_eth_sock: allocate dummy net_device
dynamically"), which turned the embedded struct net_device dummy_dev into a
pointer allocated only after the register_netdev() loop.  Before that commit
the object was embedded, so mtk_create_page_pool() could never pass NULL to
__xdp_rxq_info_reg().

Without the tag, will stable tooling pick this up for the affected kernels?

> diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.c b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
> index fd7a49ae88d01..ae6a19bb3a418 100644
> --- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c
> +++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
> @@ -5337,6 +5337,22 @@ static int mtk_probe(struct platform_device *pdev)
>  		}
>  	}
>  
> +	/* we run 2 devices on the same DMA ring so we need a dummy device
> +	 * for NAPI to work. Allocate it before registering the netdevs so a
> +	 * concurrent ndo_open (e.g. netifd bringing the first netdev up) never
> +	 * observes eth->dummy_dev == NULL, which would make mtk_rx_alloc() ->
> +	 * __xdp_rxq_info_reg() warn ("Missing net_device from driver",
> +	 * net/core/xdp.c) and leave the interface down on boot.
> +	 */

[Severity: Medium]
Do the changelog and this comment match what the code does?

The changelog says the dummy netdev is freed "from the probe error path
instead of the now-unneeded err_unreg_netdev cleanup", but mtk_unreg_dev()
was the only netdev-unregistration step reachable from any mtk_probe() error
label, and the register_netdev() failure path still jumps into that unwind:

	err = register_netdev(eth->netdev[i]);
	if (err) {
		dev_err(eth->dev, "error bringing up device\n");
		goto err_deinit_ppe;

What remains on that path is mtk_free_dev(), which only frees:

	for (i = 0; i < MTK_MAX_DEVS; i++) {
		if (!eth->netdev[i])
			continue;
		free_netdev(eth->netdev[i]);
	}

Is the cleanup really "now-unneeded", or was it still required by that path?

The new comment also states that pre-allocating dummy_dev makes a concurrent
ndo_open safe.  Is that the whole story, given that the same concurrent open
is what leaves the shared NAPI enabled when the new error path frees its
carrier?

> +	eth->dummy_dev = alloc_netdev_dummy(0);
> +	if (!eth->dummy_dev) {
> +		err = -ENOMEM;
> +		dev_err(eth->dev, "failed to allocated dummy device\n");
> +		goto err_deinit_ppe;
> +	}

[Severity: High]
Can this allocation failure leave a registered notifier_block pointing at
freed memory?

On MTK_QDMA SoCs mtk_add_mac() registers a notifier embedded in the MAC
private area:

	mac->device_notifier.notifier_call = mtk_device_event;
	register_netdevice_notifier(&mac->device_notifier);

Previously an alloc_netdev_dummy() failure went to err_unreg_netdev ->
mtk_unreg_dev(), which does:

	if (MTK_HAS_CAPS(eth->soc->caps, MTK_QDMA))
		unregister_netdevice_notifier(&mac->device_notifier);
	unregister_netdev(eth->netdev[i]);

Now it goes to err_deinit_ppe and falls through to mtk_free_dev(), which
free_netdev()s the MAC netdevs without any unregister_netdevice_notifier()
call.  Does the notifier then stay on the global chain, so that the next
NETDEV_* event calls mtk_device_event() with container_of() on freed
netdev_priv() storage?

> +	netif_napi_add(eth->dummy_dev, &eth->tx_napi, mtk_napi_tx);
> +	netif_napi_add(eth->dummy_dev, &eth->rx_napi, mtk_napi_rx);
> +

[Severity: High]
Attaching the shared NAPI here means it is live before the netdevs become
openable.  Can the error path then free it underneath an enabled NAPI?

Per the changelog's own premise, netifd can open netdev[0] as soon as
register_netdev() returns, and with this patch that open now succeeds and
runs the first-open path in mtk_open():

	napi_enable(&eth->tx_napi);
	napi_enable(&eth->rx_napi);
	mtk_tx_irq_enable(eth, MTK_TX_DONE_INT);
	mtk_rx_irq_enable(eth, soc->rx.irq_done_mask);

If register_netdev() for a later MAC fails, err_deinit_ppe frees the carrier:

	free_netdev(eth->dummy_dev)
	  netdev_napi_exit()
	    __netif_napi_del_locked()
	      /* Make sure NAPI is disabled (or was never enabled). */
	      WARN_ON(!test_bit(NAPI_STATE_SCHED, &napi->state));

Does this trip that WARN_ON and leave eth->tx_napi.dev / eth->rx_napi.dev
pointing at freed memory while the devm-managed FE interrupts are still
installed and unmasked, so the next RX/TX interrupt schedules a poll on the
freed dummy netdev?

>  	for (i = 0; i < MTK_MAX_DEVS; i++) {
>  		if (!eth->netdev[i])
>  			continue;

[Severity: High]
This isn't a bug introduced by this patch, but the register loop's jump to
err_deinit_ppe was already there before the change, and this patch removes
the label that could have fixed it.

On a multi-MAC SoC, if register_netdev(eth->netdev[0]) succeeds and
register_netdev(eth->netdev[1]) fails, the unwind reaches mtk_free_dev(),
which calls free_netdev() on netdev[0] while it is still NETREG_REGISTERED:

	BUG_ON(dev->reg_state != NETREG_UNREGISTERED);

mtk_cleanup() documents the expected ordering:

	mtk_unreg_dev(eth);
	mtk_free_dev(eth);

Since this patch deletes the only probe-side mtk_unreg_dev() call, could the
unregister step be kept between the register loop and the dummy_dev free
instead?

> @@ -5351,17 +5367,6 @@ static int mtk_probe(struct platform_device *pdev)
>  				   eth->netdev[i]->base_addr, eth->irq[MTK_FE_IRQ_SHARED]);
>  	}
>  
> -	/* we run 2 devices on the same DMA ring so we need a dummy device
> -	 * for NAPI to work
> -	 */
> -	eth->dummy_dev = alloc_netdev_dummy(0);
> -	if (!eth->dummy_dev) {
> -		err = -ENOMEM;
> -		dev_err(eth->dev, "failed to allocated dummy device\n");
> -		goto err_unreg_netdev;
> -	}
> -	netif_napi_add(eth->dummy_dev, &eth->tx_napi, mtk_napi_tx);
> -	netif_napi_add(eth->dummy_dev, &eth->rx_napi, mtk_napi_rx);
>  
>  	platform_set_drvdata(pdev, eth);

[Severity: Low]
This isn't a bug, but removing the block leaves two consecutive blank lines
between the closing brace of the register loop and platform_set_drvdata(),
which checkpatch.pl flags as "CHECK: Please don't use multiple blank lines".

>  	schedule_delayed_work(&eth->reset.monitor_work,
> @@ -5369,9 +5374,9 @@ static int mtk_probe(struct platform_device *pdev)
>  
>  	return 0;
>  
> -err_unreg_netdev:
> -	mtk_unreg_dev(eth);
>  err_deinit_ppe:
> +	if (eth->dummy_dev)
> +		free_netdev(eth->dummy_dev);
>  	mtk_ppe_deinit(eth);
>  	mtk_mdio_cleanup(eth);
>  err_free_dev:

[Severity: High]
With no unregister step left on this path, can the frame engine stay armed
while probe gates its clocks and power?

Because the concurrent open now succeeds, mtk_open() -> mtk_start_dma() can
set MTK_TX_DMA_EN|MTK_RX_DMA_EN, program the rings, start the PPE and unmask
the TX/RX interrupts.  The only code that undoes that is mtk_stop():

	mtk_tx_irq_disable(eth, MTK_TX_DONE_INT);
	mtk_rx_irq_disable(eth, eth->soc->rx.irq_done_mask);
	napi_disable(&eth->tx_napi);
	napi_disable(&eth->rx_napi);
	...
	mtk_stop_dma(eth, eth->soc->reg_map->pdma.glo_cfg);
	mtk_dma_free(eth);

Nothing on this unwind triggers ndo_stop, and it continues into
mtk_ppe_deinit() (rhashtable_destroy with the PPE still started) and
mtk_hw_deinit():

	mtk_clk_disable(eth);

	pm_runtime_put_sync(eth->dev);
	pm_runtime_disable(eth->dev);

Does this drop the FE clocks and power domain with DMA still enabled and
interrupts still unmasked, and leak the descriptor rings and page-pool
buffers?  Since platform_set_drvdata() was never reached, mtk_remove()
cannot clean up afterwards either.

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/patch-12c1a57b1d914b28b89455f24de55ee4%40loljews.com

[PATCH net v2] net: ethernet: mtk_eth_soc: allocate dummy netdev before registering netdevs

[Sandeep Haemoon]

The RX rings and the shared NAPI are carried by an internal "dummy" netdev
(eth->dummy_dev). mtk_probe() creates that dummy device only after the MAC
netdevs have been registered, so for a brief window the netdevs are visible
to the network core while eth->dummy_dev is still NULL.

If anything brings the first netdev up during that window (e.g. netifd
opening the WAN link the instant the interface is registered), mtk_open()
takes the first-open path and runs mtk_start_dma() -> mtk_dma_init() ->
mtk_rx_alloc(), which calls __xdp_rxq_info_reg() with eth->dummy_dev ==
NULL. That triggers

  WARNING: CPU: ... Missing net_device from driver

in net/core/xdp.c and returns -ENODEV, so mtk_open() fails and the
interface stays down until it is manually brought up once probe finishes.

Allocate the dummy netdev and add the shared tx/rx NAPI to it before the
register_netdev() loop. Both the dummy-allocation failure and a
register_netdev() failure now unwind through mtk_unreg_dev(), which
unregisters the net_device notifiers and the netdevs registered so far -
stopping a concurrently-opened netdev (ndo_stop) and its NAPI before the
dummy carrier is freed. The never-registered netdevs are skipped thanks to
the NETREG_REGISTERED guard in mtk_unreg_dev() and freed directly.

Fixes: b209bd6d0bff ("net: mediatek: mtk_eth_sock: allocate dummy net_device dynamically")
Signed-off-by: Sandeep Haemoon <sandeeph@loljews.com>
---
Version 2:
  - Rebased on top of netdev/net.git, which now carries Lorenzo Bianconi's
    "mtk_eth_soc: unregister net_devices in case of probe failure" fix; the
    dummy allocation sits before the register_netdev() loop and both its
    failure and a register_netdev() failure unwind through mtk_unreg_dev(),
    unregistering the net_device notifiers and every netdev registered so
    far (never-registered ones are skipped by the NETREG_REGISTERED guard)
    before the dummy carrier is freed.
  - Error path stops any concurrently-opened netdev (ndo_stop, NAPI
    disabled) via that unregister, then frees the dummy carrier; a
    dummy-allocation failure likewise cleans the notifier chain that
    mtk_add_mac() registered.
  - Add Fixes: b209bd6d0bff, the commit that made eth->dummy_dev dynamic
    and introduced the NULL window.

 drivers/net/ethernet/mediatek/mtk_eth_soc.c | 33 +++++++++++++--------
 1 file changed, 21 insertions(+), 12 deletions(-)

diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.c b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
index 2ea5dfe85..5e7d8b8be 100644
--- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c
+++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
@@ -5341,6 +5341,22 @@ static int mtk_probe(struct platform_device *pdev)
 		}
 	}
 
+	/* we run 2 devices on the same DMA ring so we need a dummy device
+	 * for NAPI to work. Allocate it before registering the netdevs so a
+	 * concurrent ndo_open (e.g. netifd bringing the first netdev up the
+	 * instant it is registered) never observes eth->dummy_dev == NULL in
+	 * mtk_dma_init() -> mtk_rx_alloc() -> __xdp_rxq_info_reg() (net/core/xdp.c
+	 * "Missing net_device from driver") and fails the first open.
+	 */
+	eth->dummy_dev = alloc_netdev_dummy(0);
+	if (!eth->dummy_dev) {
+		err = -ENOMEM;
+		dev_err(eth->dev, "failed to allocated dummy device\n");
+		goto err_unreg_netdev;
+	}
+	netif_napi_add(eth->dummy_dev, &eth->tx_napi, mtk_napi_tx);
+	netif_napi_add(eth->dummy_dev, &eth->rx_napi, mtk_napi_rx);
+
 	for (i = 0; i < MTK_MAX_DEVS; i++) {
 		if (!eth->netdev[i])
 			continue;
@@ -5355,18 +5371,6 @@ static int mtk_probe(struct platform_device *pdev)
 				   eth->netdev[i]->base_addr, eth->irq[MTK_FE_IRQ_SHARED]);
 	}
 
-	/* we run 2 devices on the same DMA ring so we need a dummy device
-	 * for NAPI to work
-	 */
-	eth->dummy_dev = alloc_netdev_dummy(0);
-	if (!eth->dummy_dev) {
-		err = -ENOMEM;
-		dev_err(eth->dev, "failed to allocated dummy device\n");
-		goto err_unreg_netdev;
-	}
-	netif_napi_add(eth->dummy_dev, &eth->tx_napi, mtk_napi_tx);
-	netif_napi_add(eth->dummy_dev, &eth->rx_napi, mtk_napi_rx);
-
 	platform_set_drvdata(pdev, eth);
 	schedule_delayed_work(&eth->reset.monitor_work,
 			      MTK_DMA_MONITOR_TIMEOUT);
@@ -5376,6 +5380,11 @@ static int mtk_probe(struct platform_device *pdev)
 err_unreg_netdev:
 	mtk_unreg_dev(eth);
 err_deinit_ppe:
+	if (eth->dummy_dev) {
+		netif_napi_del(&eth->tx_napi);
+		netif_napi_del(&eth->rx_napi);
+		free_netdev(eth->dummy_dev);
+	}
 	mtk_ppe_deinit(eth);
 	mtk_mdio_cleanup(eth);
 err_free_dev:
-- 
2.53.0

Re: [PATCH net v2] net: ethernet: mtk_eth_soc: allocate dummy netdev before registering netdevs

[Jakub Kicinski]

This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
---
net: ethernet: mtk_eth_soc: allocate dummy netdev before registering netdevs

This patch moves the allocation of eth->dummy_dev, and the netif_napi_add()
calls for the shared tx/rx NAPI, ahead of the register_netdev() loop in
mtk_probe(). A netdev opened as soon as it is registered therefore no
longer sees a NULL dummy_dev. The error unwind also now deletes the NAPIs
and frees the dummy device.

> If anything brings the first netdev up during that window (e.g. netifd
> opening the WAN link the instant the interface is registered), mtk_open()
> takes the first-open path and runs mtk_start_dma() -> mtk_dma_init() ->
> mtk_rx_alloc(), which calls __xdp_rxq_info_reg() with eth->dummy_dev ==
> NULL. That triggers
>
>   WARNING: CPU: ... Missing net_device from driver
>
> in net/core/xdp.c and returns -ENODEV, so mtk_open() fails and the
> interface stays down until it is manually brought up once probe finishes.

[Severity: Low]
Is this the full impact of the race? mtk_rx_alloc() only uses dummy_dev
through mtk_create_page_pool(). That call happens only when
mtk_page_pool_enabled() is true, which means
mtk_is_netsys_v2_or_greater().

On NETSYS v1 SoCs (MT7621/MT7622/MT7623/MT7629), mtk_start_dma() succeeds.
mtk_open() then does:

    napi_enable(&eth->tx_napi);
    napi_enable(&eth->rx_napi);

At that point netif_napi_add() has not yet initialised these NAPIs. eth
comes from devm_kzalloc(), so tx_napi.dev is NULL, and
napi_enable() -> netdev_lock(n->dev) dereferences a NULL pointer.

This patch fixes that case too. Could the commit message mention the oops
on v1 SoCs? Stable backporters will use the description to judge urgency.

> Fixes: b209bd6d0bff ("net: mediatek: mtk_eth_sock: allocate dummy net_device dynamically")

[Severity: Medium]
Is this the right commit for the Fixes: tag? The v2 notes say b209bd6d0bff
"introduced the NULL window", but the ordering this patch corrects looks
older.

At b209bd6d0bff^, mtk_probe() already ran the register_netdev() loop
before init_dummy_netdev() and netif_napi_add(). The same order is in
656e705243fd ("net-next: mediatek: add support for MT7623 ethernet").

b209bd6d0bff only turned the embedded dummy_dev into a pointer, which
changed the symptom. In older kernels, an open in the same window reaches
napi_enable() on a zeroed napi_struct and hits:

    BUG_ON(!test_bit(NAPI_STATE_SCHED, &val));

With the current tag, would stable tooling skip LTS trees that lack
b209bd6d0bff (for example 6.6, 6.1, 5.15 and 5.10), even though they have
the same race?

> diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.c b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
> index 2ea5dfe85539b..5e7d8b8be7687 100644
> --- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c
> +++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
> @@ -5341,6 +5341,22 @@ static int mtk_probe(struct platform_device *pdev)

[ ... ]

> +	eth->dummy_dev = alloc_netdev_dummy(0);
> +	if (!eth->dummy_dev) {
> +		err = -ENOMEM;
> +		dev_err(eth->dev, "failed to allocated dummy device\n");
> +		goto err_unreg_netdev;
> +	}
> +	netif_napi_add(eth->dummy_dev, &eth->tx_napi, mtk_napi_tx);
> +	netif_napi_add(eth->dummy_dev, &eth->rx_napi, mtk_napi_rx);
> +
>  	for (i = 0; i < MTK_MAX_DEVS; i++) {
>  		if (!eth->netdev[i])
>  			continue;

[ ... ]

> @@ -5376,6 +5380,11 @@ static int mtk_probe(struct platform_device *pdev)
>  err_unreg_netdev:
>  	mtk_unreg_dev(eth);
>  err_deinit_ppe:
> +	if (eth->dummy_dev) {
> +		netif_napi_del(&eth->tx_napi);
> +		netif_napi_del(&eth->rx_napi);
> +		free_netdev(eth->dummy_dev);
> +	}

[Severity: High]
A MAC netdev can now open fully while mtk_probe() is still in the
register_netdev() loop. Can this unwind then leave eth->pending_work queued
or running?

Suppose a later register_netdev() fails, for example with -EINTR from
rtnl_net_lock_killable(). The unwind runs
mtk_unreg_dev() -> unregister_netdev() -> mtk_stop(), and mtk_stop() only
cancels rx_dim.work and tx_dim.work.

While netdev0 was up, the TX watchdog could have queued the reset worker:

mtk_tx_timeout()
    schedule_work(&eth->pending_work);

mtk_pending_work() takes rtnl_lock. It then uses eth, eth->netdev[] and the
hardware through mtk_prepare_for_reset(), mtk_stop()/mtk_open() and
mtk_hw_init().

If the work was queued while unregister_netdev() held rtnl, it can run as
soon as rtnl is dropped. That can overlap the rest of the unwind (freeing
dummy_dev, mtk_free_dev(), mtk_hw_deinit()), or come after devres has
freed eth.

The remove path handles this in mtk_cleanup():

    cancel_work_sync(&eth->pending_work);
    cancel_delayed_work_sync(&eth->reset.monitor_work);

Should the probe error path also call cancel_work_sync(&eth->pending_work)
after mtk_unreg_dev()? The changelog says the unwind stops any
concurrently-opened netdev before its resources are freed, and this case
seems to contradict that.

>  	mtk_ppe_deinit(eth);
>  	mtk_mdio_cleanup(eth);
>  err_free_dev:

[Severity: High]
This isn't a bug introduced by this patch, but the other error paths that
jump to err_deinit_ppe and err_free_dev look like they can leave a freed
notifier_block on the global netdev chain.

On MTK_QDMA SoCs, mtk_add_mac() does:

    if (MTK_HAS_CAPS(eth->soc->caps, MTK_QDMA)) {
        mac->device_notifier.notifier_call = mtk_device_event;
        register_netdevice_notifier(&mac->device_notifier);
    }

Here mac lives in the netdev private area. The notifier is only
unregistered by mtk_unreg_dev(), which is reached only through
err_unreg_netdev.

Two failure paths skip it:

- devm_request_irq() or mtk_mdio_init() fails and jumps to err_free_dev.
- mtk_ppe_init() or mtk_eth_offload_init() fails and jumps to
  err_deinit_ppe.

In both cases mtk_free_dev() -> free_netdev() kvfree()s the
NETREG_UNINITIALIZED netdev while its notifier is still registered.

Could the next netdev notifier walk then touch freed memory? The same
question applies to the next register_netdevice_notifier() call, for
example on re-probe after -EPROBE_DEFER from of_mdiobus_register().

Separately, if mtk_add_mac() fails for a second MAC, the goto err_deinit_hw
skips both mtk_unreg_dev() and mtk_free_dev(). Does that leak the first
MAC's netdev and leave its notifier registered, with mac->hw pointing at
the devm-freed eth?
-- 
pw-bot: cr