[PATCH AUTOSEL 4.20 008/304] gpiolib: Fix possible use after free on label

From: Sasha Levin
Date: Mon Jan 28 2019 - 12:52:34 EST


From: Muchun Song <smuchun@xxxxxxxxx>

[ Upstream commit 18534df419041e6c1f4b41af56ee7d41f757815c ]

gpiod_request_commit() copies the pointer to the label passed as
an argument only to be used later. But there's a chance the caller
could immediately free the passed string(e.g., local variable).
This could trigger a use after free when we use gpio label(e.g.,
gpiochip_unlock_as_irq(), gpiochip_is_requested()).

To be on the safe side: duplicate the string with kstrdup_const()
so that if an unaware user passes an address to a stack-allocated
buffer, we won't get the arbitrary label.

Also fix gpiod_set_consumer_name().

Signed-off-by: Muchun Song <smuchun@xxxxxxxxx>
Signed-off-by: Linus Walleij <linus.walleij@xxxxxxxxxx>
Signed-off-by: Sasha Levin <sashal@xxxxxxxxxx>
---
drivers/gpio/gpiolib.c | 25 +++++++++++++++++++++----
include/linux/gpio/consumer.h | 6 ++++--
2 files changed, 25 insertions(+), 6 deletions(-)

diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index a2cbb474901c..33266460e81c 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -2299,6 +2299,12 @@ static int gpiod_request_commit(struct gpio_desc *desc, const char *label)
unsigned long flags;
unsigned offset;

+ if (label) {
+ label = kstrdup_const(label, GFP_KERNEL);
+ if (!label)
+ return -ENOMEM;
+ }
+
spin_lock_irqsave(&gpio_lock, flags);

/* NOTE: gpio_request() can be called in early boot,
@@ -2309,6 +2315,7 @@ static int gpiod_request_commit(struct gpio_desc *desc, const char *label)
desc_set_label(desc, label ? : "?");
status = 0;
} else {
+ kfree_const(label);
status = -EBUSY;
goto done;
}
@@ -2325,6 +2332,7 @@ static int gpiod_request_commit(struct gpio_desc *desc, const char *label)

if (status < 0) {
desc_set_label(desc, NULL);
+ kfree_const(label);
clear_bit(FLAG_REQUESTED, &desc->flags);
goto done;
}
@@ -2420,6 +2428,7 @@ static bool gpiod_free_commit(struct gpio_desc *desc)
chip->free(chip, gpio_chip_hwgpio(desc));
spin_lock_irqsave(&gpio_lock, flags);
}
+ kfree_const(desc->label);
desc_set_label(desc, NULL);
clear_bit(FLAG_ACTIVE_LOW, &desc->flags);
clear_bit(FLAG_REQUESTED, &desc->flags);
@@ -3375,11 +3384,19 @@ EXPORT_SYMBOL_GPL(gpiod_cansleep);
* @desc: gpio to set the consumer name on
* @name: the new consumer name
*/
-void gpiod_set_consumer_name(struct gpio_desc *desc, const char *name)
+int gpiod_set_consumer_name(struct gpio_desc *desc, const char *name)
{
- VALIDATE_DESC_VOID(desc);
- /* Just overwrite whatever the previous name was */
- desc->label = name;
+ VALIDATE_DESC(desc);
+ if (name) {
+ name = kstrdup_const(name, GFP_KERNEL);
+ if (!name)
+ return -ENOMEM;
+ }
+
+ kfree_const(desc->label);
+ desc_set_label(desc, name);
+
+ return 0;
}
EXPORT_SYMBOL_GPL(gpiod_set_consumer_name);

diff --git a/include/linux/gpio/consumer.h b/include/linux/gpio/consumer.h
index f2f887795d43..ed070512b40e 100644
--- a/include/linux/gpio/consumer.h
+++ b/include/linux/gpio/consumer.h
@@ -162,7 +162,7 @@ int gpiod_is_active_low(const struct gpio_desc *desc);
int gpiod_cansleep(const struct gpio_desc *desc);

int gpiod_to_irq(const struct gpio_desc *desc);
-void gpiod_set_consumer_name(struct gpio_desc *desc, const char *name);
+int gpiod_set_consumer_name(struct gpio_desc *desc, const char *name);

/* Convert between the old gpio_ and new gpiod_ interfaces */
struct gpio_desc *gpio_to_desc(unsigned gpio);
@@ -495,10 +495,12 @@ static inline int gpiod_to_irq(const struct gpio_desc *desc)
return -EINVAL;
}

-static inline void gpiod_set_consumer_name(struct gpio_desc *desc, const char *name)
+static inline int gpiod_set_consumer_name(struct gpio_desc *desc,
+ const char *name)
{
/* GPIO can never have been requested */
WARN_ON(1);
+ return -EINVAL;
}

static inline struct gpio_desc *gpio_to_desc(unsigned gpio)
--
2.19.1