Commit 18534df4 authored by Muchun Song's avatar Muchun Song Committed by Linus Walleij

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: default avatarMuchun Song <smuchun@gmail.com>
Signed-off-by: default avatarLinus Walleij <linus.walleij@linaro.org>
parent 02cb87f7
...@@ -2282,6 +2282,12 @@ static int gpiod_request_commit(struct gpio_desc *desc, const char *label) ...@@ -2282,6 +2282,12 @@ static int gpiod_request_commit(struct gpio_desc *desc, const char *label)
unsigned long flags; unsigned long flags;
unsigned offset; unsigned offset;
if (label) {
label = kstrdup_const(label, GFP_KERNEL);
if (!label)
return -ENOMEM;
}
spin_lock_irqsave(&gpio_lock, flags); spin_lock_irqsave(&gpio_lock, flags);
/* NOTE: gpio_request() can be called in early boot, /* 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) ...@@ -2292,6 +2298,7 @@ static int gpiod_request_commit(struct gpio_desc *desc, const char *label)
desc_set_label(desc, label ? : "?"); desc_set_label(desc, label ? : "?");
status = 0; status = 0;
} else { } else {
kfree_const(label);
status = -EBUSY; status = -EBUSY;
goto done; goto done;
} }
...@@ -2308,6 +2315,7 @@ static int gpiod_request_commit(struct gpio_desc *desc, const char *label) ...@@ -2308,6 +2315,7 @@ static int gpiod_request_commit(struct gpio_desc *desc, const char *label)
if (status < 0) { if (status < 0) {
desc_set_label(desc, NULL); desc_set_label(desc, NULL);
kfree_const(label);
clear_bit(FLAG_REQUESTED, &desc->flags); clear_bit(FLAG_REQUESTED, &desc->flags);
goto done; goto done;
} }
...@@ -2403,6 +2411,7 @@ static bool gpiod_free_commit(struct gpio_desc *desc) ...@@ -2403,6 +2411,7 @@ static bool gpiod_free_commit(struct gpio_desc *desc)
chip->free(chip, gpio_chip_hwgpio(desc)); chip->free(chip, gpio_chip_hwgpio(desc));
spin_lock_irqsave(&gpio_lock, flags); spin_lock_irqsave(&gpio_lock, flags);
} }
kfree_const(desc->label);
desc_set_label(desc, NULL); desc_set_label(desc, NULL);
clear_bit(FLAG_ACTIVE_LOW, &desc->flags); clear_bit(FLAG_ACTIVE_LOW, &desc->flags);
clear_bit(FLAG_REQUESTED, &desc->flags); clear_bit(FLAG_REQUESTED, &desc->flags);
...@@ -3358,11 +3367,19 @@ EXPORT_SYMBOL_GPL(gpiod_cansleep); ...@@ -3358,11 +3367,19 @@ EXPORT_SYMBOL_GPL(gpiod_cansleep);
* @desc: gpio to set the consumer name on * @desc: gpio to set the consumer name on
* @name: the new consumer name * @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); VALIDATE_DESC(desc);
/* Just overwrite whatever the previous name was */ if (name) {
desc->label = 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); EXPORT_SYMBOL_GPL(gpiod_set_consumer_name);
......
...@@ -162,7 +162,7 @@ int gpiod_is_active_low(const struct gpio_desc *desc); ...@@ -162,7 +162,7 @@ int gpiod_is_active_low(const struct gpio_desc *desc);
int gpiod_cansleep(const struct gpio_desc *desc); int gpiod_cansleep(const struct gpio_desc *desc);
int gpiod_to_irq(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 */ /* Convert between the old gpio_ and new gpiod_ interfaces */
struct gpio_desc *gpio_to_desc(unsigned gpio); struct gpio_desc *gpio_to_desc(unsigned gpio);
...@@ -495,10 +495,12 @@ static inline int gpiod_to_irq(const struct gpio_desc *desc) ...@@ -495,10 +495,12 @@ static inline int gpiod_to_irq(const struct gpio_desc *desc)
return -EINVAL; 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 */ /* GPIO can never have been requested */
WARN_ON(1); WARN_ON(1);
return -EINVAL;
} }
static inline struct gpio_desc *gpio_to_desc(unsigned gpio) static inline struct gpio_desc *gpio_to_desc(unsigned gpio)
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment