Re: Re: [PATCH] net: ethernet: arc: fix use-after-free in probe error path
From: Andrew Lunn <andrew@lunn.ch>
Date: 2026-03-08 18:00:02
Also in:
linux-arm-kernel, linux-rockchip, stable
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