lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:	Sat, 24 Aug 2013 12:57:53 -0700
From:	Guenter Roeck <linux@...ck-us.net>
To:	Daniel Santos <daniel.santos@...ox.com>
CC:	danielfsantos@....net, Linus Walleij <linus.walleij@...aro.org>,
	linux-gpio@...r.kernel.org, LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] gpiolib: Fix crash when exporting non-existant gpio

On 08/24/2013 11:48 AM, danielfsantos@....net wrote:
> I got this on an RPi and I can't find anything specific to that.
> Besides, it's clearly wrong to try to access desc->chip when we have
> just tested that it may be NULL at drivers/gpio/gpiolib.c:1409:
>
> 	chip = desc->chip;
> 	if (chip == NULL)
> 		goto done;
>
> ....
>
> done:
> 	if (status)
> 		pr_debug("_gpio_request: gpio-%d (%s) status %d\n",
> 			 desc_to_gpio(desc), label ? : "?", status);
>
> To reproduce, just pick an invalid gpio nubmer and:
>
> echo -n 248 > /sys/class/gpio/export
>
> However, I wasn't able to reproduce it on my laptop, maybe because I
> don't have any real gpio chips there, not sure.  More info on RPi bug
> report: https://github.com/raspberrypi/linux/issues/364
>
> [  222.961384] Unable to handle kernel NULL pointer dereference at
> virtual address 00000044
> [  222.969486] pgd = d97d0000
> [  222.972190] [00000044] *pgd=1aaca831, *pte=00000000, *ppte=00000000
> [  222.978483] Internal error: Oops: 17 [#1] PREEMPT ARM
> ---
>   drivers/gpio/gpiolib.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
> index d6413b2..e547f75f8b 100644
> --- a/drivers/gpio/gpiolib.c
> +++ b/drivers/gpio/gpiolib.c
> @@ -136,7 +136,7 @@ static struct gpio_desc *gpio_to_desc(unsigned gpio)
>    */
>   static int desc_to_gpio(const struct gpio_desc *desc)
>   {
> -	return desc->chip->base + gpio_chip_hwgpio(desc);
> +	return desc->chip ?  desc->chip->base + gpio_chip_hwgpio(desc) : -1;
>   }
>

Looking into calling code, desc_to_gpio() is clearly not supposed to return an error,
and it will result in odd behavior if it returns -1. For example, the resulting debug
message of "gpio--1 (...) status ..." is not very useful.

It would make more sense to fix the calling code. You could for example
validate in affected functions if the gpio pin exists by not only
checking for desc but also for desc->chip. Another option might be
to have gpio_to_desc() return NULL if desc->chip is NULL.

Guenter


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists