Re: [PATCH net] net: ibm: emac: Fix NULL pointer dereference in emac_probe

[Rosen Penev]

On Sun, May 31, 2026 at 4:06 PM Andrew Lunn <andrew@xxxxxxx> wrote:
>
> 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.
Beats me. When I did the devm conversions, I tested the hardware to
make sure it comes up and is usable. Various probe ordering tests were
not done.

FWIW, the AI is probably correct. How correct it is is beyond my pay grade.
>
> > 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?
Sure. Although this patch is targeted towards net, not net-next. Just
saying it's more consistent will get a flat out rejection.
>
>      Andrew