linux-media.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* KASAN: slab-out-of-bounds Read in technisat_usb2_rc_query
@ 2019-04-14 20:06 syzbot
  2019-07-02 14:02 ` [PATCH] media: usb: technisat-usb2: fix buffer overflow Phong Tran
  2019-07-03 14:52 ` [PATCH] media: technisat-usb2: break out of loop at end of buffer Sean Young
  0 siblings, 2 replies; 8+ messages in thread
From: syzbot @ 2019-04-14 20:06 UTC (permalink / raw)
  To: andreyknvl, hans.verkuil, keescook, linux-kernel, linux-media,
	linux-usb, mchehab, syzkaller-bugs

Hello,

syzbot found the following crash on:

HEAD commit:    9a33b369 usb-fuzzer: main usb gadget fuzzer driver
git tree:       https://github.com/google/kasan/tree/usb-fuzzer
console output: https://syzkaller.appspot.com/x/log.txt?x=1441219f200000
kernel config:  https://syzkaller.appspot.com/x/.config?x=23e37f59d94ddd15
dashboard link: https://syzkaller.appspot.com/bug?extid=eaaaf38a95427be88f4b
compiler:       gcc (GCC) 9.0.0 20181231 (experimental)
syz repro:      https://syzkaller.appspot.com/x/repro.syz?x=174a67bb200000
C reproducer:   https://syzkaller.appspot.com/x/repro.c?x=17aa2023200000

IMPORTANT: if you fix the bug, please add the following tag to the commit:
Reported-by: syzbot+eaaaf38a95427be88f4b@syzkaller.appspotmail.com

rc rc1: Technisat SkyStar USB HD (DVB-S/S2) as  
/devices/platform/dummy_hcd.0/usb1/1-1/rc/rc1
input: Technisat SkyStar USB HD (DVB-S/S2) as  
/devices/platform/dummy_hcd.0/usb1/1-1/rc/rc1/input10
rc rc1: lirc_dev: driver technisat-usb2 registered at minor = 1, raw IR  
receiver, no transmitter
dvb-usb: schedule remote query interval to 100 msecs.
==================================================================
BUG: KASAN: slab-out-of-bounds in technisat_usb2_get_ir  
drivers/media/usb/dvb-usb/technisat-usb2.c:664 [inline]
BUG: KASAN: slab-out-of-bounds in technisat_usb2_rc_query+0x5fa/0x660  
drivers/media/usb/dvb-usb/technisat-usb2.c:679
Read of size 1 at addr ffff8880a8791ea8 by task kworker/0:1/12

CPU: 0 PID: 12 Comm: kworker/0:1 Not tainted 5.1.0-rc4-319354-g9a33b36 #3
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS  
Google 01/01/2011
Workqueue: events dvb_usb_read_remote_control
Call Trace:
  __dump_stack lib/dump_stack.c:77 [inline]
  dump_stack+0xe8/0x16e lib/dump_stack.c:113
  print_address_description+0x6c/0x236 mm/kasan/report.c:187
  kasan_report.cold+0x1a/0x3c mm/kasan/report.c:317
  technisat_usb2_get_ir drivers/media/usb/dvb-usb/technisat-usb2.c:664  
[inline]
  technisat_usb2_rc_query+0x5fa/0x660  
drivers/media/usb/dvb-usb/technisat-usb2.c:679
  dvb_usb_read_remote_control  
drivers/media/usb/dvb-usb-v2/dvb_usb_core.c:128 [inline]
  dvb_usb_read_remote_control+0xe5/0x1c0  
drivers/media/usb/dvb-usb-v2/dvb_usb_core.c:105
  process_one_work+0x90f/0x1580 kernel/workqueue.c:2269
  worker_thread+0x9b/0xe20 kernel/workqueue.c:2415
  kthread+0x313/0x420 kernel/kthread.c:253
  ret_from_fork+0x3a/0x50 arch/x86/entry/entry_64.S:352

Allocated by task 615:
  set_track mm/kasan/common.c:87 [inline]
  __kasan_kmalloc mm/kasan/common.c:497 [inline]
  __kasan_kmalloc.constprop.0+0xbf/0xd0 mm/kasan/common.c:470
  dvb_usb_init drivers/media/usb/dvb-usb/dvb-usb-init.c:152 [inline]
  dvb_usb_device_init.cold+0x317/0x10b3  
drivers/media/usb/dvb-usb/dvb-usb-init.c:277
  technisat_usb2_probe+0x82/0x2d0  
drivers/media/usb/dvb-usb/technisat-usb2.c:763
  usb_probe_interface+0x31d/0x820 drivers/usb/core/driver.c:361
  really_probe+0x2da/0xb10 drivers/base/dd.c:509
  driver_probe_device+0x21d/0x350 drivers/base/dd.c:671
  __device_attach_driver+0x1d8/0x290 drivers/base/dd.c:778
  bus_for_each_drv+0x163/0x1e0 drivers/base/bus.c:454
  __device_attach+0x223/0x3a0 drivers/base/dd.c:844
  bus_probe_device+0x1f1/0x2a0 drivers/base/bus.c:514
  device_add+0xad2/0x16e0 drivers/base/core.c:2106
  usb_set_configuration+0xdf7/0x1740 drivers/usb/core/message.c:2021
  generic_probe+0xa2/0xda drivers/usb/core/generic.c:210
  usb_probe_device+0xc0/0x150 drivers/usb/core/driver.c:266
  really_probe+0x2da/0xb10 drivers/base/dd.c:509
  driver_probe_device+0x21d/0x350 drivers/base/dd.c:671
  __device_attach_driver+0x1d8/0x290 drivers/base/dd.c:778
  bus_for_each_drv+0x163/0x1e0 drivers/base/bus.c:454
  __device_attach+0x223/0x3a0 drivers/base/dd.c:844
  bus_probe_device+0x1f1/0x2a0 drivers/base/bus.c:514
  device_add+0xad2/0x16e0 drivers/base/core.c:2106
  usb_new_device.cold+0x537/0xccf drivers/usb/core/hub.c:2534
  hub_port_connect drivers/usb/core/hub.c:5089 [inline]
  hub_port_connect_change drivers/usb/core/hub.c:5204 [inline]
  port_event drivers/usb/core/hub.c:5350 [inline]
  hub_event+0x138e/0x3b00 drivers/usb/core/hub.c:5432
  process_one_work+0x90f/0x1580 kernel/workqueue.c:2269
  worker_thread+0x9b/0xe20 kernel/workqueue.c:2415
  kthread+0x313/0x420 kernel/kthread.c:253
  ret_from_fork+0x3a/0x50 arch/x86/entry/entry_64.S:352

Freed by task 1:
  set_track mm/kasan/common.c:87 [inline]
  __kasan_slab_free+0x130/0x180 mm/kasan/common.c:459
  slab_free_hook mm/slub.c:1429 [inline]
  slab_free_freelist_hook+0x5e/0x140 mm/slub.c:1456
  slab_free mm/slub.c:3003 [inline]
  kfree+0xce/0x290 mm/slub.c:3958
  krealloc+0x7d/0xc0 mm/slab_common.c:1570
  add_sysfs_param.isra.0+0xcd/0x930 kernel/params.c:633
  kernel_add_sysfs_param kernel/params.c:794 [inline]
  param_sysfs_builtin kernel/params.c:833 [inline]
  param_sysfs_init+0x364/0x435 kernel/params.c:954
  do_one_initcall+0xde/0x597 init/main.c:901
  do_initcall_level init/main.c:969 [inline]
  do_initcalls init/main.c:977 [inline]
  do_basic_setup init/main.c:995 [inline]
  kernel_init_freeable+0x4da/0x5c7 init/main.c:1150
  kernel_init+0x12/0x1ca init/main.c:1068
  ret_from_fork+0x3a/0x50 arch/x86/entry/entry_64.S:352

The buggy address belongs to the object at ffff8880a8791dc0
  which belongs to the cache kmalloc-256 of size 256
The buggy address is located 232 bytes inside of
  256-byte region [ffff8880a8791dc0, ffff8880a8791ec0)
The buggy address belongs to the page:
page:ffffea0002a1e440 count:1 mapcount:0 mapping:ffff88812c3f4e00 index:0x0
flags: 0xfff00000000200(slab)
raw: 00fff00000000200 dead000000000100 dead000000000200 ffff88812c3f4e00
raw: 0000000000000000 00000000000c000c 00000001ffffffff 0000000000000000
page dumped because: kasan: bad access detected

Memory state around the buggy address:
  ffff8880a8791d80: fc fc fc fc fc fc fc fc 00 00 00 00 00 00 00 00
  ffff8880a8791e00: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> ffff8880a8791e80: 00 00 00 00 00 fc fc fc fc fc fc fc fc fc fc fc
                                   ^
  ffff8880a8791f00: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
  ffff8880a8791f80: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
==================================================================


---
This bug is generated by a bot. It may contain errors.
See https://goo.gl/tpsmEJ for more information about syzbot.
syzbot engineers can be reached at syzkaller@googlegroups.com.

syzbot will keep track of this bug report. See:
https://goo.gl/tpsmEJ#status for how to communicate with syzbot.
syzbot can test patches for this bug, for details see:
https://goo.gl/tpsmEJ#testing-patches

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH] media: usb: technisat-usb2: fix buffer overflow
  2019-04-14 20:06 KASAN: slab-out-of-bounds Read in technisat_usb2_rc_query syzbot
@ 2019-07-02 14:02 ` Phong Tran
  2019-07-02 14:23   ` Alexander Potapenko
                     ` (2 more replies)
  2019-07-03 14:52 ` [PATCH] media: technisat-usb2: break out of loop at end of buffer Sean Young
  1 sibling, 3 replies; 8+ messages in thread
From: Phong Tran @ 2019-07-02 14:02 UTC (permalink / raw)
  To: syzbot+eaaaf38a95427be88f4b, andreyknvl, hans.verkuil, mchehab,
	skhan, gregkh
  Cc: keescook, linux-kernel, linux-media, linux-usb, syzkaller-bugs,
	linux-kernel-mentees, Phong Tran

The buffer will be overflow in case of the while loop can not break.
Add the checking buffer condition in while loop for avoiding
overlooping index.

This issue was reported by syzbot

Reported-by: syzbot+eaaaf38a95427be88f4b@syzkaller.appspotmail.com

Tested by:
https://groups.google.com/d/msg/syzkaller-bugs/CySBCKuUOOs/0hKq1CdjCwAJ

Signed-off-by: Phong Tran <tranmanphong@gmail.com>
---
 drivers/media/usb/dvb-usb/technisat-usb2.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/usb/dvb-usb/technisat-usb2.c b/drivers/media/usb/dvb-usb/technisat-usb2.c
index c659e18b358b..4e0b6185666a 100644
--- a/drivers/media/usb/dvb-usb/technisat-usb2.c
+++ b/drivers/media/usb/dvb-usb/technisat-usb2.c
@@ -655,7 +655,7 @@ static int technisat_usb2_get_ir(struct dvb_usb_device *d)
 #endif
 
 	ev.pulse = 0;
-	while (1) {
+	while (b != (buf + 63)) {
 		ev.pulse = !ev.pulse;
 		ev.duration = (*b * FIRMWARE_CLOCK_DIVISOR * FIRMWARE_CLOCK_TICK) / 1000;
 		ir_raw_event_store(d->rc_dev, &ev);
-- 
2.11.0


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] media: usb: technisat-usb2: fix buffer overflow
  2019-07-02 14:02 ` [PATCH] media: usb: technisat-usb2: fix buffer overflow Phong Tran
@ 2019-07-02 14:23   ` Alexander Potapenko
  2019-07-02 16:03   ` Kees Cook
  2019-07-03  2:14   ` [PATCH V2] " Phong Tran
  2 siblings, 0 replies; 8+ messages in thread
From: Alexander Potapenko @ 2019-07-02 14:23 UTC (permalink / raw)
  To: Phong Tran
  Cc: syzbot+eaaaf38a95427be88f4b, Andrey Konovalov, hans.verkuil,
	mchehab, skhan, gregkh, Kees Cook, LKML, linux-media, linux-usb,
	syzkaller-bugs, linux-kernel-mentees

On Tue, Jul 2, 2019 at 4:02 PM Phong Tran <tranmanphong@gmail.com> wrote:
>
> The buffer will be overflow in case of the while loop can not break.
> Add the checking buffer condition in while loop for avoiding
> overlooping index.
>
> This issue was reported by syzbot
>
> Reported-by: syzbot+eaaaf38a95427be88f4b@syzkaller.appspotmail.com
>
> Tested by:
> https://groups.google.com/d/msg/syzkaller-bugs/CySBCKuUOOs/0hKq1CdjCwAJ
>
> Signed-off-by: Phong Tran <tranmanphong@gmail.com>
> ---
>  drivers/media/usb/dvb-usb/technisat-usb2.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/media/usb/dvb-usb/technisat-usb2.c b/drivers/media/usb/dvb-usb/technisat-usb2.c
> index c659e18b358b..4e0b6185666a 100644
> --- a/drivers/media/usb/dvb-usb/technisat-usb2.c
> +++ b/drivers/media/usb/dvb-usb/technisat-usb2.c
> @@ -655,7 +655,7 @@ static int technisat_usb2_get_ir(struct dvb_usb_device *d)
>  #endif
>
>         ev.pulse = 0;
> -       while (1) {
> +       while (b != (buf + 63)) {
I think it won't hurt to either use ARRAY_SIZE here, or define some
magic constant for the buffer size in struct technisat_usb2_state.

>                 ev.pulse = !ev.pulse;
>                 ev.duration = (*b * FIRMWARE_CLOCK_DIVISOR * FIRMWARE_CLOCK_TICK) / 1000;
>                 ir_raw_event_store(d->rc_dev, &ev);
> --
> 2.11.0
>
> --
> You received this message because you are subscribed to the Google Groups "syzkaller-bugs" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to syzkaller-bugs+unsubscribe@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/syzkaller-bugs/20190702140211.28399-1-tranmanphong%40gmail.com.
> For more options, visit https://groups.google.com/d/optout.



-- 
Alexander Potapenko
Software Engineer

Google Germany GmbH
Erika-Mann-Straße, 33
80636 München

Geschäftsführer: Paul Manicle, Halimah DeLaine Prado
Registergericht und -nummer: Hamburg, HRB 86891
Sitz der Gesellschaft: Hamburg

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] media: usb: technisat-usb2: fix buffer overflow
  2019-07-02 14:02 ` [PATCH] media: usb: technisat-usb2: fix buffer overflow Phong Tran
  2019-07-02 14:23   ` Alexander Potapenko
@ 2019-07-02 16:03   ` Kees Cook
  2019-07-03  2:14   ` [PATCH V2] " Phong Tran
  2 siblings, 0 replies; 8+ messages in thread
From: Kees Cook @ 2019-07-02 16:03 UTC (permalink / raw)
  To: Phong Tran
  Cc: syzbot+eaaaf38a95427be88f4b, andreyknvl, hans.verkuil, mchehab,
	skhan, gregkh, linux-kernel, linux-media, linux-usb,
	syzkaller-bugs, linux-kernel-mentees

On Tue, Jul 02, 2019 at 09:02:11PM +0700, Phong Tran wrote:
> The buffer will be overflow in case of the while loop can not break.
> Add the checking buffer condition in while loop for avoiding
> overlooping index.
> 
> This issue was reported by syzbot
> 
> Reported-by: syzbot+eaaaf38a95427be88f4b@syzkaller.appspotmail.com
> 
> Tested by:
> https://groups.google.com/d/msg/syzkaller-bugs/CySBCKuUOOs/0hKq1CdjCwAJ
> 
> Signed-off-by: Phong Tran <tranmanphong@gmail.com>
> ---
>  drivers/media/usb/dvb-usb/technisat-usb2.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/media/usb/dvb-usb/technisat-usb2.c b/drivers/media/usb/dvb-usb/technisat-usb2.c
> index c659e18b358b..4e0b6185666a 100644
> --- a/drivers/media/usb/dvb-usb/technisat-usb2.c
> +++ b/drivers/media/usb/dvb-usb/technisat-usb2.c
> @@ -655,7 +655,7 @@ static int technisat_usb2_get_ir(struct dvb_usb_device *d)
>  #endif
>  
>  	ev.pulse = 0;
> -	while (1) {
> +	while (b != (buf + 63)) {

This matches the "62" from the earlier read -- instead of these literal
numbers, could you replace the "62"s with a named define for whatever
would make sense for this driver (maybe "IR_MAX_EVENTS"?), and then you
can make the above be something like:

	while (b <= buf + IR_MAX_EVENTS) {


>  		ev.pulse = !ev.pulse;
>  		ev.duration = (*b * FIRMWARE_CLOCK_DIVISOR * FIRMWARE_CLOCK_TICK) / 1000;
>  		ir_raw_event_store(d->rc_dev, &ev);
> -- 
> 2.11.0
> 

-- 
Kees Cook

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH V2] media: usb: technisat-usb2: fix buffer overflow
  2019-07-02 14:02 ` [PATCH] media: usb: technisat-usb2: fix buffer overflow Phong Tran
  2019-07-02 14:23   ` Alexander Potapenko
  2019-07-02 16:03   ` Kees Cook
@ 2019-07-03  2:14   ` Phong Tran
  2019-07-03  2:26     ` Kees Cook
  2 siblings, 1 reply; 8+ messages in thread
From: Phong Tran @ 2019-07-03  2:14 UTC (permalink / raw)
  To: tranmanphong
  Cc: andreyknvl, gregkh, hans.verkuil, keescook, linux-kernel-mentees,
	linux-kernel, linux-media, linux-usb, mchehab, skhan,
	syzbot+eaaaf38a95427be88f4b, syzkaller-bugs, glider

The buffer will be overflow in case of the while loop can not break.
Add the checking buffer condition in while loop for avoiding
overlooping index.

This issue was reported by syzbot

Reported-by: syzbot+eaaaf38a95427be88f4b@syzkaller.appspotmail.com

Tested-by:
https://groups.google.com/d/msg/syzkaller-bugs/CySBCKuUOOs/t3PvVheSAAAJ

Signed-off-by: Phong Tran <tranmanphong@gmail.com>
---
Change Log:
 * V2: add IR_MAX_BUFFER_INDEX and adjust the while loop condition as comments
---
 drivers/media/usb/dvb-usb/technisat-usb2.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/media/usb/dvb-usb/technisat-usb2.c b/drivers/media/usb/dvb-usb/technisat-usb2.c
index c659e18b358b..cdabff97c1ea 100644
--- a/drivers/media/usb/dvb-usb/technisat-usb2.c
+++ b/drivers/media/usb/dvb-usb/technisat-usb2.c
@@ -49,6 +49,7 @@ MODULE_PARM_DESC(disable_led_control,
 		"disable LED control of the device (default: 0 - LED control is active).");
 
 /* device private data */
+#define IR_MAX_BUFFER_INDEX	63
 struct technisat_usb2_state {
 	struct dvb_usb_device *dev;
 	struct delayed_work green_led_work;
@@ -56,7 +57,7 @@ struct technisat_usb2_state {
 
 	u16 last_scan_code;
 
-	u8 buf[64];
+	u8 buf[IR_MAX_BUFFER_INDEX + 1];
 };
 
 /* debug print helpers */
@@ -655,7 +656,7 @@ static int technisat_usb2_get_ir(struct dvb_usb_device *d)
 #endif
 
 	ev.pulse = 0;
-	while (1) {
+	while (b <= (buf + IR_MAX_BUFFER_INDEX)) {
 		ev.pulse = !ev.pulse;
 		ev.duration = (*b * FIRMWARE_CLOCK_DIVISOR * FIRMWARE_CLOCK_TICK) / 1000;
 		ir_raw_event_store(d->rc_dev, &ev);
-- 
2.11.0


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH V2] media: usb: technisat-usb2: fix buffer overflow
  2019-07-03  2:14   ` [PATCH V2] " Phong Tran
@ 2019-07-03  2:26     ` Kees Cook
  0 siblings, 0 replies; 8+ messages in thread
From: Kees Cook @ 2019-07-03  2:26 UTC (permalink / raw)
  To: Phong Tran
  Cc: andreyknvl, gregkh, hans.verkuil, linux-kernel-mentees,
	linux-kernel, linux-media, linux-usb, mchehab, skhan,
	syzbot+eaaaf38a95427be88f4b, syzkaller-bugs, glider

On Wed, Jul 03, 2019 at 09:14:44AM +0700, Phong Tran wrote:
> The buffer will be overflow in case of the while loop can not break.
> Add the checking buffer condition in while loop for avoiding
> overlooping index.
> 
> This issue was reported by syzbot
> 
> Reported-by: syzbot+eaaaf38a95427be88f4b@syzkaller.appspotmail.com
> 
> Tested-by:
> https://groups.google.com/d/msg/syzkaller-bugs/CySBCKuUOOs/t3PvVheSAAAJ
> 

Avoid these blank lines please. (More below...)

> Signed-off-by: Phong Tran <tranmanphong@gmail.com>
> ---
> Change Log:
>  * V2: add IR_MAX_BUFFER_INDEX and adjust the while loop condition as comments
> ---
>  drivers/media/usb/dvb-usb/technisat-usb2.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/media/usb/dvb-usb/technisat-usb2.c b/drivers/media/usb/dvb-usb/technisat-usb2.c
> index c659e18b358b..cdabff97c1ea 100644
> --- a/drivers/media/usb/dvb-usb/technisat-usb2.c
> +++ b/drivers/media/usb/dvb-usb/technisat-usb2.c
> @@ -49,6 +49,7 @@ MODULE_PARM_DESC(disable_led_control,
>  		"disable LED control of the device (default: 0 - LED control is active).");
>  
>  /* device private data */
> +#define IR_MAX_BUFFER_INDEX	63

How does this map to the literal "62" used before the loop you're
fixing?

Otherwise, it's looking good; thanks!

-Kees

>  struct technisat_usb2_state {
>  	struct dvb_usb_device *dev;
>  	struct delayed_work green_led_work;
> @@ -56,7 +57,7 @@ struct technisat_usb2_state {
>  
>  	u16 last_scan_code;
>  
> -	u8 buf[64];
> +	u8 buf[IR_MAX_BUFFER_INDEX + 1];
>  };
>  
>  /* debug print helpers */
> @@ -655,7 +656,7 @@ static int technisat_usb2_get_ir(struct dvb_usb_device *d)
>  #endif
>  
>  	ev.pulse = 0;
> -	while (1) {
> +	while (b <= (buf + IR_MAX_BUFFER_INDEX)) {
>  		ev.pulse = !ev.pulse;
>  		ev.duration = (*b * FIRMWARE_CLOCK_DIVISOR * FIRMWARE_CLOCK_TICK) / 1000;
>  		ir_raw_event_store(d->rc_dev, &ev);
> -- 
> 2.11.0
> 

-- 
Kees Cook

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH] media: technisat-usb2: break out of loop at end of buffer
  2019-04-14 20:06 KASAN: slab-out-of-bounds Read in technisat_usb2_rc_query syzbot
  2019-07-02 14:02 ` [PATCH] media: usb: technisat-usb2: fix buffer overflow Phong Tran
@ 2019-07-03 14:52 ` Sean Young
  2019-07-03 16:54   ` Kees Cook
  1 sibling, 1 reply; 8+ messages in thread
From: Sean Young @ 2019-07-03 14:52 UTC (permalink / raw)
  To: syzbot
  Cc: andreyknvl, hans.verkuil, keescook, linux-kernel, linux-media,
	linux-usb, mchehab, syzkaller-bugs

Ensure we do not access the buffer beyond the end if no 0xff byte
is encountered.

Reported-by: syzbot+eaaaf38a95427be88f4b@syzkaller.appspotmail.com
Signed-off-by: Sean Young <sean@mess.org>
---
 drivers/media/usb/dvb-usb/technisat-usb2.c | 22 ++++++++++------------
 1 file changed, 10 insertions(+), 12 deletions(-)

diff --git a/drivers/media/usb/dvb-usb/technisat-usb2.c b/drivers/media/usb/dvb-usb/technisat-usb2.c
index c659e18b358b..676d233d46d5 100644
--- a/drivers/media/usb/dvb-usb/technisat-usb2.c
+++ b/drivers/media/usb/dvb-usb/technisat-usb2.c
@@ -608,10 +608,9 @@ static int technisat_usb2_frontend_attach(struct dvb_usb_adapter *a)
 static int technisat_usb2_get_ir(struct dvb_usb_device *d)
 {
 	struct technisat_usb2_state *state = d->priv;
-	u8 *buf = state->buf;
-	u8 *b;
-	int ret;
 	struct ir_raw_event ev;
+	u8 *buf = state->buf;
+	int i, ret;
 
 	buf[0] = GET_IR_DATA_VENDOR_REQUEST;
 	buf[1] = 0x08;
@@ -647,26 +646,25 @@ static int technisat_usb2_get_ir(struct dvb_usb_device *d)
 		return 0; /* no key pressed */
 
 	/* decoding */
-	b = buf+1;
 
 #if 0
 	deb_rc("RC: %d ", ret);
-	debug_dump(b, ret, deb_rc);
+	debug_dump(buf + 1, ret, deb_rc);
 #endif
 
 	ev.pulse = 0;
-	while (1) {
-		ev.pulse = !ev.pulse;
-		ev.duration = (*b * FIRMWARE_CLOCK_DIVISOR * FIRMWARE_CLOCK_TICK) / 1000;
-		ir_raw_event_store(d->rc_dev, &ev);
-
-		b++;
-		if (*b == 0xff) {
+	for (i = 1; i < ARRAY_SIZE(state->buf); i++) {
+		if (buf[i] == 0xff) {
 			ev.pulse = 0;
 			ev.duration = 888888*2;
 			ir_raw_event_store(d->rc_dev, &ev);
 			break;
 		}
+
+		ev.pulse = !ev.pulse;
+		ev.duration = (buf[i] * FIRMWARE_CLOCK_DIVISOR *
+			       FIRMWARE_CLOCK_TICK) / 1000;
+		ir_raw_event_store(d->rc_dev, &ev);
 	}
 
 	ir_raw_event_handle(d->rc_dev);
-- 
2.21.0


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] media: technisat-usb2: break out of loop at end of buffer
  2019-07-03 14:52 ` [PATCH] media: technisat-usb2: break out of loop at end of buffer Sean Young
@ 2019-07-03 16:54   ` Kees Cook
  0 siblings, 0 replies; 8+ messages in thread
From: Kees Cook @ 2019-07-03 16:54 UTC (permalink / raw)
  To: Sean Young
  Cc: syzbot, andreyknvl, hans.verkuil, linux-kernel, linux-media,
	linux-usb, mchehab, syzkaller-bugs, Phong Tran

On Wed, Jul 03, 2019 at 03:52:39PM +0100, Sean Young wrote:
> Ensure we do not access the buffer beyond the end if no 0xff byte
> is encountered.
> 
> Reported-by: syzbot+eaaaf38a95427be88f4b@syzkaller.appspotmail.com
> Signed-off-by: Sean Young <sean@mess.org>

Both you and Phong Tran appear to be working on fixing this. At a
glance, this patch appears to be more complete in that it makes the code
flow more sane too.

Reviewed-by: Kees Cook <keescook@chromium.org>

-Kees

> ---
>  drivers/media/usb/dvb-usb/technisat-usb2.c | 22 ++++++++++------------
>  1 file changed, 10 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/media/usb/dvb-usb/technisat-usb2.c b/drivers/media/usb/dvb-usb/technisat-usb2.c
> index c659e18b358b..676d233d46d5 100644
> --- a/drivers/media/usb/dvb-usb/technisat-usb2.c
> +++ b/drivers/media/usb/dvb-usb/technisat-usb2.c
> @@ -608,10 +608,9 @@ static int technisat_usb2_frontend_attach(struct dvb_usb_adapter *a)
>  static int technisat_usb2_get_ir(struct dvb_usb_device *d)
>  {
>  	struct technisat_usb2_state *state = d->priv;
> -	u8 *buf = state->buf;
> -	u8 *b;
> -	int ret;
>  	struct ir_raw_event ev;
> +	u8 *buf = state->buf;
> +	int i, ret;
>  
>  	buf[0] = GET_IR_DATA_VENDOR_REQUEST;
>  	buf[1] = 0x08;
> @@ -647,26 +646,25 @@ static int technisat_usb2_get_ir(struct dvb_usb_device *d)
>  		return 0; /* no key pressed */
>  
>  	/* decoding */
> -	b = buf+1;
>  
>  #if 0
>  	deb_rc("RC: %d ", ret);
> -	debug_dump(b, ret, deb_rc);
> +	debug_dump(buf + 1, ret, deb_rc);
>  #endif
>  
>  	ev.pulse = 0;
> -	while (1) {
> -		ev.pulse = !ev.pulse;
> -		ev.duration = (*b * FIRMWARE_CLOCK_DIVISOR * FIRMWARE_CLOCK_TICK) / 1000;
> -		ir_raw_event_store(d->rc_dev, &ev);
> -
> -		b++;
> -		if (*b == 0xff) {
> +	for (i = 1; i < ARRAY_SIZE(state->buf); i++) {
> +		if (buf[i] == 0xff) {
>  			ev.pulse = 0;
>  			ev.duration = 888888*2;
>  			ir_raw_event_store(d->rc_dev, &ev);
>  			break;
>  		}
> +
> +		ev.pulse = !ev.pulse;
> +		ev.duration = (buf[i] * FIRMWARE_CLOCK_DIVISOR *
> +			       FIRMWARE_CLOCK_TICK) / 1000;
> +		ir_raw_event_store(d->rc_dev, &ev);
>  	}
>  
>  	ir_raw_event_handle(d->rc_dev);
> -- 
> 2.21.0
> 

-- 
Kees Cook

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2019-07-03 16:55 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-04-14 20:06 KASAN: slab-out-of-bounds Read in technisat_usb2_rc_query syzbot
2019-07-02 14:02 ` [PATCH] media: usb: technisat-usb2: fix buffer overflow Phong Tran
2019-07-02 14:23   ` Alexander Potapenko
2019-07-02 16:03   ` Kees Cook
2019-07-03  2:14   ` [PATCH V2] " Phong Tran
2019-07-03  2:26     ` Kees Cook
2019-07-03 14:52 ` [PATCH] media: technisat-usb2: break out of loop at end of buffer Sean Young
2019-07-03 16:54   ` Kees Cook

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).