Messages in this thread | | | From | Andy Shevchenko <> | Date | Tue, 15 Mar 2022 13:00:56 +0200 | Subject | Re: [PATCH v2] gpio: Restrict usage of gc irq members before initialization |
| |
On Tue, Mar 15, 2022 at 12:38 PM Shreeya Patel <shreeya.patel@collabora.com> wrote:
Thanks for the update, my comments below.
> gc irq members are exposed before they could be completely
gc --> GPIO chip
> initialized and this leads to race conditions.
Any example here. like ~3-4 lines of the Oops in question?
> One such issue was observed for the gc->irq.domain variable which > was accessed through the I2C interface in gpiochip_to_irq() before > it could be initialized by gpiochip_add_irqchip(). This resulted in > Kernel NULL pointer dereference. > > To avoid such scenarios, restrict usage of gc irq members before
gc --> GPIO chip
> they are completely initialized.
...
> + /* > + * Using barrier() here to prevent compiler from reordering > + * gc->irq.gc_irq_initialized before initialization of above > + * gc irq members. > + */ > + barrier(); > + > + gc->irq.gc_irq_initialized = true;
There are too many duplications. Why not simply call it 'initialized'?
> - if (gc->to_irq) { > + if (gc->to_irq && gc->irq.gc_irq_initialized) {
Why can't this check be added into gpiochip_to_irq() ?
if (!gc->irq.initialized) return -ENXIO;
...
> + bool gc_irq_initialized;
Can you move it closer to .init_hw so it will be weakly grouped by logic similarities? Also see above.
-- With Best Regards, Andy Shevchenko
| |