gpio: Request interrupts after IRQ is initialized
Subsystems (2)
A message that is not in the archive (sent off-list or not yet received): [email protected]
PATCH 1/2 gpio: Restrict usage of GPIO chip irq members before initialization
From: Shreeya Patel <[email protected]>
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.
--- drivers/gpio/gpiolib.c | 19 +++++++++++++++++++ include/linux/gpio/driver.h | 9 +++++++++ 2 files changed, 28 insertions(+)
drivers/gpio/gpiolib.c +19 -0
@@ -2146,6 +2146,16 @@ static int gpiochip_to_irq(struct gpio_chip *chip, unsigned offset) {struct irq_domain *domain = chip->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 (!chip->irq.initialized) +return -EPROBE_DEFER ; +#endif +if (!gpiochip_irqchip_irq_valid (chip, offset))return -ENXIO ;
@@ -2321,6 +2331,15 @@ static int gpiochip_add_irqchip(struct gpio_chip *gpiochip,acpi_gpiochip_request_interrupts (gpiochip); +/* +* Using barrier() here to prevent compiler from reordering +* gc->irq.initialized before initialization of above +* GPIO chip irq members. +*/ +barrier (); + + gpiochip->irq.initialized = true; +return 0 ; }
include/linux/gpio/driver.h +9 -0
@@ -201,6 +201,15 @@ struct gpio_irq_chip {*/ bool threaded; +/** +* @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
Signature · 1 line
2.30.1
PATCH 2/2 gpio: Request interrupts after IRQ is initialized
In reply to a message not in the archiveFrom: Mario Limonciello <[email protected]>
Commit 5467801f1fcb ("gpio: Restrict usage of GPIO chip irq members before initialization") attempted to fix a race condition that lead to a NULL pointer, but in the process caused a regression for _AEI/_EVT declared GPIOs.
This manifests in messages showing deferred probing while trying to allocate IRQs like so:
amd_gpio AMDI0030:00: Failed to translate GPIO pin 0x0000 to IRQ, err -517 amd_gpio AMDI0030:00: Failed to translate GPIO pin 0x002C to IRQ, err -517 amd_gpio AMDI0030:00: Failed to translate GPIO pin 0x003D to IRQ, err -517 [ .. more of the same .. ]
The code for walking _AEI doesn't handle deferred probing and so this leads to non-functional GPIO interrupts.
Fix this issue by moving the call to acpi_gpiochip_request_interrupts to occur after gc->irc.initialized is set.
--- drivers/gpio/gpiolib.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
drivers/gpio/gpiolib.c +2 -2
@@ -2329,8 +2329,6 @@ static int gpiochip_add_irqchip(struct gpio_chip *gpiochip,gpiochip_set_irq_hooks (gpiochip); -acpi_gpiochip_request_interrupts (gpiochip); -/* * Using barrier() here to prevent compiler from reordering * gc->irq.initialized before initialization of above
@@ -2340,6 +2338,8 @@ static int gpiochip_add_irqchip(struct gpio_chip *gpiochip, gpiochip->irq.initialized = true; +acpi_gpiochip_request_interrupts (gpiochip); +return 0 ; }
Signature · 1 line
2.30.1