gpiolib: Fix possible use after free on label

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@gmail.com>
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>

diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index 9ccc096a0df7fa6d93f965f358e0f643ec013c98..2a9d50678aa1c410eb1198f19fb8f028e410da3a 100644 (file)
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -2282,6 +2282,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,
@@ -2292,6 +2298,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;
        }
@@ -2308,6 +2315,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;
                }
@@ -2403,6 +2411,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);
@@ -3358,11 +3367,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 f2f887795d43e9ae2cbe8b15be896547dbebbb43..ed070512b40eed97a99b1b837b5b980584d3a9b3 100644 (file)
--- 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)