* [PATCH] input: pxa27x_keypad: fix NULL pointer dereference
@ 2013-09-16 16:44 Mike Dunn
2013-09-16 16:49 ` Marek Vasut
0 siblings, 1 reply; 6+ messages in thread
From: Mike Dunn @ 2013-09-16 16:44 UTC (permalink / raw)
To: linux-input
Cc: Mike Dunn, Dmitry Torokhov, Chao Xie, Robert Jarzmik, Marek Vasut
A NULL pointer dereference exception occurs in the driver probe function when
device tree is used. The pdata pointer will be NULL in this case, but the code
dereferences it in all cases. When device tree is used, a platform data
structure is allocated and initialized, and in all cases this pointer is copied
to the driver's private data, so the variable being tested should be accessed
through the driver's private data structure.
Signed-off-by: Mike Dunn <mikedunn@newsguy.com>
---
drivers/input/keyboard/pxa27x_keypad.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/input/keyboard/pxa27x_keypad.c b/drivers/input/keyboard/pxa27x_keypad.c
index 134c3b4..3b2a614 100644
--- a/drivers/input/keyboard/pxa27x_keypad.c
+++ b/drivers/input/keyboard/pxa27x_keypad.c
@@ -795,8 +795,10 @@ static int pxa27x_keypad_probe(struct platform_device *pdev)
goto failed_put_clk;
}
- if ((pdata->enable_rotary0 && keypad->rotary_rel_code[0] != -1) ||
- (pdata->enable_rotary1 && keypad->rotary_rel_code[1] != -1)) {
+ if ((keypad->pdata->enable_rotary0 &&
+ keypad->rotary_rel_code[0] != -1) ||
+ (keypad->pdata->enable_rotary1 &&
+ keypad->rotary_rel_code[1] != -1)) {
input_dev->evbit[0] |= BIT_MASK(EV_REL);
}
--
1.8.1.5
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] input: pxa27x_keypad: fix NULL pointer dereference
2013-09-16 16:44 [PATCH] input: pxa27x_keypad: fix NULL pointer dereference Mike Dunn
@ 2013-09-16 16:49 ` Marek Vasut
2013-09-16 17:06 ` Dmitry Torokhov
0 siblings, 1 reply; 6+ messages in thread
From: Marek Vasut @ 2013-09-16 16:49 UTC (permalink / raw)
To: Mike Dunn; +Cc: linux-input, Dmitry Torokhov, Chao Xie, Robert Jarzmik
Dear Mike Dunn,
> A NULL pointer dereference exception occurs in the driver probe function
> when device tree is used. The pdata pointer will be NULL in this case,
> but the code dereferences it in all cases. When device tree is used, a
> platform data structure is allocated and initialized, and in all cases
> this pointer is copied to the driver's private data, so the variable being
> tested should be accessed through the driver's private data structure.
>
> Signed-off-by: Mike Dunn <mikedunn@newsguy.com>
> ---
> drivers/input/keyboard/pxa27x_keypad.c | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/input/keyboard/pxa27x_keypad.c
> b/drivers/input/keyboard/pxa27x_keypad.c index 134c3b4..3b2a614 100644
> --- a/drivers/input/keyboard/pxa27x_keypad.c
> +++ b/drivers/input/keyboard/pxa27x_keypad.c
> @@ -795,8 +795,10 @@ static int pxa27x_keypad_probe(struct platform_device
> *pdev) goto failed_put_clk;
> }
>
> - if ((pdata->enable_rotary0 && keypad->rotary_rel_code[0] != -1) ||
> - (pdata->enable_rotary1 && keypad->rotary_rel_code[1] != -1)) {
> + if ((keypad->pdata->enable_rotary0 &&
> + keypad->rotary_rel_code[0] != -1) ||
> + (keypad->pdata->enable_rotary1 &&
> + keypad->rotary_rel_code[1] != -1)) {
> input_dev->evbit[0] |= BIT_MASK(EV_REL);
> }
Nice find. Acked-by: Marek Vasut <marex@denx.de>
Best regards,
Marek Vasut
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] input: pxa27x_keypad: fix NULL pointer dereference
2013-09-16 16:49 ` Marek Vasut
@ 2013-09-16 17:06 ` Dmitry Torokhov
2013-09-18 4:05 ` Mike Dunn
0 siblings, 1 reply; 6+ messages in thread
From: Dmitry Torokhov @ 2013-09-16 17:06 UTC (permalink / raw)
To: Marek Vasut; +Cc: Mike Dunn, linux-input, Chao Xie, Robert Jarzmik
On Mon, Sep 16, 2013 at 06:49:53PM +0200, Marek Vasut wrote:
> Dear Mike Dunn,
>
> > A NULL pointer dereference exception occurs in the driver probe function
> > when device tree is used. The pdata pointer will be NULL in this case,
> > but the code dereferences it in all cases. When device tree is used, a
> > platform data structure is allocated and initialized, and in all cases
> > this pointer is copied to the driver's private data, so the variable being
> > tested should be accessed through the driver's private data structure.
> >
> > Signed-off-by: Mike Dunn <mikedunn@newsguy.com>
> > ---
> > drivers/input/keyboard/pxa27x_keypad.c | 6 ++++--
> > 1 file changed, 4 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/input/keyboard/pxa27x_keypad.c
> > b/drivers/input/keyboard/pxa27x_keypad.c index 134c3b4..3b2a614 100644
> > --- a/drivers/input/keyboard/pxa27x_keypad.c
> > +++ b/drivers/input/keyboard/pxa27x_keypad.c
> > @@ -795,8 +795,10 @@ static int pxa27x_keypad_probe(struct platform_device
> > *pdev) goto failed_put_clk;
> > }
> >
> > - if ((pdata->enable_rotary0 && keypad->rotary_rel_code[0] != -1) ||
> > - (pdata->enable_rotary1 && keypad->rotary_rel_code[1] != -1)) {
> > + if ((keypad->pdata->enable_rotary0 &&
> > + keypad->rotary_rel_code[0] != -1) ||
> > + (keypad->pdata->enable_rotary1 &&
> > + keypad->rotary_rel_code[1] != -1)) {
> > input_dev->evbit[0] |= BIT_MASK(EV_REL);
> > }
>
> Nice find. Acked-by: Marek Vasut <marex@denx.de>
Excellent booby trap. I would prefer if we explicitly did
pdata = keypad->pdata;
after calling the parse DT fucntion with a nice comment, because we
somebody might want to rearrange the code and accidentially revert the
checks to the original state.
Thanks.
--
Dmitry
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] input: pxa27x_keypad: fix NULL pointer dereference
2013-09-16 17:06 ` Dmitry Torokhov
@ 2013-09-18 4:05 ` Mike Dunn
2013-09-18 4:24 ` Dmitry Torokhov
0 siblings, 1 reply; 6+ messages in thread
From: Mike Dunn @ 2013-09-18 4:05 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: Marek Vasut, linux-input, Chao Xie, Robert Jarzmik
On 09/16/2013 10:06 AM, Dmitry Torokhov wrote:
> On Mon, Sep 16, 2013 at 06:49:53PM +0200, Marek Vasut wrote:
>> Dear Mike Dunn,
>>
>>> A NULL pointer dereference exception occurs in the driver probe function
>>> when device tree is used. The pdata pointer will be NULL in this case,
>>> but the code dereferences it in all cases. When device tree is used, a
>>> platform data structure is allocated and initialized, and in all cases
>>> this pointer is copied to the driver's private data, so the variable being
>>> tested should be accessed through the driver's private data structure.
>>>
>>> Signed-off-by: Mike Dunn <mikedunn@newsguy.com>
>>> ---
>>> drivers/input/keyboard/pxa27x_keypad.c | 6 ++++--
>>> 1 file changed, 4 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/input/keyboard/pxa27x_keypad.c
>>> b/drivers/input/keyboard/pxa27x_keypad.c index 134c3b4..3b2a614 100644
>>> --- a/drivers/input/keyboard/pxa27x_keypad.c
>>> +++ b/drivers/input/keyboard/pxa27x_keypad.c
>>> @@ -795,8 +795,10 @@ static int pxa27x_keypad_probe(struct platform_device
>>> *pdev) goto failed_put_clk;
>>> }
>>>
>>> - if ((pdata->enable_rotary0 && keypad->rotary_rel_code[0] != -1) ||
>>> - (pdata->enable_rotary1 && keypad->rotary_rel_code[1] != -1)) {
>>> + if ((keypad->pdata->enable_rotary0 &&
>>> + keypad->rotary_rel_code[0] != -1) ||
>>> + (keypad->pdata->enable_rotary1 &&
>>> + keypad->rotary_rel_code[1] != -1)) {
>>> input_dev->evbit[0] |= BIT_MASK(EV_REL);
>>> }
>>
>> Nice find. Acked-by: Marek Vasut <marex@denx.de>
>
> Excellent booby trap. I would prefer if we explicitly did
>
> pdata = keypad->pdata;
>
> after calling the parse DT fucntion with a nice comment, because we
> somebody might want to rearrange the code and accidentially revert the
> checks to the original state.
Yes, that would have been better. Is someone picking this up? I'm not familir
with the input subsystem maintainer (sorry). If this will be upstreamed in
someone's tree, I'll be glad to resubmit with this change. Or, if you prefer,
please feel free to shepherd this Dmitry.
Sorry for the delay.
Thanks,
Mike
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] input: pxa27x_keypad: fix NULL pointer dereference
2013-09-18 4:05 ` Mike Dunn
@ 2013-09-18 4:24 ` Dmitry Torokhov
2013-09-18 14:28 ` Mike Dunn
0 siblings, 1 reply; 6+ messages in thread
From: Dmitry Torokhov @ 2013-09-18 4:24 UTC (permalink / raw)
To: Mike Dunn; +Cc: Marek Vasut, linux-input, Chao Xie, Robert Jarzmik
On Tue, Sep 17, 2013 at 09:05:54PM -0700, Mike Dunn wrote:
> On 09/16/2013 10:06 AM, Dmitry Torokhov wrote:
> > On Mon, Sep 16, 2013 at 06:49:53PM +0200, Marek Vasut wrote:
> >> Dear Mike Dunn,
> >>
> >>> A NULL pointer dereference exception occurs in the driver probe function
> >>> when device tree is used. The pdata pointer will be NULL in this case,
> >>> but the code dereferences it in all cases. When device tree is used, a
> >>> platform data structure is allocated and initialized, and in all cases
> >>> this pointer is copied to the driver's private data, so the variable being
> >>> tested should be accessed through the driver's private data structure.
> >>>
> >>> Signed-off-by: Mike Dunn <mikedunn@newsguy.com>
> >>> ---
> >>> drivers/input/keyboard/pxa27x_keypad.c | 6 ++++--
> >>> 1 file changed, 4 insertions(+), 2 deletions(-)
> >>>
> >>> diff --git a/drivers/input/keyboard/pxa27x_keypad.c
> >>> b/drivers/input/keyboard/pxa27x_keypad.c index 134c3b4..3b2a614 100644
> >>> --- a/drivers/input/keyboard/pxa27x_keypad.c
> >>> +++ b/drivers/input/keyboard/pxa27x_keypad.c
> >>> @@ -795,8 +795,10 @@ static int pxa27x_keypad_probe(struct platform_device
> >>> *pdev) goto failed_put_clk;
> >>> }
> >>>
> >>> - if ((pdata->enable_rotary0 && keypad->rotary_rel_code[0] != -1) ||
> >>> - (pdata->enable_rotary1 && keypad->rotary_rel_code[1] != -1)) {
> >>> + if ((keypad->pdata->enable_rotary0 &&
> >>> + keypad->rotary_rel_code[0] != -1) ||
> >>> + (keypad->pdata->enable_rotary1 &&
> >>> + keypad->rotary_rel_code[1] != -1)) {
> >>> input_dev->evbit[0] |= BIT_MASK(EV_REL);
> >>> }
> >>
> >> Nice find. Acked-by: Marek Vasut <marex@denx.de>
> >
> > Excellent booby trap. I would prefer if we explicitly did
> >
> > pdata = keypad->pdata;
> >
> > after calling the parse DT fucntion with a nice comment, because we
> > somebody might want to rearrange the code and accidentially revert the
> > checks to the original state.
>
>
> Yes, that would have been better. Is someone picking this up? I'm not familir
> with the input subsystem maintainer (sorry).
That would be yours truly.
> If this will be upstreamed in
> someone's tree, I'll be glad to resubmit with this change.
If you could resubmit that would be great - I do not have the hardware
and I prefer applying patches that were tested, if possible.
Thanks.
--
Dmitry
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] input: pxa27x_keypad: fix NULL pointer dereference
2013-09-18 4:24 ` Dmitry Torokhov
@ 2013-09-18 14:28 ` Mike Dunn
0 siblings, 0 replies; 6+ messages in thread
From: Mike Dunn @ 2013-09-18 14:28 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: Marek Vasut, linux-input, Chao Xie, Robert Jarzmik
On 09/17/2013 09:24 PM, Dmitry Torokhov wrote:
> On Tue, Sep 17, 2013 at 09:05:54PM -0700, Mike Dunn wrote:
>> On 09/16/2013 10:06 AM, Dmitry Torokhov wrote:
>>> On Mon, Sep 16, 2013 at 06:49:53PM +0200, Marek Vasut wrote:
>>>> Dear Mike Dunn,
>>>>
>>>>> A NULL pointer dereference exception occurs in the driver probe function
>>>>> when device tree is used. The pdata pointer will be NULL in this case,
>>>>> but the code dereferences it in all cases. When device tree is used, a
>>>>> platform data structure is allocated and initialized, and in all cases
>>>>> this pointer is copied to the driver's private data, so the variable being
>>>>> tested should be accessed through the driver's private data structure.
>>>>>
>>>>> Signed-off-by: Mike Dunn <mikedunn@newsguy.com>
>>>>> ---
>>>>> drivers/input/keyboard/pxa27x_keypad.c | 6 ++++--
>>>>> 1 file changed, 4 insertions(+), 2 deletions(-)
>>>>>
>>>>> diff --git a/drivers/input/keyboard/pxa27x_keypad.c
>>>>> b/drivers/input/keyboard/pxa27x_keypad.c index 134c3b4..3b2a614 100644
>>>>> --- a/drivers/input/keyboard/pxa27x_keypad.c
>>>>> +++ b/drivers/input/keyboard/pxa27x_keypad.c
>>>>> @@ -795,8 +795,10 @@ static int pxa27x_keypad_probe(struct platform_device
>>>>> *pdev) goto failed_put_clk;
>>>>> }
>>>>>
>>>>> - if ((pdata->enable_rotary0 && keypad->rotary_rel_code[0] != -1) ||
>>>>> - (pdata->enable_rotary1 && keypad->rotary_rel_code[1] != -1)) {
>>>>> + if ((keypad->pdata->enable_rotary0 &&
>>>>> + keypad->rotary_rel_code[0] != -1) ||
>>>>> + (keypad->pdata->enable_rotary1 &&
>>>>> + keypad->rotary_rel_code[1] != -1)) {
>>>>> input_dev->evbit[0] |= BIT_MASK(EV_REL);
>>>>> }
>>>>
>>>> Nice find. Acked-by: Marek Vasut <marex@denx.de>
>>>
>>> Excellent booby trap. I would prefer if we explicitly did
>>>
>>> pdata = keypad->pdata;
>>>
>>> after calling the parse DT fucntion with a nice comment, because we
>>> somebody might want to rearrange the code and accidentially revert the
>>> checks to the original state.
>>
>>
>> Yes, that would have been better. Is someone picking this up? I'm not familir
>> with the input subsystem maintainer (sorry).
>
> That would be yours truly.
>
>> If this will be upstreamed in
>> someone's tree, I'll be glad to resubmit with this change.
>
> If you could resubmit that would be great - I do not have the hardware
> and I prefer applying patches that were tested, if possible.
OK, will do. Yes, I'm testing this on a palm treo 680 with matrix keys and one
direct key.
Thanks,
Mike
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2013-09-18 14:29 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-09-16 16:44 [PATCH] input: pxa27x_keypad: fix NULL pointer dereference Mike Dunn
2013-09-16 16:49 ` Marek Vasut
2013-09-16 17:06 ` Dmitry Torokhov
2013-09-18 4:05 ` Mike Dunn
2013-09-18 4:24 ` Dmitry Torokhov
2013-09-18 14:28 ` Mike Dunn
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).