Re: [PATCH net] net: ibm: emac: Fix NULL pointer dereference in emac_probe
From: Andrew Lunn
Date: Sun May 31 2026 - 19:07:18 EST
On Sun, May 31, 2026 at 01:52:04PM -0700, Rosen Penev wrote:
> On Sun, May 31, 2026 at 7:39 AM Andrew Lunn <andrew@xxxxxxx> wrote:
> >
> > On Sat, May 30, 2026 at 07:22:19PM -0700, Rosen Penev wrote:
> > > Move devm_request_irq() after devm_platform_ioremap_resource() so that
> > > dev->emacp is mapped before the interrupt handler can fire. An early
> > > interrupt hitting emac_irq() would dereference the NULL dev->emacp and
> > > crash.
> >
> > Please add an explanation how this can happen, given that iser is
> > written in open() not probe().
> https://sashiko.dev/#/patchset/20260517033621.1839397-1-rosenp%40gmail.com
>
> Also, does requesting the hardware interrupt before mapping the device I/O
> registers create a NULL pointer dereference regression in emac_probe()?
> The hardware interrupt is requested and enabled via devm_request_irq()
> before the device I/O registers are mapped via
> devm_platform_ioremap_resource().
> If an interrupt is asserted early, emac_irq() will execute immediately and
> attempt to read the ISR register by dereferencing dev->emacp. Since
> dev->emacp is still NULL at this point, this would cause a kernel panic.
You have still not explained how an interrupt could happen before the
interrupts are enabled by writing to the iser in open(). I don't care
too much what the AI says, i want a human to explain how this can
happen. You have put your Signed-off-by: here, so it is down to you to
explain your patch, even is AI told you to do this, and you blindly
did it.
> Having devm_platform_ioremap_resource earlier
> like this is more consistent with other drivers as well.
So maybe this is a better thing to put in the commit message, rather
than something you cannot actually explain?
Andrew