Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
From:   Shreeya Patel <shreeya.patel@collabora.com>
To:     linus.walleij@linaro.org, brgl@bgdev.pl, krisman@collabora.com,
        andy.shevchenko@gmail.com
Cc:     linux-gpio@vger.kernel.org, linux-kernel@vger.kernel.org,
        kernel@collabora.com, Shreeya Patel <shreeya.patel@collabora.com>
Subject: [PATCH v2] gpio: Restrict usage of gc irq members before initialization
Date:   Tue, 15 Mar 2022 16:08:13 +0530
Message-Id: <20220315103813.84407-1-shreeya.patel@collabora.com>
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit
Precedence: bulk

gc 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.

To avoid such scenarios, restrict usage of gc irq members before
they are completely initialized.

Signed-off-by: Shreeya Patel <shreeya.patel@collabora.com>
---

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      | 11 ++++++++++-
 include/linux/gpio/driver.h |  9 +++++++++
 2 files changed, 19 insertions(+), 1 deletion(-)

diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index defb7c464b87..3973146736a1 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -1593,6 +1593,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.gc_irq_initialized before initialization of above
+	 * gc irq members.
+	 */
+	barrier();
+
+	gc->irq.gc_irq_initialized = true;
+
 	return 0;
 }
 
@@ -3138,7 +3147,7 @@ int gpiod_to_irq(const struct gpio_desc *desc)
 
 	gc = desc->gdev->chip;
 	offset = gpio_chip_hwgpio(desc);
-	if (gc->to_irq) {
+	if (gc->to_irq && gc->irq.gc_irq_initialized) {
 		int retirq = gc->to_irq(gc, offset);
 
 		/* Zero means NO_IRQ */
diff --git a/include/linux/gpio/driver.h b/include/linux/gpio/driver.h
index b0728c8ad90c..e93de63feece 100644
--- a/include/linux/gpio/driver.h
+++ b/include/linux/gpio/driver.h
@@ -203,6 +203,15 @@ struct gpio_irq_chip {
 	 */
 	unsigned int *map;
 
+	/**
+	 * @gc_irq_initialized:
+	 *
+	 * Flag to track gc irq member's initialization.
+	 * This flag will make sure gc irq members are not used before
+	 * they are initialized.
+	 */
+	bool gc_irq_initialized;
+
 	/**
 	 * @threaded:
 	 *
-- 
2.30.2