[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