Message ID | 20220321133241.121367-1-shreeya.patel@collabora.com |
---|---|
State | Accepted |
Commit | 5467801f1fcbdc46bc7298a84dbf3ca1ff2a7320 |
Headers | show |
Series | [v3] gpio: Restrict usage of GPIO chip irq members before initialization | expand |
On Mon, Mar 21, 2022 at 2:33 PM Shreeya Patel <shreeya.patel@collabora.com> wrote: > GPIO chip irq members are exposed before they could be completely > initialized and this leads to race conditions. > > 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. > > Following are the logs for reference :- > > kernel: Call Trace: > kernel: gpiod_to_irq+0x53/0x70 > kernel: acpi_dev_gpio_irq_get_by+0x113/0x1f0 > kernel: i2c_acpi_get_irq+0xc0/0xd0 > kernel: i2c_device_probe+0x28a/0x2a0 > kernel: really_probe+0xf2/0x460 > kernel: RIP: 0010:gpiochip_to_irq+0x47/0xc0 > > To avoid such scenarios, restrict usage of GPIO chip irq members before > they are completely initialized. > > Signed-off-by: Shreeya Patel <shreeya.patel@collabora.com> Reviewed-by: Linus Walleij <linus.walleij@linaro.org> Yours, Linus Walleij
diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c index defb7c464b87..4ff68f48b87f 100644 --- a/drivers/gpio/gpiolib.c +++ b/drivers/gpio/gpiolib.c @@ -1404,6 +1404,16 @@ static int gpiochip_to_irq(struct gpio_chip *gc, unsigned int offset) { struct irq_domain *domain = gc->irq.domain; +#ifdef CONFIG_GPIOLIB_IRQCHIP + /* + * Avoid race condition with other code, which tries to lookup + * an IRQ before the irqchip has been properly registered, + * i.e. while gpiochip is still being brought up. + */ + if (!gc->irq.initialized) + return -EPROBE_DEFER; +#endif + if (!gpiochip_irqchip_irq_valid(gc, offset)) return -ENXIO; @@ -1593,6 +1603,15 @@ static int gpiochip_add_irqchip(struct gpio_chip *gc, acpi_gpiochip_request_interrupts(gc); + /* + * Using barrier() here to prevent compiler from reordering + * gc->irq.initialized before initialization of above + * GPIO chip irq members. + */ + barrier(); + + gc->irq.initialized = true; + return 0; } diff --git a/include/linux/gpio/driver.h b/include/linux/gpio/driver.h index b0728c8ad90c..f8996b46f430 100644 --- a/include/linux/gpio/driver.h +++ b/include/linux/gpio/driver.h @@ -218,6 +218,15 @@ struct gpio_irq_chip { */ bool per_parent_data; + /** + * @initialized: + * + * Flag to track GPIO chip irq member's initialization. + * This flag will make sure GPIO chip irq members are not used + * before they are initialized. + */ + bool initialized; + /** * @init_hw: optional routine to initialize hardware before * an IRQ chip will be added. This is quite useful when
GPIO chip irq members are exposed before they could be completely initialized and this leads to race conditions. 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. Following are the logs for reference :- kernel: Call Trace: kernel: gpiod_to_irq+0x53/0x70 kernel: acpi_dev_gpio_irq_get_by+0x113/0x1f0 kernel: i2c_acpi_get_irq+0xc0/0xd0 kernel: i2c_device_probe+0x28a/0x2a0 kernel: really_probe+0xf2/0x460 kernel: RIP: 0010:gpiochip_to_irq+0x47/0xc0 To avoid such scenarios, restrict usage of GPIO chip irq members before they are completely initialized. Signed-off-by: Shreeya Patel <shreeya.patel@collabora.com> --- Changes in v3 - Move the gc->irq.initialized check inside gpiochip_to_irq(). - Rename gc to GPIO chip. - Add barrier() to avoid compiler reordering. Changes in v2 - Make gc_irq_initialized flag a member of gpio_irq_chip structure. - Make use of barrier() to avoid reordering of flag initialization before other gc irq members are initialized. drivers/gpio/gpiolib.c | 19 +++++++++++++++++++ include/linux/gpio/driver.h | 9 +++++++++ 2 files changed, 28 insertions(+)