gpio: null pointer dereference in error handling in gpiolib.c

PATCH In review Frank Rowand 2 participants 2 messages Aug 30, 2013 GPIO

lore

Share & tools

lore

Frank Rowand

author

2013-08-30 05:10 UTC #0 lore raw

PATCH 1/1 gpio: null pointer dereference in error handling in gpiolib.c

Avoid calling desc_to_gpio() if desc->chip is NULL, as this will cause a kernel panic.

In the code above the calls, there is a test for !chip, which comes to the 'fail' label if true. In this case, the code panics, since desc_to_gpio() uses desc->chip to look up the gpio number.

An RFC patch that explained the cause of one example of panic when desc->chip is NULL and fixed that example
(http://lkml.indiana.edu/hypermail/linux/kernel/1308.3/01473.html)
was accepted. This patch fixes the remaining locations which have the same problem.

Signed-off-by Frank Rowand <[email protected]>

---
drivers/gpio/gpiolib.c | 33 24 + 9 - 0 !

 1 file changed, 24 insertions(+), 9 deletions(-)

drivers/gpio/gpiolib.c +24 -9

Index: b/drivers/gpio/gpiolib.c
===================================================================
@@ -1676,9 +1676,14 @@ lose:
 	return status;
 fail:
 	spin_unlock_irqrestore(&gpio_lock, flags);
-	if (status)
-		pr_debug("%s: gpio-%d status %d\n", __func__,
-			 desc_to_gpio(desc), status);
+	if (status) {
+		if (desc->chip) {
+			pr_debug("%s: gpio-%d status %d\n", __func__,
+				 desc_to_gpio(desc), status);
+		} else {
+			pr_debug("%s: gpio-?? status %d\n", __func__, status);
+		}
+	}
 	return status;
 }

@@ -1745,9 +1750,14 @@ lose:
 	return status;
 fail:
 	spin_unlock_irqrestore(&gpio_lock, flags);
-	if (status)
-		pr_debug("%s: gpio-%d status %d\n", __func__,
-			 desc_to_gpio(desc), status);
+	if (status) {
+		if (desc->chip) {
+			pr_debug("%s: gpio-%d status %d\n", __func__,
+				 desc_to_gpio(desc), status);
+		} else {
+			pr_debug("%s: gpio-?? status %d\n", __func__, status);
+		}
+	}
 	return status;
 }

@@ -1795,9 +1805,14 @@ static int gpiod_set_debounce(struct gpi
 
 fail:
 	spin_unlock_irqrestore(&gpio_lock, flags);
-	if (status)
-		pr_debug("%s: gpio-%d status %d\n", __func__,
-			 desc_to_gpio(desc), status);
+	if (status) {
+		if (desc->chip) {
+			pr_debug("%s: gpio-%d status %d\n", __func__,
+				 desc_to_gpio(desc), status);
+		} else {
+			pr_debug("%s: gpio-?? status %d\n", __func__, status);
+		}
+	}
 
 	return status;
 }

Linus Walleij

2013-08-30 07:29 UTC #1 lore raw

Quoted 8 lines · Frank Rowand

Avoid calling desc_to_gpio() if desc->chip is NULL, as this will cause a kernel panic.
In the code above the calls, there is a test for !chip, which comes to the 'fail' label if true. In this case, the code panics, since desc_to_gpio() uses desc->chip to look up the gpio number.

Quoted 7 lines

An RFC patch that explained the cause of one example of panic when desc->chip is NULL and fixed that example
(http://lkml.indiana.edu/hypermail/linux/kernel/1308.3/01473.html)
was accepted. This patch fixes the remaining locations which have the same problem.
Signed-off-by Frank Rowand <[email protected]>

Hm it appears Alexandre has another idea on how it should be fixed, but he was in a hurry and didn't provide a proper patch.

Let me cook something up so we can see of we can get this right.

Yours,
Linus Walleij