* [-next] [bug report] media/IR/imon: array overflow
@ 2010-05-04 12:20 Dan Carpenter
2010-05-04 14:03 ` [PATCH] IR/imon: remove dead IMON_KEY_RELEASE_OFFSET Jarod Wilson
0 siblings, 1 reply; 6+ messages in thread
From: Dan Carpenter @ 2010-05-04 12:20 UTC (permalink / raw)
To: jarod; +Cc: linux-media
This is a Smatch thing. I'm not sure what's intended.
drivers/media/IR/imon.c +1211 imon_panel_key_lookup(10) error: buffer overflow 'imon_panel_key_table' 21 <= 21
1207 for (i = 0; i < ARRAY_SIZE(imon_panel_key_table); i++)
1208 if (imon_panel_key_table[i].hw_code == (code | 0xffee))
1209 break;
After the for loop i can be 0 to 21.
1210
1211 keycode = imon_panel_key_table[i % IMON_KEY_RELEASE_OFFSET].keycode;
IMON_KEY_RELEASE_OFFSET is 1000 so it doesn't affect anything.
regards,
dan carpenter
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH] IR/imon: remove dead IMON_KEY_RELEASE_OFFSET
2010-05-04 12:20 [-next] [bug report] media/IR/imon: array overflow Dan Carpenter
@ 2010-05-04 14:03 ` Jarod Wilson
2010-05-04 16:06 ` Dan Carpenter
0 siblings, 1 reply; 6+ messages in thread
From: Jarod Wilson @ 2010-05-04 14:03 UTC (permalink / raw)
To: Dan Carpenter; +Cc: linux-media
This hack was used when the imon driver was using internal key lookup
routines, but became dead weight when the driver was converted to use
ir-core's key lookup routines. These bits simply didn't get removed,
drop 'em now.
Pointed out by Dan Carpenter.
Signed-off-by: Jarod Wilson <jarod@redhat.com>
---
drivers/media/IR/imon.c | 3 +--
1 files changed, 1 insertions(+), 2 deletions(-)
diff --git a/drivers/media/IR/imon.c b/drivers/media/IR/imon.c
index 27743eb..bce8ef8 100644
--- a/drivers/media/IR/imon.c
+++ b/drivers/media/IR/imon.c
@@ -55,7 +55,6 @@
#define BIT_DURATION 250 /* each bit received is 250us */
#define IMON_CLOCK_ENABLE_PACKETS 2
-#define IMON_KEY_RELEASE_OFFSET 1000
/*** P R O T O T Y P E S ***/
@@ -1205,7 +1204,7 @@ static u32 imon_panel_key_lookup(u64 hw_code)
if (imon_panel_key_table[i].hw_code == (code | 0xffee))
break;
- keycode = imon_panel_key_table[i % IMON_KEY_RELEASE_OFFSET].keycode;
+ keycode = imon_panel_key_table[i].keycode;
return keycode;
}
--
Jarod Wilson
jarod@redhat.com
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] IR/imon: remove dead IMON_KEY_RELEASE_OFFSET
2010-05-04 14:03 ` [PATCH] IR/imon: remove dead IMON_KEY_RELEASE_OFFSET Jarod Wilson
@ 2010-05-04 16:06 ` Dan Carpenter
2010-05-04 19:17 ` [PATCH v2] " Jarod Wilson
0 siblings, 1 reply; 6+ messages in thread
From: Dan Carpenter @ 2010-05-04 16:06 UTC (permalink / raw)
To: Jarod Wilson; +Cc: linux-media
On Tue, May 04, 2010 at 10:03:18AM -0400, Jarod Wilson wrote:
> @@ -1205,7 +1204,7 @@ static u32 imon_panel_key_lookup(u64 hw_code)
> if (imon_panel_key_table[i].hw_code == (code | 0xffee))
> break;
>
> - keycode = imon_panel_key_table[i % IMON_KEY_RELEASE_OFFSET].keycode;
> + keycode = imon_panel_key_table[i].keycode;
>
> return keycode;
> }
There is still potentially a problem here because if we don't hit the
break statement, then we're one past the end of the array.
regards,
dan carpenter
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v2] IR/imon: remove dead IMON_KEY_RELEASE_OFFSET
2010-05-04 16:06 ` Dan Carpenter
@ 2010-05-04 19:17 ` Jarod Wilson
2010-05-07 11:52 ` Mauro Carvalho Chehab
0 siblings, 1 reply; 6+ messages in thread
From: Jarod Wilson @ 2010-05-04 19:17 UTC (permalink / raw)
To: Dan Carpenter; +Cc: linux-media
On Tue, May 04, 2010 at 06:06:41PM +0200, Dan Carpenter wrote:
> On Tue, May 04, 2010 at 10:03:18AM -0400, Jarod Wilson wrote:
> > @@ -1205,7 +1204,7 @@ static u32 imon_panel_key_lookup(u64 hw_code)
> > if (imon_panel_key_table[i].hw_code == (code | 0xffee))
> > break;
> >
> > - keycode = imon_panel_key_table[i % IMON_KEY_RELEASE_OFFSET].keycode;
> > + keycode = imon_panel_key_table[i].keycode;
> >
> > return keycode;
> > }
>
> There is still potentially a problem here because if we don't hit the
> break statement, then we're one past the end of the array.
D'oh. Okay, here's v2, should fix that buglet too.
This hack was used when the imon driver was using internal key lookup
routines, but became dead weight when the driver was converted to use
ir-core's key lookup routines. These bits simply didn't get removed,
drop 'em now.
Pointed out by Dan Carpenter.
v2: fix possible attempt to access beyond end of key table array,
also pointed out by Dan.
---
drivers/media/IR/imon.c | 12 ++++++------
1 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/media/IR/imon.c b/drivers/media/IR/imon.c
index 27743eb..efe219a 100644
--- a/drivers/media/IR/imon.c
+++ b/drivers/media/IR/imon.c
@@ -55,7 +55,6 @@
#define BIT_DURATION 250 /* each bit received is 250us */
#define IMON_CLOCK_ENABLE_PACKETS 2
-#define IMON_KEY_RELEASE_OFFSET 1000
/*** P R O T O T Y P E S ***/
@@ -1199,13 +1198,14 @@ static u32 imon_panel_key_lookup(u64 hw_code)
{
int i;
u64 code = be64_to_cpu(hw_code);
- u32 keycode;
+ u32 keycode = KEY_RESERVED;
- for (i = 0; i < ARRAY_SIZE(imon_panel_key_table); i++)
- if (imon_panel_key_table[i].hw_code == (code | 0xffee))
+ for (i = 0; i < ARRAY_SIZE(imon_panel_key_table); i++) {
+ if (imon_panel_key_table[i].hw_code == (code | 0xffee)) {
+ keycode = imon_panel_key_table[i].keycode;
break;
-
- keycode = imon_panel_key_table[i % IMON_KEY_RELEASE_OFFSET].keycode;
+ }
+ }
return keycode;
}
--
Jarod Wilson
jarod@redhat.com
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v2] IR/imon: remove dead IMON_KEY_RELEASE_OFFSET
2010-05-04 19:17 ` [PATCH v2] " Jarod Wilson
@ 2010-05-07 11:52 ` Mauro Carvalho Chehab
2010-05-07 13:13 ` Jarod Wilson
0 siblings, 1 reply; 6+ messages in thread
From: Mauro Carvalho Chehab @ 2010-05-07 11:52 UTC (permalink / raw)
To: Jarod Wilson; +Cc: Dan Carpenter, linux-media
Jarod Wilson wrote:
> On Tue, May 04, 2010 at 06:06:41PM +0200, Dan Carpenter wrote:
>> On Tue, May 04, 2010 at 10:03:18AM -0400, Jarod Wilson wrote:
>>> @@ -1205,7 +1204,7 @@ static u32 imon_panel_key_lookup(u64 hw_code)
>>> if (imon_panel_key_table[i].hw_code == (code | 0xffee))
>>> break;
>>>
>>> - keycode = imon_panel_key_table[i % IMON_KEY_RELEASE_OFFSET].keycode;
>>> + keycode = imon_panel_key_table[i].keycode;
>>>
>>> return keycode;
>>> }
>> There is still potentially a problem here because if we don't hit the
>> break statement, then we're one past the end of the array.
>
> D'oh. Okay, here's v2, should fix that buglet too.
>
> This hack was used when the imon driver was using internal key lookup
> routines, but became dead weight when the driver was converted to use
> ir-core's key lookup routines. These bits simply didn't get removed,
> drop 'em now.
>
> Pointed out by Dan Carpenter.
>
> v2: fix possible attempt to access beyond end of key table array,
> also pointed out by Dan.
-ENOSOB
Please, add your SOB here ;)
>
> ---
> drivers/media/IR/imon.c | 12 ++++++------
> 1 files changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/media/IR/imon.c b/drivers/media/IR/imon.c
> index 27743eb..efe219a 100644
> --- a/drivers/media/IR/imon.c
> +++ b/drivers/media/IR/imon.c
> @@ -55,7 +55,6 @@
> #define BIT_DURATION 250 /* each bit received is 250us */
>
> #define IMON_CLOCK_ENABLE_PACKETS 2
> -#define IMON_KEY_RELEASE_OFFSET 1000
>
> /*** P R O T O T Y P E S ***/
>
> @@ -1199,13 +1198,14 @@ static u32 imon_panel_key_lookup(u64 hw_code)
> {
> int i;
> u64 code = be64_to_cpu(hw_code);
> - u32 keycode;
> + u32 keycode = KEY_RESERVED;
>
> - for (i = 0; i < ARRAY_SIZE(imon_panel_key_table); i++)
> - if (imon_panel_key_table[i].hw_code == (code | 0xffee))
> + for (i = 0; i < ARRAY_SIZE(imon_panel_key_table); i++) {
> + if (imon_panel_key_table[i].hw_code == (code | 0xffee)) {
> + keycode = imon_panel_key_table[i].keycode;
> break;
> -
> - keycode = imon_panel_key_table[i % IMON_KEY_RELEASE_OFFSET].keycode;
> + }
> + }
>
> return keycode;
> }
>
--
Cheers,
Mauro
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] IR/imon: remove dead IMON_KEY_RELEASE_OFFSET
2010-05-07 11:52 ` Mauro Carvalho Chehab
@ 2010-05-07 13:13 ` Jarod Wilson
0 siblings, 0 replies; 6+ messages in thread
From: Jarod Wilson @ 2010-05-07 13:13 UTC (permalink / raw)
To: Mauro Carvalho Chehab; +Cc: Dan Carpenter, linux-media
On Fri, May 07, 2010 at 08:52:00AM -0300, Mauro Carvalho Chehab wrote:
> Jarod Wilson wrote:
> > On Tue, May 04, 2010 at 06:06:41PM +0200, Dan Carpenter wrote:
> >> On Tue, May 04, 2010 at 10:03:18AM -0400, Jarod Wilson wrote:
> >>> @@ -1205,7 +1204,7 @@ static u32 imon_panel_key_lookup(u64 hw_code)
> >>> if (imon_panel_key_table[i].hw_code == (code | 0xffee))
> >>> break;
> >>>
> >>> - keycode = imon_panel_key_table[i % IMON_KEY_RELEASE_OFFSET].keycode;
> >>> + keycode = imon_panel_key_table[i].keycode;
> >>>
> >>> return keycode;
> >>> }
> >> There is still potentially a problem here because if we don't hit the
> >> break statement, then we're one past the end of the array.
> >
> > D'oh. Okay, here's v2, should fix that buglet too.
> >
> > This hack was used when the imon driver was using internal key lookup
> > routines, but became dead weight when the driver was converted to use
> > ir-core's key lookup routines. These bits simply didn't get removed,
> > drop 'em now.
> >
> > Pointed out by Dan Carpenter.
> >
> > v2: fix possible attempt to access beyond end of key table array,
> > also pointed out by Dan.
>
> -ENOSOB
>
> Please, add your SOB here ;)
D'oh.
Signed-off-by: Jarod Wilson <jarod@redhat.com>
> > ---
> > drivers/media/IR/imon.c | 12 ++++++------
> > 1 files changed, 6 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/media/IR/imon.c b/drivers/media/IR/imon.c
> > index 27743eb..efe219a 100644
> > --- a/drivers/media/IR/imon.c
> > +++ b/drivers/media/IR/imon.c
> > @@ -55,7 +55,6 @@
> > #define BIT_DURATION 250 /* each bit received is 250us */
> >
> > #define IMON_CLOCK_ENABLE_PACKETS 2
> > -#define IMON_KEY_RELEASE_OFFSET 1000
> >
> > /*** P R O T O T Y P E S ***/
> >
> > @@ -1199,13 +1198,14 @@ static u32 imon_panel_key_lookup(u64 hw_code)
> > {
> > int i;
> > u64 code = be64_to_cpu(hw_code);
> > - u32 keycode;
> > + u32 keycode = KEY_RESERVED;
> >
> > - for (i = 0; i < ARRAY_SIZE(imon_panel_key_table); i++)
> > - if (imon_panel_key_table[i].hw_code == (code | 0xffee))
> > + for (i = 0; i < ARRAY_SIZE(imon_panel_key_table); i++) {
> > + if (imon_panel_key_table[i].hw_code == (code | 0xffee)) {
> > + keycode = imon_panel_key_table[i].keycode;
> > break;
> > -
> > - keycode = imon_panel_key_table[i % IMON_KEY_RELEASE_OFFSET].keycode;
> > + }
> > + }
> >
> > return keycode;
> > }
> >
>
>
> --
>
> Cheers,
> Mauro
--
Jarod Wilson
jarod@redhat.com
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2010-05-07 13:13 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-05-04 12:20 [-next] [bug report] media/IR/imon: array overflow Dan Carpenter
2010-05-04 14:03 ` [PATCH] IR/imon: remove dead IMON_KEY_RELEASE_OFFSET Jarod Wilson
2010-05-04 16:06 ` Dan Carpenter
2010-05-04 19:17 ` [PATCH v2] " Jarod Wilson
2010-05-07 11:52 ` Mauro Carvalho Chehab
2010-05-07 13:13 ` Jarod Wilson
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).