Thread (6 messages) 6 messages, 3 authors, 4d ago

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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help