On Sun, Mar 22, 2020 at 7:14 PM Jonathan Cameron <jic23@xxxxxxxxxx> wrote:
> On Tue, 17 Mar 2020 12:18:09 +0200
> Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx> wrote:
>
> > With DEBUG_SHIRQ enabled we have a kernel crash
> >
> > [ 116.482696] BUG: kernel NULL pointer dereference, address: 0000000000000000
> >
> > ...
> >
> > [ 116.606571] Call Trace:
> > [ 116.609023] <IRQ>
> > [ 116.611047] complete+0x34/0x50
> > [ 116.614206] bmp085_eoc_irq+0x9/0x10 [bmp280]
> >
> > because DEBUG_SHIRQ mechanism fires an IRQ before registration and drivers
> > ought to be able to handle an interrupt happening before request_irq() returns.
> >
> > Fixes: aae953949651 ("iio: pressure: bmp280: add support for BMP085 EOC interrupt")
> > Cc: Linus Walleij <linus.walleij@xxxxxxxxxx>
> > Signed-off-by: Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx>
>
> Hmm. Nasty corner case but fair enough to fix it up.
> I guess we should audit other drivers for similar paths...
One can easily test, no device even needed (however in this case I have one)
...
> > - if (data->use_eoc)
> > - init_completion(&data->done);
> > + reinit_completion(&data->done);
>
> reinit on the completion when we don't even have an interrupt
> (hence data->use_eoc == false) seems excessive. Why did
> you drop the conditional?
It's harmless and gives less code I believe.
But if you insist I can put it back.
--
With Best Regards,
Andy Shevchenko
