On 13/02/13 18:02, Alexandre Courbot wrote:
> From: Alexandre Courbot <acour...@nvidia.com>
>
> Some functions dereferenced their GPIO descriptor argument without
> checking its validity first, potentially leading to an oops when given
> an invalid argument.
>
> This patch also makes gpio_get_value() more resilient when given an
> invalid GPIO, returning 0 instead of oopsing.
>
> Signed-off-by: Alexandre Courbot <acour...@nvidia.com>
> ---
> drivers/gpio/gpiolib.c | 64
> +++++++++++++++++++++++++++-----------------------
> 1 file changed, 35 insertions(+), 29 deletions(-)
>
> diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
> index fff9786..8a2cf9c 100644
> --- a/drivers/gpio/gpiolib.c
> +++ b/drivers/gpio/gpiolib.c
> @@ -174,7 +174,7 @@ static int gpio_ensure_requested(struct gpio_desc *desc)
> /* caller holds gpio_lock *OR* gpio is marked as requested */
> static struct gpio_chip *gpiod_to_chip(struct gpio_desc *desc)
> {
> - return desc->chip;
> + return desc ? desc->chip : NULL;
> }
>
> struct gpio_chip *gpio_to_chip(unsigned gpio)
> @@ -653,7 +653,12 @@ static ssize_t export_store(struct class *class,
> if (status < 0)
> goto done;
>
> + status = -EINVAL;
> +
> desc = gpio_to_desc(gpio);
> + /* reject invalid GPIOs */
> + if (!desc)
> + goto done;
>
> /* No extra locking here; FLAG_SYSFS just signifies that the
> * request and export were done by on behalf of userspace, so
> @@ -867,8 +872,8 @@ static int gpiod_export_link(struct device *dev, const
> char *name,
>
> done:
> if (status)
> - pr_debug("%s: gpio%d status %d\n", __func__, desc_to_gpio(desc),
> - status);
> + pr_debug("%s: gpio%d status %d\n", __func__,
> + desc ? desc_to_gpio(desc) : -1, status);
Is it really useful to use the same pr_debug for the error case? Why not do:
desc = gpio_to_desc(gpio);
if (!desc) {
pr_debug("%s - Invalid gpio %d\n", __func__, gpio);
return -EINVAL;
}
...
At this point desc is known valid, though you could just use the gpio
number that was passed in (assuming that it is always the same as
desc_to_gpio).
pr_debug("%s: gpio%d status %d\n", __func__,
desc_to_gpio(desc), status);
return status;
That provides more information (the original gpio number and the reason
for the -EINVAL) if the gpio is not valid, and removes the ugly ternary
operator from the pr_debug. Same goes for the other functions.
~Ryan
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
