* [RFC PATCH] gpiolib: fix oops, if gpio name is NULL
@ 2015-11-11 12:36 Vladimir Zapolskiy
2015-11-17 9:24 ` Markus Pargmann
2015-11-29 21:18 ` Linus Walleij
0 siblings, 2 replies; 7+ messages in thread
From: Vladimir Zapolskiy @ 2015-11-11 12:36 UTC (permalink / raw)
To: Linus Walleij, Markus Pargmann; +Cc: linux-gpio
Commit c0017ed71966 ("gpio: Introduce gpio descriptor 'name'") causes
OOPS on boot on LPC32xx boards:
Unable to handle kernel NULL pointer dereference at virtual address 00000000
CPU: 0 PID: 1 Comm: swapper Not tainted 4.3.0+ #707
Hardware name: LPC32XX SoC (Flattened Device Tree)
task: c381baa0 ti: c381e000 task.ti: c381e000
PC is at strcmp+0x10/0x40
LR is at gpiochip_add+0x3d0/0x4d4
pc : [<>] lr : [<>] psr: a0000093
sp : c381fd60 ip : c381fd70 fp : c381fd6c
[snip]
Backtrace:
[<>] (strcmp) from [<>] (gpiochip_add+0x3d0/0x4d4)
[<>] (gpiochip_add) from [<>] (lpc32xx_gpio_probe+0x44/0x60)
[<>] (lpc32xx_gpio_probe) from [<>] (platform_drv_probe+0x40/0x8c)
[<>] (platform_drv_probe) from [<>] (driver_probe_device+0x110/0x294)
[<>] (driver_probe_device) from [<>] (__driver_attach+0x70/0x94)
[<>] (__driver_attach) from [<>] (bus_for_each_dev+0x74/0x98)
[<>] (bus_for_each_dev) from [<>] (driver_attach+0x20/0x28)
[<>] (driver_attach) from [<>] (bus_add_driver+0xd4/0x1f0)
[<>] (bus_add_driver) from [<>] (driver_register+0xa4/0xe8)
[<>] (driver_register) from [<>] (__platform_driver_register+0x38/0x4c)
[<>] (__platform_driver_register) from [<>] (lpc32xx_gpio_driver_init+0x18/0x20)
[<>] (lpc32xx_gpio_driver_init) from [<>] (do_one_initcall+0x108/0x1c8)
[<>] (do_one_initcall) from [<>] (kernel_init_freeable+0x10c/0x1d4)
[<>] (kernel_init_freeable) from [<>] (kernel_init+0x10/0xec)
[<>] (kernel_init) from [<>] (ret_from_fork+0x14/0x24)
This is caused by the fact that at the moment some GPIO names are set
to NULL, there is a hole in linear representation of one GPI bank, see
drivers/gpio/gpio-lpc32xx.c / gpi_p3_names[] for details.
The same problem most probably affects also gpio-cs5535.c, see
cs5535_gpio_names[].
Signed-off-by: Vladimir Zapolskiy <vz@mleia.com>
---
Linus, Markus,
I understand that LPC32xx GPIO driver is ugly (hopefully this
will be changed in future), but please account this problem.
I don't ask for inclusion of this change, because my own analysis is
not done completely.
Also please review, the same problem probably affects gpio-cs5535.c
as well, see cs5535_gpio_names[].
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 a18f00f..2a91f32 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -233,7 +233,7 @@ static struct gpio_desc *gpio_name_to_desc(const char * const name)
for (i = 0; i != chip->ngpio; ++i) {
struct gpio_desc *gpio = &chip->desc[i];
- if (!gpio->name)
+ if (!gpio->name || !name)
continue;
if (!strcmp(gpio->name, name)) {
--
2.5.0
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [RFC PATCH] gpiolib: fix oops, if gpio name is NULL
2015-11-11 12:36 [RFC PATCH] gpiolib: fix oops, if gpio name is NULL Vladimir Zapolskiy
@ 2015-11-17 9:24 ` Markus Pargmann
2015-11-17 11:20 ` Vladimir Zapolskiy
2015-11-29 21:18 ` Linus Walleij
1 sibling, 1 reply; 7+ messages in thread
From: Markus Pargmann @ 2015-11-17 9:24 UTC (permalink / raw)
To: Vladimir Zapolskiy; +Cc: Linus Walleij, linux-gpio
[-- Attachment #1: Type: text/plain, Size: 3533 bytes --]
Hi,
On Wednesday 11 November 2015 14:36:53 Vladimir Zapolskiy wrote:
> Commit c0017ed71966 ("gpio: Introduce gpio descriptor 'name'") causes
> OOPS on boot on LPC32xx boards:
>
> Unable to handle kernel NULL pointer dereference at virtual address 00000000
> CPU: 0 PID: 1 Comm: swapper Not tainted 4.3.0+ #707
> Hardware name: LPC32XX SoC (Flattened Device Tree)
> task: c381baa0 ti: c381e000 task.ti: c381e000
> PC is at strcmp+0x10/0x40
> LR is at gpiochip_add+0x3d0/0x4d4
> pc : [<>] lr : [<>] psr: a0000093
> sp : c381fd60 ip : c381fd70 fp : c381fd6c
>
> [snip]
>
> Backtrace:
> [<>] (strcmp) from [<>] (gpiochip_add+0x3d0/0x4d4)
> [<>] (gpiochip_add) from [<>] (lpc32xx_gpio_probe+0x44/0x60)
> [<>] (lpc32xx_gpio_probe) from [<>] (platform_drv_probe+0x40/0x8c)
> [<>] (platform_drv_probe) from [<>] (driver_probe_device+0x110/0x294)
> [<>] (driver_probe_device) from [<>] (__driver_attach+0x70/0x94)
> [<>] (__driver_attach) from [<>] (bus_for_each_dev+0x74/0x98)
> [<>] (bus_for_each_dev) from [<>] (driver_attach+0x20/0x28)
> [<>] (driver_attach) from [<>] (bus_add_driver+0xd4/0x1f0)
> [<>] (bus_add_driver) from [<>] (driver_register+0xa4/0xe8)
> [<>] (driver_register) from [<>] (__platform_driver_register+0x38/0x4c)
> [<>] (__platform_driver_register) from [<>] (lpc32xx_gpio_driver_init+0x18/0x20)
> [<>] (lpc32xx_gpio_driver_init) from [<>] (do_one_initcall+0x108/0x1c8)
> [<>] (do_one_initcall) from [<>] (kernel_init_freeable+0x10c/0x1d4)
> [<>] (kernel_init_freeable) from [<>] (kernel_init+0x10/0xec)
> [<>] (kernel_init) from [<>] (ret_from_fork+0x14/0x24)
>
> This is caused by the fact that at the moment some GPIO names are set
> to NULL, there is a hole in linear representation of one GPI bank, see
> drivers/gpio/gpio-lpc32xx.c / gpi_p3_names[] for details.
>
> The same problem most probably affects also gpio-cs5535.c, see
> cs5535_gpio_names[].
>
> Signed-off-by: Vladimir Zapolskiy <vz@mleia.com>
> ---
> Linus, Markus,
>
> I understand that LPC32xx GPIO driver is ugly (hopefully this
> will be changed in future), but please account this problem.
>
> I don't ask for inclusion of this change, because my own analysis is
> not done completely.
>
> Also please review, the same problem probably affects gpio-cs5535.c
> as well, see cs5535_gpio_names[].
Thanks for this hint. But I think these patches for gpio_name_to_desc won't get
mainline any time soon as this depends on the chardev interface and how it will
look like.
Best Regards,
Markus
>
> 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 a18f00f..2a91f32 100644
> --- a/drivers/gpio/gpiolib.c
> +++ b/drivers/gpio/gpiolib.c
> @@ -233,7 +233,7 @@ static struct gpio_desc *gpio_name_to_desc(const char * const name)
> for (i = 0; i != chip->ngpio; ++i) {
> struct gpio_desc *gpio = &chip->desc[i];
>
> - if (!gpio->name)
> + if (!gpio->name || !name)
> continue;
>
> if (!strcmp(gpio->name, name)) {
>
--
Pengutronix e.K. | |
Industrial Linux Solutions | http://www.pengutronix.de/ |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [RFC PATCH] gpiolib: fix oops, if gpio name is NULL
2015-11-17 9:24 ` Markus Pargmann
@ 2015-11-17 11:20 ` Vladimir Zapolskiy
2015-11-17 11:50 ` Markus Pargmann
0 siblings, 1 reply; 7+ messages in thread
From: Vladimir Zapolskiy @ 2015-11-17 11:20 UTC (permalink / raw)
To: Markus Pargmann; +Cc: Linus Walleij, linux-gpio
Hi Markus,
On 17.11.2015 11:24, Markus Pargmann wrote:
> Hi,
>
> On Wednesday 11 November 2015 14:36:53 Vladimir Zapolskiy wrote:
>> Commit c0017ed71966 ("gpio: Introduce gpio descriptor 'name'") causes
>> OOPS on boot on LPC32xx boards:
>>
>> Unable to handle kernel NULL pointer dereference at virtual address 00000000
>> CPU: 0 PID: 1 Comm: swapper Not tainted 4.3.0+ #707
>> Hardware name: LPC32XX SoC (Flattened Device Tree)
>> task: c381baa0 ti: c381e000 task.ti: c381e000
>> PC is at strcmp+0x10/0x40
>> LR is at gpiochip_add+0x3d0/0x4d4
>> pc : [<>] lr : [<>] psr: a0000093
>> sp : c381fd60 ip : c381fd70 fp : c381fd6c
>>
>> [snip]
>>
>> Backtrace:
>> [<>] (strcmp) from [<>] (gpiochip_add+0x3d0/0x4d4)
>> [<>] (gpiochip_add) from [<>] (lpc32xx_gpio_probe+0x44/0x60)
>> [<>] (lpc32xx_gpio_probe) from [<>] (platform_drv_probe+0x40/0x8c)
>> [<>] (platform_drv_probe) from [<>] (driver_probe_device+0x110/0x294)
>> [<>] (driver_probe_device) from [<>] (__driver_attach+0x70/0x94)
>> [<>] (__driver_attach) from [<>] (bus_for_each_dev+0x74/0x98)
>> [<>] (bus_for_each_dev) from [<>] (driver_attach+0x20/0x28)
>> [<>] (driver_attach) from [<>] (bus_add_driver+0xd4/0x1f0)
>> [<>] (bus_add_driver) from [<>] (driver_register+0xa4/0xe8)
>> [<>] (driver_register) from [<>] (__platform_driver_register+0x38/0x4c)
>> [<>] (__platform_driver_register) from [<>] (lpc32xx_gpio_driver_init+0x18/0x20)
>> [<>] (lpc32xx_gpio_driver_init) from [<>] (do_one_initcall+0x108/0x1c8)
>> [<>] (do_one_initcall) from [<>] (kernel_init_freeable+0x10c/0x1d4)
>> [<>] (kernel_init_freeable) from [<>] (kernel_init+0x10/0xec)
>> [<>] (kernel_init) from [<>] (ret_from_fork+0x14/0x24)
>>
>> This is caused by the fact that at the moment some GPIO names are set
>> to NULL, there is a hole in linear representation of one GPI bank, see
>> drivers/gpio/gpio-lpc32xx.c / gpi_p3_names[] for details.
>>
>> The same problem most probably affects also gpio-cs5535.c, see
>> cs5535_gpio_names[].
>>
>> Signed-off-by: Vladimir Zapolskiy <vz@mleia.com>
>> ---
>> Linus, Markus,
>>
>> I understand that LPC32xx GPIO driver is ugly (hopefully this
>> will be changed in future), but please account this problem.
>>
>> I don't ask for inclusion of this change, because my own analysis is
>> not done completely.
>>
>> Also please review, the same problem probably affects gpio-cs5535.c
>> as well, see cs5535_gpio_names[].
>
> Thanks for this hint. But I think these patches for gpio_name_to_desc won't get
> mainline any time soon as this depends on the chardev interface and how it will
> look like.
>
sorry for misunderstanding, what do mean by "these patches"?
I see two options here, one is to fix or revert your c0017ed71966 due to
caused regressions, another one is to update affected GPIO drivers
removing discontinuity in GPIO enumeration inside a bank.
With best wishes,
Vladimir
>>
>> 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 a18f00f..2a91f32 100644
>> --- a/drivers/gpio/gpiolib.c
>> +++ b/drivers/gpio/gpiolib.c
>> @@ -233,7 +233,7 @@ static struct gpio_desc *gpio_name_to_desc(const char * const name)
>> for (i = 0; i != chip->ngpio; ++i) {
>> struct gpio_desc *gpio = &chip->desc[i];
>>
>> - if (!gpio->name)
>> + if (!gpio->name || !name)
>> continue;
>>
>> if (!strcmp(gpio->name, name)) {
>>
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [RFC PATCH] gpiolib: fix oops, if gpio name is NULL
2015-11-17 11:20 ` Vladimir Zapolskiy
@ 2015-11-17 11:50 ` Markus Pargmann
2015-11-29 12:44 ` Vladimir Zapolskiy
0 siblings, 1 reply; 7+ messages in thread
From: Markus Pargmann @ 2015-11-17 11:50 UTC (permalink / raw)
To: Vladimir Zapolskiy; +Cc: Linus Walleij, linux-gpio
[-- Attachment #1: Type: text/plain, Size: 4564 bytes --]
Hi Vladimir,
On Tuesday 17 November 2015 13:20:40 Vladimir Zapolskiy wrote:
> Hi Markus,
>
> On 17.11.2015 11:24, Markus Pargmann wrote:
> > Hi,
> >
> > On Wednesday 11 November 2015 14:36:53 Vladimir Zapolskiy wrote:
> >> Commit c0017ed71966 ("gpio: Introduce gpio descriptor 'name'") causes
> >> OOPS on boot on LPC32xx boards:
> >>
> >> Unable to handle kernel NULL pointer dereference at virtual address 00000000
> >> CPU: 0 PID: 1 Comm: swapper Not tainted 4.3.0+ #707
> >> Hardware name: LPC32XX SoC (Flattened Device Tree)
> >> task: c381baa0 ti: c381e000 task.ti: c381e000
> >> PC is at strcmp+0x10/0x40
> >> LR is at gpiochip_add+0x3d0/0x4d4
> >> pc : [<>] lr : [<>] psr: a0000093
> >> sp : c381fd60 ip : c381fd70 fp : c381fd6c
> >>
> >> [snip]
> >>
> >> Backtrace:
> >> [<>] (strcmp) from [<>] (gpiochip_add+0x3d0/0x4d4)
> >> [<>] (gpiochip_add) from [<>] (lpc32xx_gpio_probe+0x44/0x60)
> >> [<>] (lpc32xx_gpio_probe) from [<>] (platform_drv_probe+0x40/0x8c)
> >> [<>] (platform_drv_probe) from [<>] (driver_probe_device+0x110/0x294)
> >> [<>] (driver_probe_device) from [<>] (__driver_attach+0x70/0x94)
> >> [<>] (__driver_attach) from [<>] (bus_for_each_dev+0x74/0x98)
> >> [<>] (bus_for_each_dev) from [<>] (driver_attach+0x20/0x28)
> >> [<>] (driver_attach) from [<>] (bus_add_driver+0xd4/0x1f0)
> >> [<>] (bus_add_driver) from [<>] (driver_register+0xa4/0xe8)
> >> [<>] (driver_register) from [<>] (__platform_driver_register+0x38/0x4c)
> >> [<>] (__platform_driver_register) from [<>] (lpc32xx_gpio_driver_init+0x18/0x20)
> >> [<>] (lpc32xx_gpio_driver_init) from [<>] (do_one_initcall+0x108/0x1c8)
> >> [<>] (do_one_initcall) from [<>] (kernel_init_freeable+0x10c/0x1d4)
> >> [<>] (kernel_init_freeable) from [<>] (kernel_init+0x10/0xec)
> >> [<>] (kernel_init) from [<>] (ret_from_fork+0x14/0x24)
> >>
> >> This is caused by the fact that at the moment some GPIO names are set
> >> to NULL, there is a hole in linear representation of one GPI bank, see
> >> drivers/gpio/gpio-lpc32xx.c / gpi_p3_names[] for details.
> >>
> >> The same problem most probably affects also gpio-cs5535.c, see
> >> cs5535_gpio_names[].
> >>
> >> Signed-off-by: Vladimir Zapolskiy <vz@mleia.com>
> >> ---
> >> Linus, Markus,
> >>
> >> I understand that LPC32xx GPIO driver is ugly (hopefully this
> >> will be changed in future), but please account this problem.
> >>
> >> I don't ask for inclusion of this change, because my own analysis is
> >> not done completely.
> >>
> >> Also please review, the same problem probably affects gpio-cs5535.c
> >> as well, see cs5535_gpio_names[].
> >
> > Thanks for this hint. But I think these patches for gpio_name_to_desc won't get
> > mainline any time soon as this depends on the chardev interface and how it will
> > look like.
> >
>
> sorry for misunderstanding, what do mean by "these patches"?
>
> I see two options here, one is to fix or revert your c0017ed71966 due to
> caused regressions, another one is to update affected GPIO drivers
> removing discontinuity in GPIO enumeration inside a bank.
Sorry, seems I was on a wrong kernel version when checking this morning. Linus
removed lots of my patches from his tree after some correct comments so I
thought this patch was removed as well.
As it is mainline your patch looks good to me. Perhaps a comment on why we need
to check for !name at this point would be good.
Best Regards,
Markus
>
> With best wishes,
> Vladimir
>
> >>
> >> 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 a18f00f..2a91f32 100644
> >> --- a/drivers/gpio/gpiolib.c
> >> +++ b/drivers/gpio/gpiolib.c
> >> @@ -233,7 +233,7 @@ static struct gpio_desc *gpio_name_to_desc(const char * const name)
> >> for (i = 0; i != chip->ngpio; ++i) {
> >> struct gpio_desc *gpio = &chip->desc[i];
> >>
> >> - if (!gpio->name)
> >> + if (!gpio->name || !name)
> >> continue;
> >>
> >> if (!strcmp(gpio->name, name)) {
> >>
> >
>
--
Pengutronix e.K. | |
Industrial Linux Solutions | http://www.pengutronix.de/ |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [RFC PATCH] gpiolib: fix oops, if gpio name is NULL
2015-11-17 11:50 ` Markus Pargmann
@ 2015-11-29 12:44 ` Vladimir Zapolskiy
2015-11-30 9:51 ` Markus Pargmann
0 siblings, 1 reply; 7+ messages in thread
From: Vladimir Zapolskiy @ 2015-11-29 12:44 UTC (permalink / raw)
To: Markus Pargmann; +Cc: Linus Walleij, linux-gpio
Hi Linus,
On 17.11.2015 13:50, Markus Pargmann wrote:
> Hi Vladimir,
>
> On Tuesday 17 November 2015 13:20:40 Vladimir Zapolskiy wrote:
>> Hi Markus,
>>
>> On 17.11.2015 11:24, Markus Pargmann wrote:
>>> Hi,
>>>
>>> On Wednesday 11 November 2015 14:36:53 Vladimir Zapolskiy wrote:
>>>> Commit c0017ed71966 ("gpio: Introduce gpio descriptor 'name'") causes
>>>> OOPS on boot on LPC32xx boards:
>>>>
>>>> Unable to handle kernel NULL pointer dereference at virtual address 00000000
>>>> CPU: 0 PID: 1 Comm: swapper Not tainted 4.3.0+ #707
>>>> Hardware name: LPC32XX SoC (Flattened Device Tree)
>>>> task: c381baa0 ti: c381e000 task.ti: c381e000
>>>> PC is at strcmp+0x10/0x40
>>>> LR is at gpiochip_add+0x3d0/0x4d4
>>>> pc : [<>] lr : [<>] psr: a0000093
>>>> sp : c381fd60 ip : c381fd70 fp : c381fd6c
>>>>
>>>> [snip]
>>>>
>>>> Backtrace:
>>>> [<>] (strcmp) from [<>] (gpiochip_add+0x3d0/0x4d4)
>>>> [<>] (gpiochip_add) from [<>] (lpc32xx_gpio_probe+0x44/0x60)
>>>> [<>] (lpc32xx_gpio_probe) from [<>] (platform_drv_probe+0x40/0x8c)
>>>> [<>] (platform_drv_probe) from [<>] (driver_probe_device+0x110/0x294)
>>>> [<>] (driver_probe_device) from [<>] (__driver_attach+0x70/0x94)
>>>> [<>] (__driver_attach) from [<>] (bus_for_each_dev+0x74/0x98)
>>>> [<>] (bus_for_each_dev) from [<>] (driver_attach+0x20/0x28)
>>>> [<>] (driver_attach) from [<>] (bus_add_driver+0xd4/0x1f0)
>>>> [<>] (bus_add_driver) from [<>] (driver_register+0xa4/0xe8)
>>>> [<>] (driver_register) from [<>] (__platform_driver_register+0x38/0x4c)
>>>> [<>] (__platform_driver_register) from [<>] (lpc32xx_gpio_driver_init+0x18/0x20)
>>>> [<>] (lpc32xx_gpio_driver_init) from [<>] (do_one_initcall+0x108/0x1c8)
>>>> [<>] (do_one_initcall) from [<>] (kernel_init_freeable+0x10c/0x1d4)
>>>> [<>] (kernel_init_freeable) from [<>] (kernel_init+0x10/0xec)
>>>> [<>] (kernel_init) from [<>] (ret_from_fork+0x14/0x24)
>>>>
>>>> This is caused by the fact that at the moment some GPIO names are set
>>>> to NULL, there is a hole in linear representation of one GPI bank, see
>>>> drivers/gpio/gpio-lpc32xx.c / gpi_p3_names[] for details.
>>>>
>>>> The same problem most probably affects also gpio-cs5535.c, see
>>>> cs5535_gpio_names[].
>>>>
>>>> Signed-off-by: Vladimir Zapolskiy <vz@mleia.com>
>>>> ---
>>>> Linus, Markus,
>>>>
>>>> I understand that LPC32xx GPIO driver is ugly (hopefully this
>>>> will be changed in future), but please account this problem.
>>>>
>>>> I don't ask for inclusion of this change, because my own analysis is
>>>> not done completely.
>>>>
>>>> Also please review, the same problem probably affects gpio-cs5535.c
>>>> as well, see cs5535_gpio_names[].
>>>
>>> Thanks for this hint. But I think these patches for gpio_name_to_desc won't get
>>> mainline any time soon as this depends on the chardev interface and how it will
>>> look like.
>>>
>>
>> sorry for misunderstanding, what do mean by "these patches"?
>>
>> I see two options here, one is to fix or revert your c0017ed71966 due to
>> caused regressions, another one is to update affected GPIO drivers
>> removing discontinuity in GPIO enumeration inside a bank.
>
> Sorry, seems I was on a wrong kernel version when checking this morning. Linus
> removed lots of my patches from his tree after some correct comments so I
> thought this patch was removed as well.
Linus, should I resend this fix for v4.4 or is it good enough for
application?
> As it is mainline your patch looks good to me. Perhaps a comment on why we need
> to check for !name at this point would be good.
There is such a description in the commit message, why this check is needed.
With best wishes,
Vladimir
> Best Regards,
>
> Markus
>
>>
>> With best wishes,
>> Vladimir
>>
>>>>
>>>> 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 a18f00f..2a91f32 100644
>>>> --- a/drivers/gpio/gpiolib.c
>>>> +++ b/drivers/gpio/gpiolib.c
>>>> @@ -233,7 +233,7 @@ static struct gpio_desc *gpio_name_to_desc(const char * const name)
>>>> for (i = 0; i != chip->ngpio; ++i) {
>>>> struct gpio_desc *gpio = &chip->desc[i];
>>>>
>>>> - if (!gpio->name)
>>>> + if (!gpio->name || !name)
>>>> continue;
>>>>
>>>> if (!strcmp(gpio->name, name)) {
>>>>
>>>
>>
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [RFC PATCH] gpiolib: fix oops, if gpio name is NULL
2015-11-11 12:36 [RFC PATCH] gpiolib: fix oops, if gpio name is NULL Vladimir Zapolskiy
2015-11-17 9:24 ` Markus Pargmann
@ 2015-11-29 21:18 ` Linus Walleij
1 sibling, 0 replies; 7+ messages in thread
From: Linus Walleij @ 2015-11-29 21:18 UTC (permalink / raw)
To: Vladimir Zapolskiy; +Cc: Markus Pargmann, linux-gpio
On Wed, Nov 11, 2015 at 1:36 PM, Vladimir Zapolskiy <vz@mleia.com> wrote:
> Commit c0017ed71966 ("gpio: Introduce gpio descriptor 'name'") causes
> OOPS on boot on LPC32xx boards:
>
> Unable to handle kernel NULL pointer dereference at virtual address 00000000
> CPU: 0 PID: 1 Comm: swapper Not tainted 4.3.0+ #707
> Hardware name: LPC32XX SoC (Flattened Device Tree)
> task: c381baa0 ti: c381e000 task.ti: c381e000
> PC is at strcmp+0x10/0x40
> LR is at gpiochip_add+0x3d0/0x4d4
> pc : [<>] lr : [<>] psr: a0000093
> sp : c381fd60 ip : c381fd70 fp : c381fd6c
>
> [snip]
>
> Backtrace:
> [<>] (strcmp) from [<>] (gpiochip_add+0x3d0/0x4d4)
> [<>] (gpiochip_add) from [<>] (lpc32xx_gpio_probe+0x44/0x60)
> [<>] (lpc32xx_gpio_probe) from [<>] (platform_drv_probe+0x40/0x8c)
> [<>] (platform_drv_probe) from [<>] (driver_probe_device+0x110/0x294)
> [<>] (driver_probe_device) from [<>] (__driver_attach+0x70/0x94)
> [<>] (__driver_attach) from [<>] (bus_for_each_dev+0x74/0x98)
> [<>] (bus_for_each_dev) from [<>] (driver_attach+0x20/0x28)
> [<>] (driver_attach) from [<>] (bus_add_driver+0xd4/0x1f0)
> [<>] (bus_add_driver) from [<>] (driver_register+0xa4/0xe8)
> [<>] (driver_register) from [<>] (__platform_driver_register+0x38/0x4c)
> [<>] (__platform_driver_register) from [<>] (lpc32xx_gpio_driver_init+0x18/0x20)
> [<>] (lpc32xx_gpio_driver_init) from [<>] (do_one_initcall+0x108/0x1c8)
> [<>] (do_one_initcall) from [<>] (kernel_init_freeable+0x10c/0x1d4)
> [<>] (kernel_init_freeable) from [<>] (kernel_init+0x10/0xec)
> [<>] (kernel_init) from [<>] (ret_from_fork+0x14/0x24)
>
> This is caused by the fact that at the moment some GPIO names are set
> to NULL, there is a hole in linear representation of one GPI bank, see
> drivers/gpio/gpio-lpc32xx.c / gpi_p3_names[] for details.
>
> The same problem most probably affects also gpio-cs5535.c, see
> cs5535_gpio_names[].
>
> Signed-off-by: Vladimir Zapolskiy <vz@mleia.com>
Patch applied for fixes. Sorry for the delay.
Yours,
Linus Walleij
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [RFC PATCH] gpiolib: fix oops, if gpio name is NULL
2015-11-29 12:44 ` Vladimir Zapolskiy
@ 2015-11-30 9:51 ` Markus Pargmann
0 siblings, 0 replies; 7+ messages in thread
From: Markus Pargmann @ 2015-11-30 9:51 UTC (permalink / raw)
To: Vladimir Zapolskiy; +Cc: Linus Walleij, linux-gpio
[-- Attachment #1: Type: text/plain, Size: 5246 bytes --]
Hi,
On Sunday 29 November 2015 14:44:30 Vladimir Zapolskiy wrote:
> Hi Linus,
>
> On 17.11.2015 13:50, Markus Pargmann wrote:
> > Hi Vladimir,
> >
> > On Tuesday 17 November 2015 13:20:40 Vladimir Zapolskiy wrote:
> >> Hi Markus,
> >>
> >> On 17.11.2015 11:24, Markus Pargmann wrote:
> >>> Hi,
> >>>
> >>> On Wednesday 11 November 2015 14:36:53 Vladimir Zapolskiy wrote:
> >>>> Commit c0017ed71966 ("gpio: Introduce gpio descriptor 'name'") causes
> >>>> OOPS on boot on LPC32xx boards:
> >>>>
> >>>> Unable to handle kernel NULL pointer dereference at virtual address 00000000
> >>>> CPU: 0 PID: 1 Comm: swapper Not tainted 4.3.0+ #707
> >>>> Hardware name: LPC32XX SoC (Flattened Device Tree)
> >>>> task: c381baa0 ti: c381e000 task.ti: c381e000
> >>>> PC is at strcmp+0x10/0x40
> >>>> LR is at gpiochip_add+0x3d0/0x4d4
> >>>> pc : [<>] lr : [<>] psr: a0000093
> >>>> sp : c381fd60 ip : c381fd70 fp : c381fd6c
> >>>>
> >>>> [snip]
> >>>>
> >>>> Backtrace:
> >>>> [<>] (strcmp) from [<>] (gpiochip_add+0x3d0/0x4d4)
> >>>> [<>] (gpiochip_add) from [<>] (lpc32xx_gpio_probe+0x44/0x60)
> >>>> [<>] (lpc32xx_gpio_probe) from [<>] (platform_drv_probe+0x40/0x8c)
> >>>> [<>] (platform_drv_probe) from [<>] (driver_probe_device+0x110/0x294)
> >>>> [<>] (driver_probe_device) from [<>] (__driver_attach+0x70/0x94)
> >>>> [<>] (__driver_attach) from [<>] (bus_for_each_dev+0x74/0x98)
> >>>> [<>] (bus_for_each_dev) from [<>] (driver_attach+0x20/0x28)
> >>>> [<>] (driver_attach) from [<>] (bus_add_driver+0xd4/0x1f0)
> >>>> [<>] (bus_add_driver) from [<>] (driver_register+0xa4/0xe8)
> >>>> [<>] (driver_register) from [<>] (__platform_driver_register+0x38/0x4c)
> >>>> [<>] (__platform_driver_register) from [<>] (lpc32xx_gpio_driver_init+0x18/0x20)
> >>>> [<>] (lpc32xx_gpio_driver_init) from [<>] (do_one_initcall+0x108/0x1c8)
> >>>> [<>] (do_one_initcall) from [<>] (kernel_init_freeable+0x10c/0x1d4)
> >>>> [<>] (kernel_init_freeable) from [<>] (kernel_init+0x10/0xec)
> >>>> [<>] (kernel_init) from [<>] (ret_from_fork+0x14/0x24)
> >>>>
> >>>> This is caused by the fact that at the moment some GPIO names are set
> >>>> to NULL, there is a hole in linear representation of one GPI bank, see
> >>>> drivers/gpio/gpio-lpc32xx.c / gpi_p3_names[] for details.
> >>>>
> >>>> The same problem most probably affects also gpio-cs5535.c, see
> >>>> cs5535_gpio_names[].
> >>>>
> >>>> Signed-off-by: Vladimir Zapolskiy <vz@mleia.com>
> >>>> ---
> >>>> Linus, Markus,
> >>>>
> >>>> I understand that LPC32xx GPIO driver is ugly (hopefully this
> >>>> will be changed in future), but please account this problem.
> >>>>
> >>>> I don't ask for inclusion of this change, because my own analysis is
> >>>> not done completely.
> >>>>
> >>>> Also please review, the same problem probably affects gpio-cs5535.c
> >>>> as well, see cs5535_gpio_names[].
> >>>
> >>> Thanks for this hint. But I think these patches for gpio_name_to_desc won't get
> >>> mainline any time soon as this depends on the chardev interface and how it will
> >>> look like.
> >>>
> >>
> >> sorry for misunderstanding, what do mean by "these patches"?
> >>
> >> I see two options here, one is to fix or revert your c0017ed71966 due to
> >> caused regressions, another one is to update affected GPIO drivers
> >> removing discontinuity in GPIO enumeration inside a bank.
> >
> > Sorry, seems I was on a wrong kernel version when checking this morning. Linus
> > removed lots of my patches from his tree after some correct comments so I
> > thought this patch was removed as well.
>
> Linus, should I resend this fix for v4.4 or is it good enough for
> application?
>
> > As it is mainline your patch looks good to me. Perhaps a comment on why we need
> > to check for !name at this point would be good.
>
> There is such a description in the commit message, why this check is needed.
I meant a code comment. But that's not too important.
Best Regards,
Markus
>
> With best wishes,
> Vladimir
>
> > Best Regards,
> >
> > Markus
> >
> >>
> >> With best wishes,
> >> Vladimir
> >>
> >>>>
> >>>> 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 a18f00f..2a91f32 100644
> >>>> --- a/drivers/gpio/gpiolib.c
> >>>> +++ b/drivers/gpio/gpiolib.c
> >>>> @@ -233,7 +233,7 @@ static struct gpio_desc *gpio_name_to_desc(const char * const name)
> >>>> for (i = 0; i != chip->ngpio; ++i) {
> >>>> struct gpio_desc *gpio = &chip->desc[i];
> >>>>
> >>>> - if (!gpio->name)
> >>>> + if (!gpio->name || !name)
> >>>> continue;
> >>>>
> >>>> if (!strcmp(gpio->name, name)) {
> >>>>
> >>>
> >>
> >
>
--
Pengutronix e.K. | |
Industrial Linux Solutions | http://www.pengutronix.de/ |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2015-11-30 9:51 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-11-11 12:36 [RFC PATCH] gpiolib: fix oops, if gpio name is NULL Vladimir Zapolskiy
2015-11-17 9:24 ` Markus Pargmann
2015-11-17 11:20 ` Vladimir Zapolskiy
2015-11-17 11:50 ` Markus Pargmann
2015-11-29 12:44 ` Vladimir Zapolskiy
2015-11-30 9:51 ` Markus Pargmann
2015-11-29 21:18 ` Linus Walleij
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.