* 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 related [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 related [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 related [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).