* [PATCH] media: rc: ir-lirc-codec: fix potential integer overflow
@ 2010-11-26 17:06 ` Vasiliy Kulikov
0 siblings, 0 replies; 16+ messages in thread
From: Vasiliy Kulikov @ 2010-11-26 17:06 UTC (permalink / raw)
To: kernel-janitors
Cc: Mauro Carvalho Chehab, David Härdeman, Jarod Wilson,
linux-media, linux-kernel
'n' may be bigger than MAX_INT*sizeof(int), if so checking of truncated
(int)(n/sizeof(int)) for LIRCBUF_SIZE overflow and then using nontruncated 'count'
doesn't make sense. Also n may be up to sizeof(int)-1 bytes bigger than expected,
so check value of (n % sizeof(int)) too.
Signed-off-by: Vasiliy Kulikov <segoon@openwall.com>
---
Compile tested only.
drivers/media/rc/ir-lirc-codec.c | 5 +++--
1 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/media/rc/ir-lirc-codec.c b/drivers/media/rc/ir-lirc-codec.c
index 1e87ee8..f011c5d 100644
--- a/drivers/media/rc/ir-lirc-codec.c
+++ b/drivers/media/rc/ir-lirc-codec.c
@@ -100,7 +100,8 @@ static ssize_t ir_lirc_transmit_ir(struct file *file, const char *buf,
struct lirc_codec *lirc;
struct rc_dev *dev;
int *txbuf; /* buffer with values to transmit */
- int ret = 0, count;
+ int ret = 0;
+ size_t count;
lirc = lirc_get_pdata(file);
if (!lirc)
@@ -110,7 +111,7 @@ static ssize_t ir_lirc_transmit_ir(struct file *file, const char *buf,
return -EINVAL;
count = n / sizeof(int);
- if (count > LIRCBUF_SIZE || count % 2 == 0)
+ if (count > LIRCBUF_SIZE || count % 2 == 0 || n % sizeof(int) != 0)
return -EINVAL;
txbuf = memdup_user(buf, n);
--
1.7.0.4
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH] media: rc: ir-lirc-codec: fix potential integer overflow
@ 2010-11-26 17:06 ` Vasiliy Kulikov
0 siblings, 0 replies; 16+ messages in thread
From: Vasiliy Kulikov @ 2010-11-26 17:06 UTC (permalink / raw)
To: kernel-janitors
Cc: Mauro Carvalho Chehab, David Härdeman, Jarod Wilson,
linux-media, linux-kernel
'n' may be bigger than MAX_INT*sizeof(int), if so checking of truncated
(int)(n/sizeof(int)) for LIRCBUF_SIZE overflow and then using nontruncated 'count'
doesn't make sense. Also n may be up to sizeof(int)-1 bytes bigger than expected,
so check value of (n % sizeof(int)) too.
Signed-off-by: Vasiliy Kulikov <segoon@openwall.com>
---
Compile tested only.
drivers/media/rc/ir-lirc-codec.c | 5 +++--
1 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/media/rc/ir-lirc-codec.c b/drivers/media/rc/ir-lirc-codec.c
index 1e87ee8..f011c5d 100644
--- a/drivers/media/rc/ir-lirc-codec.c
+++ b/drivers/media/rc/ir-lirc-codec.c
@@ -100,7 +100,8 @@ static ssize_t ir_lirc_transmit_ir(struct file *file, const char *buf,
struct lirc_codec *lirc;
struct rc_dev *dev;
int *txbuf; /* buffer with values to transmit */
- int ret = 0, count;
+ int ret = 0;
+ size_t count;
lirc = lirc_get_pdata(file);
if (!lirc)
@@ -110,7 +111,7 @@ static ssize_t ir_lirc_transmit_ir(struct file *file, const char *buf,
return -EINVAL;
count = n / sizeof(int);
- if (count > LIRCBUF_SIZE || count % 2 = 0)
+ if (count > LIRCBUF_SIZE || count % 2 = 0 || n % sizeof(int) != 0)
return -EINVAL;
txbuf = memdup_user(buf, n);
--
1.7.0.4
^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH] media: rc: ir-lirc-codec: fix potential integer overflow
2010-11-26 17:06 ` Vasiliy Kulikov
@ 2010-12-02 2:47 ` Jarod Wilson
-1 siblings, 0 replies; 16+ messages in thread
From: Jarod Wilson @ 2010-12-02 2:47 UTC (permalink / raw)
To: Vasiliy Kulikov
Cc: kernel-janitors, Mauro Carvalho Chehab, David Härdeman,
Jarod Wilson, linux-media, linux-kernel
On Nov 26, 2010, at 12:06 PM, Vasiliy Kulikov wrote:
> 'n' may be bigger than MAX_INT*sizeof(int), if so checking of truncated
> (int)(n/sizeof(int)) for LIRCBUF_SIZE overflow and then using nontruncated 'count'
> doesn't make sense. Also n may be up to sizeof(int)-1 bytes bigger than expected,
> so check value of (n % sizeof(int)) too.
>
> Signed-off-by: Vasiliy Kulikov <segoon@openwall.com>
> ---
> Compile tested only.
Looks sane.
Acked-by: Jarod Wilson <jarod@redhat.com>
--
Jarod Wilson
jarod@wilsonet.com
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] media: rc: ir-lirc-codec: fix potential integer overflow
@ 2010-12-02 2:47 ` Jarod Wilson
0 siblings, 0 replies; 16+ messages in thread
From: Jarod Wilson @ 2010-12-02 2:47 UTC (permalink / raw)
To: Vasiliy Kulikov
Cc: kernel-janitors, Mauro Carvalho Chehab, David Härdeman,
Jarod Wilson, linux-media, linux-kernel
On Nov 26, 2010, at 12:06 PM, Vasiliy Kulikov wrote:
> 'n' may be bigger than MAX_INT*sizeof(int), if so checking of truncated
> (int)(n/sizeof(int)) for LIRCBUF_SIZE overflow and then using nontruncated 'count'
> doesn't make sense. Also n may be up to sizeof(int)-1 bytes bigger than expected,
> so check value of (n % sizeof(int)) too.
>
> Signed-off-by: Vasiliy Kulikov <segoon@openwall.com>
> ---
> Compile tested only.
Looks sane.
Acked-by: Jarod Wilson <jarod@redhat.com>
--
Jarod Wilson
jarod@wilsonet.com
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] media: rc: ir-lirc-codec: fix potential integer overflow
2010-11-26 17:06 ` Vasiliy Kulikov
@ 2010-12-02 4:51 ` Dan Carpenter
-1 siblings, 0 replies; 16+ messages in thread
From: Dan Carpenter @ 2010-12-02 4:51 UTC (permalink / raw)
To: Vasiliy Kulikov
Cc: kernel-janitors, Mauro Carvalho Chehab, David Härdeman,
Jarod Wilson, linux-media, linux-kernel
On Fri, Nov 26, 2010 at 08:06:35PM +0300, Vasiliy Kulikov wrote:
> count = n / sizeof(int);
> - if (count > LIRCBUF_SIZE || count % 2 == 0)
> + if (count > LIRCBUF_SIZE || count % 2 == 0 || n % sizeof(int) != 0)
^^^^^^^^^^^^^^^^^^^^
Wait, what? We just checked this a couple lines before.
The rest of the patch is right and a clever catch. It would affect
x86_64 systems and not i386. This doesn't have security implications
does it? You'd just catch the kmalloc() stack trace for insanely large
allocations.
regards,
dan carpenter
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] media: rc: ir-lirc-codec: fix potential integer
@ 2010-12-02 4:51 ` Dan Carpenter
0 siblings, 0 replies; 16+ messages in thread
From: Dan Carpenter @ 2010-12-02 4:51 UTC (permalink / raw)
To: Vasiliy Kulikov
Cc: kernel-janitors, Mauro Carvalho Chehab, David Härdeman,
Jarod Wilson, linux-media, linux-kernel
On Fri, Nov 26, 2010 at 08:06:35PM +0300, Vasiliy Kulikov wrote:
> count = n / sizeof(int);
> - if (count > LIRCBUF_SIZE || count % 2 = 0)
> + if (count > LIRCBUF_SIZE || count % 2 = 0 || n % sizeof(int) != 0)
^^^^^^^^^^^^^^^^^^^^
Wait, what? We just checked this a couple lines before.
The rest of the patch is right and a clever catch. It would affect
x86_64 systems and not i386. This doesn't have security implications
does it? You'd just catch the kmalloc() stack trace for insanely large
allocations.
regards,
dan carpenter
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] media: rc: ir-lirc-codec: fix potential integer overflow
2010-12-02 4:51 ` [PATCH] media: rc: ir-lirc-codec: fix potential integer Dan Carpenter
@ 2010-12-02 15:00 ` Jarod Wilson
-1 siblings, 0 replies; 16+ messages in thread
From: Jarod Wilson @ 2010-12-02 15:00 UTC (permalink / raw)
To: Dan Carpenter, Vasiliy Kulikov, kernel-janitors,
Mauro Carvalho Chehab, David Härdeman, linux-media,
linux-kernel
On Thu, Dec 02, 2010 at 07:51:26AM +0300, Dan Carpenter wrote:
> On Fri, Nov 26, 2010 at 08:06:35PM +0300, Vasiliy Kulikov wrote:
> > count = n / sizeof(int);
> > - if (count > LIRCBUF_SIZE || count % 2 == 0)
> > + if (count > LIRCBUF_SIZE || count % 2 == 0 || n % sizeof(int) != 0)
> ^^^^^^^^^^^^^^^^^^^^
>
> Wait, what? We just checked this a couple lines before.
Bah. I'd only looked at the diff, which didn't have enough context. I
thought that looked familiar. Indeed, this part seems to be unnecessary.
> The rest of the patch is right and a clever catch. It would affect
> x86_64 systems and not i386. This doesn't have security implications
> does it? You'd just catch the kmalloc() stack trace for insanely large
> allocations.
Even on x86_64, it looks to my (relatively untrained) eye like you'd
actually be fine. n is a size_t (so, 64-bit on x86_64). count is an int
(so 32-bit on x86_64). We initialize count to some 64-bit value / 4, so
at most, 16 bits, which always fits just fine in the 32-bit int, no?
--
Jarod Wilson
jarod@redhat.com
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] media: rc: ir-lirc-codec: fix potential integer overflow
@ 2010-12-02 15:00 ` Jarod Wilson
0 siblings, 0 replies; 16+ messages in thread
From: Jarod Wilson @ 2010-12-02 15:00 UTC (permalink / raw)
To: Dan Carpenter, Vasiliy Kulikov, kernel-janitors,
Mauro Carvalho Chehab, David Härdeman, linux-media,
linux-kernel
On Thu, Dec 02, 2010 at 07:51:26AM +0300, Dan Carpenter wrote:
> On Fri, Nov 26, 2010 at 08:06:35PM +0300, Vasiliy Kulikov wrote:
> > count = n / sizeof(int);
> > - if (count > LIRCBUF_SIZE || count % 2 = 0)
> > + if (count > LIRCBUF_SIZE || count % 2 = 0 || n % sizeof(int) != 0)
> ^^^^^^^^^^^^^^^^^^^^
>
> Wait, what? We just checked this a couple lines before.
Bah. I'd only looked at the diff, which didn't have enough context. I
thought that looked familiar. Indeed, this part seems to be unnecessary.
> The rest of the patch is right and a clever catch. It would affect
> x86_64 systems and not i386. This doesn't have security implications
> does it? You'd just catch the kmalloc() stack trace for insanely large
> allocations.
Even on x86_64, it looks to my (relatively untrained) eye like you'd
actually be fine. n is a size_t (so, 64-bit on x86_64). count is an int
(so 32-bit on x86_64). We initialize count to some 64-bit value / 4, so
at most, 16 bits, which always fits just fine in the 32-bit int, no?
--
Jarod Wilson
jarod@redhat.com
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] media: rc: ir-lirc-codec: fix potential integer overflow
2010-12-02 15:00 ` Jarod Wilson
@ 2010-12-02 18:55 ` Jarod Wilson
-1 siblings, 0 replies; 16+ messages in thread
From: Jarod Wilson @ 2010-12-02 18:55 UTC (permalink / raw)
To: Dan Carpenter
Cc: Vasiliy Kulikov, kernel-janitors, Mauro Carvalho Chehab,
David Härdeman, linux-media, linux-kernel
On Dec 2, 2010, at 10:00 AM, Jarod Wilson wrote:
> On Thu, Dec 02, 2010 at 07:51:26AM +0300, Dan Carpenter wrote:
>> On Fri, Nov 26, 2010 at 08:06:35PM +0300, Vasiliy Kulikov wrote:
>>> count = n / sizeof(int);
>>> - if (count > LIRCBUF_SIZE || count % 2 == 0)
>>> + if (count > LIRCBUF_SIZE || count % 2 == 0 || n % sizeof(int) != 0)
>> ^^^^^^^^^^^^^^^^^^^^
>>
>> Wait, what? We just checked this a couple lines before.
>
> Bah. I'd only looked at the diff, which didn't have enough context. I
> thought that looked familiar. Indeed, this part seems to be unnecessary.
>
>> The rest of the patch is right and a clever catch. It would affect
>> x86_64 systems and not i386. This doesn't have security implications
>> does it? You'd just catch the kmalloc() stack trace for insanely large
>> allocations.
>
> Even on x86_64, it looks to my (relatively untrained) eye like you'd
> actually be fine. n is a size_t (so, 64-bit on x86_64). count is an int
> (so 32-bit on x86_64). We initialize count to some 64-bit value / 4, so
> at most, 16 bits, which always fits just fine in the 32-bit int, no?
Never mind, I shouldn't be allowed near computers on too little sleep.
Its been pointed out to me how incredibly incorrect and stupid what I
said above is. :)
(i.e., we're not dividing the bits by 4, we're dividing a 64-bit value
by 4, so you're still in 62-bit territory.)
/me sticks head back in sand
--
Jarod Wilson
jarod@wilsonet.com
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] media: rc: ir-lirc-codec: fix potential integer overflow
@ 2010-12-02 18:55 ` Jarod Wilson
0 siblings, 0 replies; 16+ messages in thread
From: Jarod Wilson @ 2010-12-02 18:55 UTC (permalink / raw)
To: Dan Carpenter
Cc: Vasiliy Kulikov, kernel-janitors, Mauro Carvalho Chehab,
David Härdeman, linux-media, linux-kernel
On Dec 2, 2010, at 10:00 AM, Jarod Wilson wrote:
> On Thu, Dec 02, 2010 at 07:51:26AM +0300, Dan Carpenter wrote:
>> On Fri, Nov 26, 2010 at 08:06:35PM +0300, Vasiliy Kulikov wrote:
>>> count = n / sizeof(int);
>>> - if (count > LIRCBUF_SIZE || count % 2 = 0)
>>> + if (count > LIRCBUF_SIZE || count % 2 = 0 || n % sizeof(int) != 0)
>> ^^^^^^^^^^^^^^^^^^^^
>>
>> Wait, what? We just checked this a couple lines before.
>
> Bah. I'd only looked at the diff, which didn't have enough context. I
> thought that looked familiar. Indeed, this part seems to be unnecessary.
>
>> The rest of the patch is right and a clever catch. It would affect
>> x86_64 systems and not i386. This doesn't have security implications
>> does it? You'd just catch the kmalloc() stack trace for insanely large
>> allocations.
>
> Even on x86_64, it looks to my (relatively untrained) eye like you'd
> actually be fine. n is a size_t (so, 64-bit on x86_64). count is an int
> (so 32-bit on x86_64). We initialize count to some 64-bit value / 4, so
> at most, 16 bits, which always fits just fine in the 32-bit int, no?
Never mind, I shouldn't be allowed near computers on too little sleep.
Its been pointed out to me how incredibly incorrect and stupid what I
said above is. :)
(i.e., we're not dividing the bits by 4, we're dividing a 64-bit value
by 4, so you're still in 62-bit territory.)
/me sticks head back in sand
--
Jarod Wilson
jarod@wilsonet.com
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] media: rc: ir-lirc-codec: fix potential integer overflow
2010-11-26 17:06 ` Vasiliy Kulikov
@ 2010-12-02 21:08 ` Andy Walls
-1 siblings, 0 replies; 16+ messages in thread
From: Andy Walls @ 2010-12-02 21:08 UTC (permalink / raw)
To: Jarod Wilson, Dan Carpenter, Vasiliy Kulikov, kernel-janitors,
Mauro Carvalho Chehab, David Härdeman, linux-media,
linux-kernel
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset=utf-8, Size: 1587 bytes --]
64 bit value / 4 = 62 bit value, right?
Jarod Wilson <jarod@redhat.com> wrote:
>On Thu, Dec 02, 2010 at 07:51:26AM +0300, Dan Carpenter wrote:
>> On Fri, Nov 26, 2010 at 08:06:35PM +0300, Vasiliy Kulikov wrote:
>> > count = n / sizeof(int);
>> > - if (count > LIRCBUF_SIZE || count % 2 == 0)
>> > + if (count > LIRCBUF_SIZE || count % 2 == 0 || n % sizeof(int) != 0)
>> ^^^^^^^^^^^^^^^^^^^^
>>
>> Wait, what? We just checked this a couple lines before.
>
>Bah. I'd only looked at the diff, which didn't have enough context. I
>thought that looked familiar. Indeed, this part seems to be unnecessary.
>
>> The rest of the patch is right and a clever catch. It would affect
>> x86_64 systems and not i386. This doesn't have security implications
>> does it? You'd just catch the kmalloc() stack trace for insanely large
>> allocations.
>
>Even on x86_64, it looks to my (relatively untrained) eye like you'd
>actually be fine. n is a size_t (so, 64-bit on x86_64). count is an int
>(so 32-bit on x86_64). We initialize count to some 64-bit value / 4, so
>at most, 16 bits, which always fits just fine in the 32-bit int, no?
>
>--
>Jarod Wilson
>jarod@redhat.com
>
>--
>To unsubscribe from this list: send the line "unsubscribe linux-media" in
>the body of a message to majordomo@vger.kernel.org
>More majordomo info at http://vger.kernel.org/majordomo-info.html
ÿôèº{.nÇ+·®+%Ëÿ±éݶ\x17¥wÿº{.nÇ+·¥{±þG«éÿ{ayº\x1dÊÚë,j\a¢f£¢·hïêÿêçz_è®\x03(éÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?¨èÚ&£ø§~á¶iOæ¬z·vØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?I¥
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] media: rc: ir-lirc-codec: fix potential integer
@ 2010-12-02 21:08 ` Andy Walls
0 siblings, 0 replies; 16+ messages in thread
From: Andy Walls @ 2010-12-02 21:08 UTC (permalink / raw)
To: Jarod Wilson, Dan Carpenter, Vasiliy Kulikov, kernel-janitors,
Mauro Carvalho Chehab, David Härdeman, linux-media,
linux-kernel
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="windows-1254", Size: 1555 bytes --]
64 bit value / 4 = 62 bit value, right?
Jarod Wilson <jarod@redhat.com> wrote:
>On Thu, Dec 02, 2010 at 07:51:26AM +0300, Dan Carpenter wrote:
>> On Fri, Nov 26, 2010 at 08:06:35PM +0300, Vasiliy Kulikov wrote:
>> > count = n / sizeof(int);
>> > - if (count > LIRCBUF_SIZE || count % 2 = 0)
>> > + if (count > LIRCBUF_SIZE || count % 2 = 0 || n % sizeof(int) != 0)
>> ^^^^^^^^^^^^^^^^^^^^
>>
>> Wait, what? We just checked this a couple lines before.
>
>Bah. I'd only looked at the diff, which didn't have enough context. I
>thought that looked familiar. Indeed, this part seems to be unnecessary.
>
>> The rest of the patch is right and a clever catch. It would affect
>> x86_64 systems and not i386. This doesn't have security implications
>> does it? You'd just catch the kmalloc() stack trace for insanely large
>> allocations.
>
>Even on x86_64, it looks to my (relatively untrained) eye like you'd
>actually be fine. n is a size_t (so, 64-bit on x86_64). count is an int
>(so 32-bit on x86_64). We initialize count to some 64-bit value / 4, so
>at most, 16 bits, which always fits just fine in the 32-bit int, no?
>
>--
>Jarod Wilson
>jarod@redhat.com
>
>--
>To unsubscribe from this list: send the line "unsubscribe linux-media" in
>the body of a message to majordomo@vger.kernel.org
>More majordomo info at http://vger.kernel.org/majordomo-info.html
ÿôèº{.nÇ+·®+%Ëÿ±éݶ\x17¥wÿº{.nÇ+·¤z¹ÞøÚ+h®ÏâØ^nr¡ö¦zË\x1aëh¨èÚ&£ûàz¿äz¹Þú+Ê+zf£¢·h§~Ûiÿÿïêÿêçz_è®\x0fæj:+v¨þ)ߣøm
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH v2] media: rc: ir-lirc-codec: fix integer overflow
2010-12-02 4:51 ` [PATCH] media: rc: ir-lirc-codec: fix potential integer Dan Carpenter
@ 2010-12-04 21:05 ` Vasiliy Kulikov
-1 siblings, 0 replies; 16+ messages in thread
From: Vasiliy Kulikov @ 2010-12-04 21:05 UTC (permalink / raw)
To: Dan Carpenter, kernel-janitors, Mauro Carvalho Chehab,
David Härdeman, Jarod Wilson, linux-media, linux-kernel
'n' may be bigger than MAX_INT*sizeof(int), if so checking of truncated
(int)(n/sizeof(int)) for LIRCBUF_SIZE overflows and then using nontruncated 'count'
doesn't make sense. This is not a security issue as too big 'n' is catched in
kmalloc() in memdup_user() call. However, it's better to prevent WARN() in kmalloc().
Signed-off-by: Vasiliy Kulikov <segoon@openwall.com>
---
Compile tested only.
drivers/media/rc/ir-lirc-codec.c | 3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/drivers/media/rc/ir-lirc-codec.c b/drivers/media/rc/ir-lirc-codec.c
index 1e87ee8..a7e91e6 100644
--- a/drivers/media/rc/ir-lirc-codec.c
+++ b/drivers/media/rc/ir-lirc-codec.c
@@ -100,7 +100,8 @@ static ssize_t ir_lirc_transmit_ir(struct file *file, const char *buf,
struct lirc_codec *lirc;
struct rc_dev *dev;
int *txbuf; /* buffer with values to transmit */
- int ret = 0, count;
+ int ret = 0;
+ size_t count;
lirc = lirc_get_pdata(file);
if (!lirc)
--
1.7.0.4
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH v2] media: rc: ir-lirc-codec: fix integer overflow
@ 2010-12-04 21:05 ` Vasiliy Kulikov
0 siblings, 0 replies; 16+ messages in thread
From: Vasiliy Kulikov @ 2010-12-04 21:05 UTC (permalink / raw)
To: Dan Carpenter, kernel-janitors, Mauro Carvalho Chehab,
David Härdeman, Jarod Wilson, linux-media, linux-kernel
'n' may be bigger than MAX_INT*sizeof(int), if so checking of truncated
(int)(n/sizeof(int)) for LIRCBUF_SIZE overflows and then using nontruncated 'count'
doesn't make sense. This is not a security issue as too big 'n' is catched in
kmalloc() in memdup_user() call. However, it's better to prevent WARN() in kmalloc().
Signed-off-by: Vasiliy Kulikov <segoon@openwall.com>
---
Compile tested only.
drivers/media/rc/ir-lirc-codec.c | 3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/drivers/media/rc/ir-lirc-codec.c b/drivers/media/rc/ir-lirc-codec.c
index 1e87ee8..a7e91e6 100644
--- a/drivers/media/rc/ir-lirc-codec.c
+++ b/drivers/media/rc/ir-lirc-codec.c
@@ -100,7 +100,8 @@ static ssize_t ir_lirc_transmit_ir(struct file *file, const char *buf,
struct lirc_codec *lirc;
struct rc_dev *dev;
int *txbuf; /* buffer with values to transmit */
- int ret = 0, count;
+ int ret = 0;
+ size_t count;
lirc = lirc_get_pdata(file);
if (!lirc)
--
1.7.0.4
^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH v2] media: rc: ir-lirc-codec: fix integer overflow
2010-12-04 21:05 ` Vasiliy Kulikov
@ 2010-12-08 16:15 ` Jarod Wilson
-1 siblings, 0 replies; 16+ messages in thread
From: Jarod Wilson @ 2010-12-08 16:15 UTC (permalink / raw)
To: Vasiliy Kulikov
Cc: Dan Carpenter, kernel-janitors, Mauro Carvalho Chehab,
David Härdeman, linux-media, linux-kernel
On Sun, Dec 05, 2010 at 12:05:22AM +0300, Vasiliy Kulikov wrote:
> 'n' may be bigger than MAX_INT*sizeof(int), if so checking of truncated
> (int)(n/sizeof(int)) for LIRCBUF_SIZE overflows and then using nontruncated 'count'
> doesn't make sense. This is not a security issue as too big 'n' is catched in
> kmalloc() in memdup_user() call. However, it's better to prevent WARN() in kmalloc().
>
> Signed-off-by: Vasiliy Kulikov <segoon@openwall.com>
Now that I have my head out of my arse wrt the actual issue here, the
redundancy issue from v1 is resolved, and I've managed a full night's
sleep... ;)
Acked-by: Jarod Wilson <jarod@redhat.com>
--
Jarod Wilson
jarod@redhat.com
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v2] media: rc: ir-lirc-codec: fix integer overflow
@ 2010-12-08 16:15 ` Jarod Wilson
0 siblings, 0 replies; 16+ messages in thread
From: Jarod Wilson @ 2010-12-08 16:15 UTC (permalink / raw)
To: Vasiliy Kulikov
Cc: Dan Carpenter, kernel-janitors, Mauro Carvalho Chehab,
David Härdeman, linux-media, linux-kernel
On Sun, Dec 05, 2010 at 12:05:22AM +0300, Vasiliy Kulikov wrote:
> 'n' may be bigger than MAX_INT*sizeof(int), if so checking of truncated
> (int)(n/sizeof(int)) for LIRCBUF_SIZE overflows and then using nontruncated 'count'
> doesn't make sense. This is not a security issue as too big 'n' is catched in
> kmalloc() in memdup_user() call. However, it's better to prevent WARN() in kmalloc().
>
> Signed-off-by: Vasiliy Kulikov <segoon@openwall.com>
Now that I have my head out of my arse wrt the actual issue here, the
redundancy issue from v1 is resolved, and I've managed a full night's
sleep... ;)
Acked-by: Jarod Wilson <jarod@redhat.com>
--
Jarod Wilson
jarod@redhat.com
^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2010-12-08 16:16 UTC | newest]
Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-12-02 21:08 [PATCH] media: rc: ir-lirc-codec: fix potential integer overflow Andy Walls
2010-12-02 21:08 ` [PATCH] media: rc: ir-lirc-codec: fix potential integer Andy Walls
-- strict thread matches above, loose matches on Subject: below --
2010-12-02 4:51 [PATCH] media: rc: ir-lirc-codec: fix potential integer overflow Dan Carpenter
2010-12-02 4:51 ` [PATCH] media: rc: ir-lirc-codec: fix potential integer Dan Carpenter
2010-12-02 15:00 ` [PATCH] media: rc: ir-lirc-codec: fix potential integer overflow Jarod Wilson
2010-12-02 15:00 ` Jarod Wilson
2010-12-02 18:55 ` Jarod Wilson
2010-12-02 18:55 ` Jarod Wilson
2010-12-04 21:05 ` [PATCH v2] media: rc: ir-lirc-codec: fix " Vasiliy Kulikov
2010-12-04 21:05 ` Vasiliy Kulikov
2010-12-08 16:15 ` Jarod Wilson
2010-12-08 16:15 ` Jarod Wilson
2010-11-26 17:06 [PATCH] media: rc: ir-lirc-codec: fix potential " Vasiliy Kulikov
2010-11-26 17:06 ` Vasiliy Kulikov
2010-12-02 2:47 ` Jarod Wilson
2010-12-02 2:47 ` Jarod Wilson
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.