* [-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