linux-kernel - [PATCH v2] sc16is7xx: Prevent dereferencing invalid data when gpio

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Message-Id: <20211220113603.1311-1-radim.pavlik@tbs-biometrics.com>
Date:   Mon, 20 Dec 2021 12:36:03 +0100
From:   Radim Pavlik <radim.pavlik@...-biometrics.com>
To:     Andy Shevchenko <andy.shevchenko@...il.com>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Jiri Slaby <jirislaby@...nel.org>,
        linux-serial@...r.kernel.org, linux-kernel@...r.kernel.org
Cc:     Radim Pavlik <radim.pavlik@...-biometrics.com>
Subject: [PATCH v2] sc16is7xx: Prevent dereferencing invalid data when gpio_hog used

gpio_hog is trying to use gpio functions during probe call.
At that time we dont have initialized port data which are dereferenced
via assigned gpio.direction_input/output functions.
This results in unable to handle kernel NULL pointer dereference
followed by Kernel panic.
This patch prevents that.

Due to changed order of initialization 'port' before 'gpio controller'
was also changed error path which resulted in removed label.

No fixes tag because patch spans across multiple patches.

Signed-off-by: Radim Pavlik <radim.pavlik@...-biometrics.com>
---
 drivers/tty/serial/sc16is7xx.c | 49 +++++++++++++++++-----------------
 1 file changed, 24 insertions(+), 25 deletions(-)

diff --git a/drivers/tty/serial/sc16is7xx.c b/drivers/tty/serial/sc16is7xx.c
index 64e7e6c8145f..416fa71dcde1 100644
--- a/drivers/tty/serial/sc16is7xx.c
+++ b/drivers/tty/serial/sc16is7xx.c
@@ -1242,25 +1242,6 @@ static int sc16is7xx_probe(struct device *dev,
 	}
 	sched_set_fifo(s->kworker_task);

-#ifdef CONFIG_GPIOLIB
-	if (devtype->nr_gpio) {
-		/* Setup GPIO cotroller */
-		s->gpio.owner		 = THIS_MODULE;
-		s->gpio.parent		 = dev;
-		s->gpio.label		 = dev_name(dev);
-		s->gpio.direction_input	 = sc16is7xx_gpio_direction_input;
-		s->gpio.get		 = sc16is7xx_gpio_get;
-		s->gpio.direction_output = sc16is7xx_gpio_direction_output;
-		s->gpio.set		 = sc16is7xx_gpio_set;
-		s->gpio.base		 = -1;
-		s->gpio.ngpio		 = devtype->nr_gpio;
-		s->gpio.can_sleep	 = 1;
-		ret = gpiochip_add_data(&s->gpio, s);
-		if (ret)
-			goto out_thread;
-	}
-#endif
-
 	/* reset device, purging any pending irq / data */
 	regmap_write(s->regmap, SC16IS7XX_IOCONTROL_REG << SC16IS7XX_REG_SHIFT,
 			SC16IS7XX_IOCONTROL_SRESET_BIT);
@@ -1315,6 +1296,25 @@ static int sc16is7xx_probe(struct device *dev,
 		sc16is7xx_power(&s->p[i].port, 0);
 	}

+#ifdef CONFIG_GPIOLIB
+	if (devtype->nr_gpio) {
+		/* Setup GPIO cotroller */
+		s->gpio.owner		 = THIS_MODULE;
+		s->gpio.parent		 = dev;
+		s->gpio.label		 = dev_name(dev);
+		s->gpio.direction_input	 = sc16is7xx_gpio_direction_input;
+		s->gpio.get		 = sc16is7xx_gpio_get;
+		s->gpio.direction_output = sc16is7xx_gpio_direction_output;
+		s->gpio.set		 = sc16is7xx_gpio_set;
+		s->gpio.base		 = -1;
+		s->gpio.ngpio		 = devtype->nr_gpio;
+		s->gpio.can_sleep	 = 1;
+		ret = gpiochip_add_data(&s->gpio, s);
+		if (ret)
+			goto out_ports;
+	}
+#endif
+
 	if (dev->of_node) {
 		struct property *prop;
 		const __be32 *p;
@@ -1345,18 +1345,17 @@ static int sc16is7xx_probe(struct device *dev,
 	if (!ret)
 		return 0;

+#ifdef CONFIG_GPIOLIB
+	if (devtype->nr_gpio)
+		gpiochip_remove(&s->gpio);
+#endif
+
 out_ports:
 	for (i--; i >= 0; i--) {
 		uart_remove_one_port(&sc16is7xx_uart, &s->p[i].port);
 		clear_bit(s->p[i].port.line, &sc16is7xx_lines);
 	}

-#ifdef CONFIG_GPIOLIB
-	if (devtype->nr_gpio)
-		gpiochip_remove(&s->gpio);
-
-out_thread:
-#endif
 	kthread_stop(s->kworker_task);

 out_clk:
--
2.32.0