usb/core: slab-out-of-bounds read in cdc_parse_cdc_header

380 views
Skip to first unread message

Andrey Konovalov

unread,
Sep 20, 2017, 10:45:09 AM9/20/17
to Greg Kroah-Hartman, Jonathan Corbet, Mauro Carvalho Chehab, Jaejoong Kim, USB list, LKML, Dmitry Vyukov, Kostya Serebryany, syzkaller
Hi!

I've got the following crash while fuzzing the kernel with syzkaller.

On commit ebb2c2437d8008d46796902ff390653822af6cc4 (Sep 18).

It looks like cdc_parse_cdc_header() doesn't validate buflen before
accessing buffer[1], buffer[2] and so on. The only check present is
while (buflen > 0).

==================================================================
BUG: KASAN: slab-out-of-bounds in cdc_parse_cdc_header+0x611/0x6b0
Read of size 1 at addr ffff88006b03e554 by task kworker/1:2/2573

CPU: 1 PID: 2573 Comm: kworker/1:2 Not tainted 4.14.0-rc1+ #191
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011
Workqueue: usb_hub_wq hub_event
Call Trace:
__dump_stack lib/dump_stack.c:16
dump_stack+0x192/0x22c lib/dump_stack.c:52
print_address_description+0x78/0x280 mm/kasan/report.c:252
kasan_report_error mm/kasan/report.c:351
kasan_report+0x230/0x340 mm/kasan/report.c:409
__asan_report_load1_noabort+0x19/0x20 mm/kasan/report.c:427
cdc_parse_cdc_header+0x611/0x6b0 drivers/usb/core/message.c:2077
usbnet_generic_cdc_bind+0x284/0x1b10 drivers/net/usb/cdc_ether.c:158
usbnet_ether_cdc_bind drivers/net/usb/cdc_ether.c:329
usbnet_cdc_bind+0x2a/0x190 drivers/net/usb/cdc_ether.c:437
usbnet_probe+0xcfd/0x2ba0 drivers/net/usb/usbnet.c:1726
usb_probe_interface+0x351/0x8d0 drivers/usb/core/driver.c:361
really_probe drivers/base/dd.c:413
driver_probe_device+0x610/0xa00 drivers/base/dd.c:557
__device_attach_driver+0x230/0x290 drivers/base/dd.c:653
bus_for_each_drv+0x15e/0x210 drivers/base/bus.c:463
__device_attach+0x269/0x3c0 drivers/base/dd.c:710
device_initial_probe+0x1f/0x30 drivers/base/dd.c:757
bus_probe_device+0x1da/0x280 drivers/base/bus.c:523
device_add+0xcf9/0x1640 drivers/base/core.c:1835
usb_set_configuration+0x1064/0x1890 drivers/usb/core/message.c:1932
generic_probe+0x73/0xe0 drivers/usb/core/generic.c:174
usb_probe_device+0xaf/0xe0 drivers/usb/core/driver.c:266
really_probe drivers/base/dd.c:413
driver_probe_device+0x610/0xa00 drivers/base/dd.c:557
__device_attach_driver+0x230/0x290 drivers/base/dd.c:653
bus_for_each_drv+0x15e/0x210 drivers/base/bus.c:463
__device_attach+0x269/0x3c0 drivers/base/dd.c:710
device_initial_probe+0x1f/0x30 drivers/base/dd.c:757
bus_probe_device+0x1da/0x280 drivers/base/bus.c:523
device_add+0xcf9/0x1640 drivers/base/core.c:1835
usb_new_device+0x7b8/0x1020 drivers/usb/core/hub.c:2457
hub_port_connect drivers/usb/core/hub.c:4903
hub_port_connect_change drivers/usb/core/hub.c:5009
port_event drivers/usb/core/hub.c:5115
hub_event+0x23c8/0x37c0 drivers/usb/core/hub.c:5195
process_one_work+0x9fb/0x1570 kernel/workqueue.c:2119
worker_thread+0x1e4/0x1350 kernel/workqueue.c:2253
kthread+0x324/0x3f0 kernel/kthread.c:231
ret_from_fork+0x25/0x30 arch/x86/entry/entry_64.S:431

Allocated by task 2573:
save_stack_trace+0x1b/0x20 arch/x86/kernel/stacktrace.c:59
save_stack+0x43/0xd0 mm/kasan/kasan.c:447
set_track mm/kasan/kasan.c:459
kasan_kmalloc+0xad/0xe0 mm/kasan/kasan.c:551
__kmalloc+0x81/0x1d0 mm/slub.c:3782
kmalloc ./include/linux/slab.h:498
usb_get_configuration+0x372/0x2a70 drivers/usb/core/config.c:848
usb_enumerate_device drivers/usb/core/hub.c:2290
usb_new_device+0xaae/0x1020 drivers/usb/core/hub.c:2426
hub_port_connect drivers/usb/core/hub.c:4903
hub_port_connect_change drivers/usb/core/hub.c:5009
port_event drivers/usb/core/hub.c:5115
hub_event+0x23c8/0x37c0 drivers/usb/core/hub.c:5195
process_one_work+0x9fb/0x1570 kernel/workqueue.c:2119
worker_thread+0x1e4/0x1350 kernel/workqueue.c:2253
kthread+0x324/0x3f0 kernel/kthread.c:231
ret_from_fork+0x25/0x30 arch/x86/entry/entry_64.S:43

Freed by task 3637:
save_stack_trace+0x1b/0x20 arch/x86/kernel/stacktrace.c:59
save_stack+0x43/0xd0 mm/kasan/kasan.c:447
set_track mm/kasan/kasan.c:459
kasan_slab_free+0x72/0xc0 mm/kasan/kasan.c:524
slab_free_hook mm/slub.c:1390
slab_free_freelist_hook mm/slub.c:1412
slab_free mm/slub.c:2988
kfree+0x96/0x1a0 mm/slub.c:3919
kvfree+0x3b/0x60 mm/util.c:416
__vunmap+0x1de/0x270 mm/vmalloc.c:1542
vfree+0x55/0xe0 mm/vmalloc.c:1606
n_tty_close+0xb7/0x120 drivers/tty/n_tty.c:1864
tty_ldisc_close.isra.0+0x9e/0xd0 drivers/tty/tty_ldisc.c:490
tty_ldisc_kill+0x4a/0xc0 drivers/tty/tty_ldisc.c:639
tty_ldisc_release+0x10b/0x220 drivers/tty/tty_ldisc.c:807
tty_release_struct+0x1f/0x50 drivers/tty/tty_io.c:1571
tty_release+0xe60/0x15f0 drivers/tty/tty_io.c:1744
__fput+0x324/0x7e0 fs/file_table.c:210
____fput+0x1a/0x20 fs/file_table.c:244
task_work_run+0x22e/0x2e0 kernel/task_work.c:112
tracehook_notify_resume ./include/linux/tracehook.h:191
exit_to_usermode_loop+0x1d7/0x210 arch/x86/entry/common.c:162
prepare_exit_to_usermode arch/x86/entry/common.c:197
syscall_return_slowpath+0x2a7/0x310 arch/x86/entry/common.c:266
entry_SYSCALL_64_fastpath+0xa3/0xa5 arch/x86/entry/entry_64.S:238

The buggy address belongs to the object at ffff88006b03e540
which belongs to the cache kmalloc-32 of size 32
The buggy address is located 20 bytes inside of
32-byte region [ffff88006b03e540, ffff88006b03e560)
The buggy address belongs to the page:
page:ffffea0001ac0f80 count:1 mapcount:0 mapping: (null) index:0x0
flags: 0x100000000000100(slab)
raw: 0100000000000100 0000000000000000 0000000000000000 0000000180550055
raw: ffffea0001a92340 0000000200000002 ffff88006c401a00 0000000000000000
page dumped because: kasan: bad access detected

Memory state around the buggy address:
ffff88006b03e400: 00 00 fc fc 00 00 00 fc fc fc fb fb fb fb fc fc
ffff88006b03e480: fb fb fb fb fc fc fb fb fb fb fc fc fb fb fb fb
>ffff88006b03e500: fc fc fb fb fb fb fc fc 00 00 04 fc fc fc fb fb
^
ffff88006b03e580: fb fb fc fc fb fb fb fb fc fc fb fb fb fb fc fc
ffff88006b03e600: fb fb fb fb fc fc fb fb fb fb fc fc fb fb fb fb
==================================================================

Greg Kroah-Hartman

unread,
Sep 21, 2017, 3:31:46 AM9/21/17
to Andrey Konovalov, Jonathan Corbet, Mauro Carvalho Chehab, Jaejoong Kim, USB list, LKML, Dmitry Vyukov, Kostya Serebryany, syzkaller
On Wed, Sep 20, 2017 at 04:45:08PM +0200, Andrey Konovalov wrote:
> Hi!
>
> I've got the following crash while fuzzing the kernel with syzkaller.
>
> On commit ebb2c2437d8008d46796902ff390653822af6cc4 (Sep 18).
>
> It looks like cdc_parse_cdc_header() doesn't validate buflen before
> accessing buffer[1], buffer[2] and so on. The only check present is
> while (buflen > 0).

Ugh, you are right, let me go work on a patch, thanks for the report...

Greg Kroah-Hartman

unread,
Sep 21, 2017, 4:04:20 AM9/21/17
to Andrey Konovalov, Jonathan Corbet, Mauro Carvalho Chehab, Jaejoong Kim, USB list, LKML, Dmitry Vyukov, Kostya Serebryany, syzkaller
Here's a first cut at a fix for this. I think this should solve it, but
it's early and my coffee has not fully kicked in...

thanks,

greg k-h
-----------------

diff --git a/drivers/usb/core/message.c b/drivers/usb/core/message.c
index 4c38ea41ae96..028feaf01aa5 100644
--- a/drivers/usb/core/message.c
+++ b/drivers/usb/core/message.c
@@ -2069,6 +2069,10 @@ int cdc_parse_cdc_header(struct usb_cdc_parsed_header *hdr,
elength = 1;
goto next_desc;
}
+ if ((buflen < elength) || (elength < 2)) {
+ dev_err(&intf->dev, "invalid descriptor buffer length\n");
+ break;
+ }
if (buffer[1] != USB_DT_CS_INTERFACE) {
dev_err(&intf->dev, "skipping garbage\n");
goto next_desc;

Andrey Konovalov

unread,
Sep 21, 2017, 9:51:45 AM9/21/17
to Greg Kroah-Hartman, Jonathan Corbet, Mauro Carvalho Chehab, Jaejoong Kim, USB list, LKML, Dmitry Vyukov, Kostya Serebryany, syzkaller
On Thu, Sep 21, 2017 at 10:04 AM, Greg Kroah-Hartman
<gre...@linuxfoundation.org> wrote:
> On Thu, Sep 21, 2017 at 09:31:54AM +0200, Greg Kroah-Hartman wrote:
>> On Wed, Sep 20, 2017 at 04:45:08PM +0200, Andrey Konovalov wrote:
>> > Hi!
>> >
>> > I've got the following crash while fuzzing the kernel with syzkaller.
>> >
>> > On commit ebb2c2437d8008d46796902ff390653822af6cc4 (Sep 18).
>> >
>> > It looks like cdc_parse_cdc_header() doesn't validate buflen before
>> > accessing buffer[1], buffer[2] and so on. The only check present is
>> > while (buflen > 0).
>>
>> Ugh, you are right, let me go work on a patch, thanks for the report...
>
> Here's a first cut at a fix for this. I think this should solve it, but
> it's early and my coffee has not fully kicked in...
>
> thanks,
>
> greg k-h
> -----------------
>
> diff --git a/drivers/usb/core/message.c b/drivers/usb/core/message.c
> index 4c38ea41ae96..028feaf01aa5 100644
> --- a/drivers/usb/core/message.c
> +++ b/drivers/usb/core/message.c
> @@ -2069,6 +2069,10 @@ int cdc_parse_cdc_header(struct usb_cdc_parsed_header *hdr,
> elength = 1;
> goto next_desc;
> }
> + if ((buflen < elength) || (elength < 2)) {

Hi Greg,

I think this should check (elength < 3), since we access both
buffer[1] and buffer[2] after this check.

Thanks!

> + dev_err(&intf->dev, "invalid descriptor buffer length\n");
> + break;
> + }
> if (buffer[1] != USB_DT_CS_INTERFACE) {
> dev_err(&intf->dev, "skipping garbage\n");
> goto next_desc;
>
> --
> You received this message because you are subscribed to the Google Groups "syzkaller" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to syzkaller+...@googlegroups.com.
> For more options, visit https://groups.google.com/d/optout.

Greg Kroah-Hartman

unread,
Sep 21, 2017, 10:07:23 AM9/21/17
to Andrey Konovalov, Jonathan Corbet, Mauro Carvalho Chehab, Jaejoong Kim, USB list, LKML, Dmitry Vyukov, Kostya Serebryany, syzkaller
{sigh} yes, you are right, counted this one wrong.

With this patch, updated one below, does it fix your crash?

thanks,

greg k-h


diff --git a/drivers/usb/core/message.c b/drivers/usb/core/message.c
index 4c38ea41ae96..028feaf01aa5 100644
--- a/drivers/usb/core/message.c
+++ b/drivers/usb/core/message.c
@@ -2069,6 +2069,10 @@ int cdc_parse_cdc_header(struct usb_cdc_parsed_header *hdr,
elength = 1;
goto next_desc;
}
+ if ((buflen < elength) || (elength < 3)) {

Andrey Konovalov

unread,
Sep 21, 2017, 10:12:38 AM9/21/17
to Greg Kroah-Hartman, Jonathan Corbet, Mauro Carvalho Chehab, Jaejoong Kim, USB list, LKML, Dmitry Vyukov, Kostya Serebryany, syzkaller
On Thu, Sep 21, 2017 at 4:07 PM, Greg Kroah-Hartman
It does, thanks!

Tested-by: Andrey Konovalov <andre...@google.com>
Reply all
Reply to author
Forward
0 new messages