[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 at lunn.ch>
> Send time:Monday, 09/03/2026 01:59:46
> To: 吴凡 <12321260 at zju.edu.cn>
> Cc: netdev at vger.kernel.org, davem at davemloft.net, kuba at kernel.org, edumazet at google.com, pabeni at redhat.com, andrew+netdev at lunn.ch, heiko at sntech.de, romain.perier at gmail.com, linux-arm-kernel at lists.infradead.org, linux-rockchip at lists.infradead.org, stable at 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