Re: [PATCH] net: fman: guard IRQ handlers against pre-init interrupt
From: Paolo Abeni <pabeni@redhat.com>
Date: 2026-07-01 15:07:02
Also in:
lkml
On 6/29/26 10:45 AM, ZhaoJinming wrote:
quoted hunk ↗ jump to hunk
read_dts_node() registers shared interrupt handlers via devm_request_irq() with fman as the dev_id, passing it to fman_irq() and fman_err_irq(). At registration time, fman is only partially initialized -- kzalloc_obj() zero-initializes all fields, so fman->cfg and fman->fpm_regs are NULL. The handlers guard against incomplete initialization via: if (!is_init_done(fman->cfg)) return IRQ_NONE; However, is_init_done(NULL) returns true (intended to indicate that fman_init() has completed and cfg has been freed). This means when cfg is NULL before fman_config() allocates it, the guard does not take effect: is_init_done(NULL) -> returns true !true -> false guard skipped -> proceeds to dereference NULL fpm_regs If another device on the same shared IRQ line fires during the window between devm_request_irq() and fman_init(), the handler accesses NULL fman->fpm_regs via ioread32be(), causing a crash. The window includes of_platform_populate() which can be slow. Add an irq_ready flag to struct fman that is set to true only after fman_init() completes in fman_probe(). Check this flag at the start of fman_irq() and fman_err_irq() before any register access. Use READ_ONCE()/WRITE_ONCE() for the flag accesses since it is a cross-context shared variable written in process context and read in interrupt context. This issue was identified in code review during the discussion of a separate IRQF_SHARED UAF fix patch: https://lore.kernel.org/netdev/20260626162323.GE1310988@horms.kernel.org/ (local) Signed-off-by: ZhaoJinming <redacted> --- drivers/net/ethernet/freescale/fman/fman.c | 5 +++++ drivers/net/ethernet/freescale/fman/fman.h | 1 + 2 files changed, 6 insertions(+)diff --git a/drivers/net/ethernet/freescale/fman/fman.c b/drivers/net/ethernet/freescale/fman/fman.c index 013273a2de32..f14cb02d85a4 100644 --- a/drivers/net/ethernet/freescale/fman/fman.c +++ b/drivers/net/ethernet/freescale/fman/fman.c@@ -2510,6 +2510,8 @@ static irqreturn_t fman_err_irq(int irq, void *handle) struct fman_fpm_regs __iomem *fpm_rg; irqreturn_t single_ret, ret = IRQ_NONE; + if (!READ_ONCE(fman->irq_ready)) + return IRQ_NONE;
Sashiko noted that on weakly ordered arch this could not be enough: https://sashiko.dev/#/patchset/20260629084529.3709393-1-zhaojinming%40uniontech.com Also I have the feeling this is papering over the real issue. Why initializing the irq when the driver is not yet ready? Why don't move IRQ initialization later? /P