[PATCH] net: ethernet: arc: fix use-after-free in probe error path

[PATCH] net: ethernet: arc: fix use-after-free in probe error path

[Fan Wu]

The arc_emac_probe() function calls devm_request_irq() with the
net_device as the dev_id. However, in the error path of
emac_rockchip_probe(), free_netdev(ndev) is called before the devm
cleanup happens. This creates a race window where an interrupt can
fire and the ISR (arc_emac_intr) will access the already freed
net_device structure.

Race window:

    CPU 0 (probe error path)           CPU 1 (interrupt)
    ------------------------           ------------------
    emac_rockchip_probe()
      arc_emac_probe()
        devm_request_irq(..., ndev)
      [probe fails]
      goto out_netdev;

      free_netdev(ndev)  // freed!
                                       <Interrupt fires>
                                       arc_emac_intr()
                                         ndev = dev_instance
                                         priv = netdev_priv(ndev)
                                         // UAF! Accessing freed mem

    return err;

    devres_release_all()  // Driver core cleanup
      devm_irq_release()  // IRQ disabled too late!

Fix this by using devm_alloc_etherdev() instead of alloc_etherdev().
With fully managed allocation, the devres mechanism ensures proper
LIFO cleanup order: IRQ is released before net_device memory, thus
eliminating the race window entirely.

Remove the now-unnecessary free_netdev() calls from both the probe
error path and the remove function, as the memory is automatically
freed by devres when the device is detached.

Fixes: 6eacf31139bf ("ethernet: arc: Add support for Rockchip SoC layer device tree bindings")
Signed-off-by: Fan Wu <fanwu01@zju.edu.cn>
---
 drivers/net/ethernet/arc/emac_rockchip.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/arc/emac_rockchip.c b/drivers/net/ethernet/arc/emac_rockchip.c
index 780e70ea1c22..a695ea6547c8 100644
--- a/drivers/net/ethernet/arc/emac_rockchip.c
+++ b/drivers/net/ethernet/arc/emac_rockchip.c
@@ -103,7 +103,7 @@ static int emac_rockchip_probe(struct platform_device *pdev)
 	if (!pdev->dev.of_node)
 		return -ENODEV;
 
-	ndev = alloc_etherdev(sizeof(struct rockchip_priv_data));
+	ndev = devm_alloc_etherdev(dev, sizeof(struct rockchip_priv_data));
 	if (!ndev)
 		return -ENOMEM;
 	platform_set_drvdata(pdev, ndev);
@@ -240,7 +240,6 @@ static int emac_rockchip_probe(struct platform_device *pdev)
 out_clk_disable:
 	clk_disable_unprepare(priv->refclk);
 out_netdev:
-	free_netdev(ndev);
 	return err;
 }
 
@@ -258,8 +257,6 @@ static void emac_rockchip_remove(struct platform_device *pdev)
 
 	if (priv->soc_data->need_div_macclk)
 		clk_disable_unprepare(priv->macclk);
-
-	free_netdev(ndev);
 }
 
 static struct platform_driver emac_rockchip_driver = {
-- 
2.34.1

Re: [PATCH] net: ethernet: arc: fix use-after-free in probe error path

[Andrew Lunn]

On Wed, Mar 04, 2026 at 02:53:03AM +0000, Fan Wu wrote:
> The arc_emac_probe() function calls devm_request_irq() with the
> net_device as the dev_id. However, in the error path of
> emac_rockchip_probe(), free_netdev(ndev) is called before the devm
> cleanup happens. This creates a race window where an interrupt can
> fire and the ISR (arc_emac_intr) will access the already freed
> net_device structure.

It looks like interrupts are only enabled in arc_emac_open(). Have you
seen interrupts before this?

     Andrew

Re: Re: [PATCH] net: ethernet: arc: fix use-after-free in probe error path

[吴凡]

You are right that normal device interrupt generation is enabled in arc_emac_open() via R_ENABLE, so we certainly don't expect regular RX/TX traffic interrupts during probe.

My main concern here is the lifetime ordering in the error path. arc_emac_probe() installs the IRQ handler via devm_request_irq(..., ndev), but if emac_rockchip_probe() fails later, it explicitly calls free_netdev(ndev) well before the devres cleanup routine runs.

In that specific gap, if an IRQ is somehow delivered—perhaps from a pending/latched line left by the firmware/bootloader, or other non-traffic anomalies—arc_emac_intr() will immediately dereference dev_id as a struct net_device *. Since ndev has already been manually freed, this results in a UAF.

So while I completely agree this isn't a normal pre-open traffic path, the mixed lifetime management (managed IRQ vs. manual netdev free) still creates a real race window.

Switching to devm_alloc_etherdev() puts both resources under devres management, permanently fixing this teardown ordering issue. I would be happy to send a v2 and reword the commit log to emphasize this as a potential race window and a hardening fix. Let me know what you think.


> -----Original Messages-----
> From: "Andrew Lunn" <andrew@lunn.ch>
> Send time:Thursday, 05/03/2026 06:29:05
> To: "Fan Wu" <fanwu01@zju.edu.cn>
> Cc: netdev@vger.kernel.org, davem@davemloft.net, kuba@kernel.org, edumazet@google.com, pabeni@redhat.com, andrew+netdev@lunn.ch, heiko@sntech.de, romain.perier@gmail.com, linux-arm-kernel@lists.infradead.org, linux-rockchip@lists.infradead.org, stable@vger.kernel.org
> Subject: Re: [PATCH] net: ethernet: arc: fix use-after-free in probe error path
> 
> On Wed, Mar 04, 2026 at 02:53:03AM +0000, Fan Wu wrote:
> > The arc_emac_probe() function calls devm_request_irq() with the
> > net_device as the dev_id. However, in the error path of
> > emac_rockchip_probe(), free_netdev(ndev) is called before the devm
> > cleanup happens. This creates a race window where an interrupt can
> > fire and the ISR (arc_emac_intr) will access the already freed
> > net_device structure.
> 
> It looks like interrupts are only enabled in arc_emac_open(). Have you
> seen interrupts before this?
> 
>      Andrew

Re: Re: [PATCH] net: ethernet: arc: fix use-after-free in probe error path

[Andrew Lunn]

On Sun, Mar 08, 2026 at 04:56:08PM +0800, 吴凡 wrote:
> You are right that normal device interrupt generation is enabled in arc_emac_open() via R_ENABLE, so we certainly don't expect regular RX/TX traffic interrupts during probe.
> 
> My main concern here is the lifetime ordering in the error path. arc_emac_probe() installs the IRQ handler via devm_request_irq(..., ndev), but if emac_rockchip_probe() fails later, it explicitly calls free_netdev(ndev) well before the devres cleanup routine runs.
> 
> In that specific gap, if an IRQ is somehow delivered—perhaps from a pending/latched line left by the firmware/bootloader, or other non-traffic anomalies—arc_emac_intr() will immediately dereference dev_id as a struct net_device *. Since ndev has already been manually freed, this results in a UAF.
> 
> So while I completely agree this isn't a normal pre-open traffic path, the mixed lifetime management (managed IRQ vs. manual netdev free) still creates a real race window.
> 
> Switching to devm_alloc_etherdev() puts both resources under devres management, permanently fixing this teardown ordering issue. I would be happy to send a v2 and reword the commit log to emphasize this as a potential race window and a hardening fix. Let me know what you think.

https://www.kernel.org/doc/html/latest/process/maintainer-netdev.html#clean-up-patches

  1.7.4. Clean-up patches¶

  Netdev discourages patches which perform simple clean-ups, which are
  not in the context of other work. For example:

  Addressing checkpatch.pl, and other trivial coding style warnings

  Addressing Local variable ordering issues

  Conversions to device-managed APIs (devm_ helpers)

  This is because it is felt that the churn that such changes produce
  comes at a greater cost than the value of such clean-ups.


Some percentage of devm_ conversation patches break drivers. We
Reviewers need to look at all such patches and try to detect such
breakage. In general, it is nor worth it. Hence we generally reject
patches like this.

This is however slightly different. It looks like this driver was
broken from the beginning. The race you point out has always been
there. That is something worth pointing out in the commit message.

But take a step back. Think about interrupt handling in general. Do
you think it is good practice to request interrupts before configuring
the hardware about what interrupts it will deliver?

If the driver wrote to R_ENABLE in probe, before requesting the
interrupt, enabled the needed interrupts in open, disabled the
interrupts in close, the different lifetimes would not matter.

So, for stable, please add code to put interrupts into a well known
state before requesting the interrupt. Please use the net tree, and
add a Fixes: tag.

You can submit this patch to net-next, but we might reject it, because
of the policy. If you are working on this driver, adding other
features, this patch is part of a bigger patchset, we are more likely
to accept it.

    Andrew

---
pw-bot: cr

Re: Re: Re: [PATCH] net: ethernet: arc: fix use-after-free in probe error path

[Fan Wu]

Hi Andrew,

Thanks for the detailed explanation—that makes a lot of sense.

I agree the devm conversion is not the right approach for this tree. I'll prepare a v2 patch targeting net that implements a minimal hardware-level fix.

Following your advice, I will make sure the interrupts are put into a known disabled and cleared state in arc_emac_probe() before we request the IRQ. This will completely eliminate the possibility of an IRQ delivery against a freed ndev during the probe error path teardown.

I will update the changelog accordingly to note that this race has been there from the beginning and ensure the proper Fixes: tag is added for the stable backports.

Thanks, 
Fan Wu


> -----Original Messages-----
> From: "Andrew Lunn" <andrew@lunn.ch>
> Send time:Monday, 09/03/2026 01:59:46
> To: 吴凡 <12321260@zju.edu.cn>
> Cc: netdev@vger.kernel.org, davem@davemloft.net, kuba@kernel.org, edumazet@google.com, pabeni@redhat.com, andrew+netdev@lunn.ch, heiko@sntech.de, romain.perier@gmail.com, linux-arm-kernel@lists.infradead.org, linux-rockchip@lists.infradead.org, stable@vger.kernel.org
> Subject: Re: Re: [PATCH] net: ethernet: arc: fix use-after-free in probe error path
> 
> On Sun, Mar 08, 2026 at 04:56:08PM +0800, 吴凡 wrote:
> > You are right that normal device interrupt generation is enabled in arc_emac_open() via R_ENABLE, so we certainly don't expect regular RX/TX traffic interrupts during probe.
> > 
> > My main concern here is the lifetime ordering in the error path. arc_emac_probe() installs the IRQ handler via devm_request_irq(..., ndev), but if emac_rockchip_probe() fails later, it explicitly calls free_netdev(ndev) well before the devres cleanup routine runs.
> > 
> > In that specific gap, if an IRQ is somehow delivered—perhaps from a pending/latched line left by the firmware/bootloader, or other non-traffic anomalies—arc_emac_intr() will immediately dereference dev_id as a struct net_device *. Since ndev has already been manually freed, this results in a UAF.
> > 
> > So while I completely agree this isn't a normal pre-open traffic path, the mixed lifetime management (managed IRQ vs. manual netdev free) still creates a real race window.
> > 
> > Switching to devm_alloc_etherdev() puts both resources under devres management, permanently fixing this teardown ordering issue. I would be happy to send a v2 and reword the commit log to emphasize this as a potential race window and a hardening fix. Let me know what you think.
> 
> https://www.kernel.org/doc/html/latest/process/maintainer-netdev.html#clean-up-patches
> 
>   1.7.4. Clean-up patches¶
> 
>   Netdev discourages patches which perform simple clean-ups, which are
>   not in the context of other work. For example:
> 
>   Addressing checkpatch.pl, and other trivial coding style warnings
> 
>   Addressing Local variable ordering issues
> 
>   Conversions to device-managed APIs (devm_ helpers)
> 
>   This is because it is felt that the churn that such changes produce
>   comes at a greater cost than the value of such clean-ups.
> 
> 
> Some percentage of devm_ conversation patches break drivers. We
> Reviewers need to look at all such patches and try to detect such
> breakage. In general, it is nor worth it. Hence we generally reject
> patches like this.
> 
> This is however slightly different. It looks like this driver was
> broken from the beginning. The race you point out has always been
> there. That is something worth pointing out in the commit message.
> 
> But take a step back. Think about interrupt handling in general. Do
> you think it is good practice to request interrupts before configuring
> the hardware about what interrupts it will deliver?
> 
> If the driver wrote to R_ENABLE in probe, before requesting the
> interrupt, enabled the needed interrupts in open, disabled the
> interrupts in close, the different lifetimes would not matter.
> 
> So, for stable, please add code to put interrupts into a well known
> state before requesting the interrupt. Please use the net tree, and
> add a Fixes: tag.
> 
> You can submit this patch to net-next, but we might reject it, because
> of the policy. If you are working on this driver, adding other
> features, this patch is part of a bigger patchset, we are more likely
> to accept it.
> 
>     Andrew
> 
> ---
> pw-bot: cr