From patchwork Wed Sep 20 04:57:22 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jaejoong Kim X-Patchwork-Id: 9960783 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 9D425601D5 for ; Wed, 20 Sep 2017 04:57:41 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 8CB9428F72 for ; Wed, 20 Sep 2017 04:57:41 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 815D328F92; Wed, 20 Sep 2017 04:57:41 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.3 required=2.0 tests=BAYES_00, DKIM_ADSP_CUSTOM_MED, DKIM_SIGNED, FREEMAIL_FROM, RCVD_IN_DNSWL_HI, RCVD_IN_SORBS_SPAM, T_DKIM_INVALID autolearn=unavailable version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id BE31028F72 for ; Wed, 20 Sep 2017 04:57:40 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751946AbdITE52 (ORCPT ); Wed, 20 Sep 2017 00:57:28 -0400 Received: from mail-it0-f67.google.com ([209.85.214.67]:35731 "EHLO mail-it0-f67.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751647AbdITE5Y (ORCPT ); Wed, 20 Sep 2017 00:57:24 -0400 Received: by mail-it0-f67.google.com with SMTP id u2so845161itb.2; Tue, 19 Sep 2017 21:57:23 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:in-reply-to:references:from:date:message-id:subject:to :cc; bh=dZfB6VMM9HzBNZlZ8+Y3ojugqTZHCa8awXUWejcnqYA=; b=iNaIlu/Moms712PhaCcoja3xYOFC1vBG5qRaXdDAzsDI7If6ylxuW8OHT6RhUGDkw/ JJYYG3Pzr0f/uHAP3a0xiWQ4tVVbuCL0+6Mm7+CRGKwFVQd5oofJjyJmthDown8FuQyl NnkXE6PyxzK040JusUCdwOrHq0KszIu6ZVf4lUDq5RNXw8dJWSFycBZaU/9r9vArcoO/ wF0s9rcW242YNM4FEG4kqfwIvTd7YCUCxoeawINwZQKtCvDMZXP+P5O+D41df4DG9QtH wedfXTb21VS09RFqOqBdK6OF4V2I4hfl+JVkmpBbKzfnoc9QmJuJyc5Ex6kDUwyXQAU7 05Ew== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc; bh=dZfB6VMM9HzBNZlZ8+Y3ojugqTZHCa8awXUWejcnqYA=; b=rZN/ZTQ7JZ+TBfTfQqN3XbJ/HTMsTOSFHxAuBvMpidTkv6MKFPl+66XVXptRoQVDdk OqOc4nA4hybC+FA8EIWIb9XStFPcd0v6JxjwidypfcYpW3Rw4uaRrk5/l/RBAE/t0+Gt 6HkO4VQRrLEHeHbphgSNeWg2KkOv2xgR0fCKl2vzDFPX1ejk0ZUTyrplyNdC90Gm9Vk2 Z0SHi6AAzS3Q3fHRQL1R5Trzvb77AS7ms28O6q0lofaJkMlkMFB+ZWAG6z9ujjTIO70f UJtgU3817FuD/QZaQVnwQWZMvouqNb4c4HK7Rieh6VWW/mJCQj5X2uzcEWiycRMG4dM4 AxVw== X-Gm-Message-State: AHPjjUhfOMU70X/qGIBS0w3coFVeIkncVcpvR4uoFZrDHG1FSdUhzZpZ 25K3gGcunZdwedul9KOhyo3JHXoXuDI17Pib/JIkRg== X-Google-Smtp-Source: AOwi7QDhFVyVlhKs0M6B2FgCqX7QfvbmBnAzC+MUi44iDYwElpPGN59O3ytqys3KJaNu/RX7ERRTNsANCBN2We8E6xI= X-Received: by 10.36.61.15 with SMTP id n15mr1158345itn.117.1505883442925; Tue, 19 Sep 2017 21:57:22 -0700 (PDT) MIME-Version: 1.0 Received: by 10.79.142.206 with HTTP; Tue, 19 Sep 2017 21:57:22 -0700 (PDT) In-Reply-To: References: From: Kim Jaejoong Date: Wed, 20 Sep 2017 13:57:22 +0900 Message-ID: Subject: Re: usb/hid: slab-out-of-bounds read in usbhid_parse To: Andrey Konovalov Cc: Jiri Kosina , Benjamin Tissoires , USB list , linux-input@vger.kernel.org, LKML , syzkaller , Dmitry Vyukov , Kostya Serebryany Sender: linux-input-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-input@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP Hi Andrey 2017-09-19 21:38 GMT+09:00 Andrey Konovalov : > On Tue, Sep 19, 2017 at 1:47 PM, Kim Jaejoong wrote: >> Hi, Andrey Konovalov >> >> Thanks for the report. >> >> 2017-09-19 2:33 GMT+09:00 Andrey Konovalov : >>> Hi! >>> >>> I've got the following crash while fuzzing the kernel with syzkaller. >>> >>> On commit ebb2c2437d8008d46796902ff390653822af6cc4 (Sep 18). >>> >>> It seems that there's no proper check on the hdesc->bNumDescriptors >>> value in usbhid_parse(). it iterates over hdesc->desc and accesses >>> hdesc->desc[n] fields, which might be out-of-bounds. >> >> The bNumDescriptors in hid descriptor means 'numeric expression >> specifying the number of class descriptors'. >> The value bNumberDescriptors identifies the number of additional class >> specific descriptors present. >> (refer to the 6.1.2 HID Descriptor in hid documents : >> http://www.usb.org/developers/hidpage/HID1_11.pdf) >> >> So, it can be out-of-bounds in hdesc->desc[n] if there is an >> additional class descriptor. >> >> Does the patch below fix this? > > Hi Kim, > > I'm not sure. Is there a check on the bLength field of a > hid_descriptor struct? Can it be less than sizeof(struct > hid_descriptor)? If so, we still do an out-of-bounds access to > hdesc->desc[0] (or some other fields). You are right. I add hid descriptr size from HID device with bLength field. Could you test and review below patch? To. usb & input guys. While dig this report, i was wondering about bNumDescriptors in HID descriptor. HID document from usb.org said, 'this number must be at least one (1) as a Report descriptor will always be present.' There is no mention of the order of class descriptors. Suppose you have a HID device with a report descriptor and a physical descriptor. If you have the following hid descriptor in this case, HID descriptor bLength: 12 bDescriptor Type: HID .. skip bNumDescriptors: 2 bDescriptorType: physical bDescriptorLength: any bDescriptorType: Report bDescriptorLength: any If the order of the report descriptor is the second as above, usbhid_parse () will fail because my patch is only check the first bDescriptor Type. But If the order of the report descriptor is always first, there is no problem. How do you think this? Thanks, jaejoong ----------------- ----------- > > Thanks! > >> >> Thanks, jaejoong >> ------ >> >> diff --git a/drivers/hid/usbhid/hid-core.c b/drivers/hid/usbhid/hid-core.c >> index 089bad8..7b6a0b6 100644 >> --- a/drivers/hid/usbhid/hid-core.c >> +++ b/drivers/hid/usbhid/hid-core.c >> @@ -974,7 +974,7 @@ static int usbhid_parse(struct hid_device *hid) >> u32 quirks = 0; >> unsigned int rsize = 0; >> char *rdesc; >> - int ret, n; >> + int ret; >> >> quirks = usbhid_lookup_quirk(le16_to_cpu(dev->descriptor.idVendor), >> le16_to_cpu(dev->descriptor.idProduct)); >> @@ -1000,9 +1000,8 @@ static int usbhid_parse(struct hid_device *hid) >> hid->version = le16_to_cpu(hdesc->bcdHID); >> hid->country = hdesc->bCountryCode; >> >> - for (n = 0; n < hdesc->bNumDescriptors; n++) >> - if (hdesc->desc[n].bDescriptorType == HID_DT_REPORT) >> - rsize = le16_to_cpu(hdesc->desc[n].wDescriptorLength); >> + if (hdesc->desc[0].bDescriptorType == HID_DT_REPORT) >> + rsize = le16_to_cpu(hdesc->desc[0].wDescriptorLength); >> >> if (!rsize || rsize > HID_MAX_DESCRIPTOR_SIZE) { >> dbg_hid("weird size of report descriptor (%u)\n", rsize); >> >> >>> >>> ================================================================== >>> BUG: KASAN: slab-out-of-bounds in usbhid_parse+0x9b1/0xa20 >>> Read of size 1 at addr ffff88006c5f8edf by task kworker/1:2/1261 >>> >>> CPU: 1 PID: 1261 Comm: kworker/1:2 Not tainted >>> 4.14.0-rc1-42251-gebb2c2437d80 #169 >>> 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+0x292/0x395 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+0x22f/0x340 mm/kasan/report.c:409 >>> __asan_report_load1_noabort+0x19/0x20 mm/kasan/report.c:427 >>> usbhid_parse+0x9b1/0xa20 drivers/hid/usbhid/hid-core.c:1004 >>> hid_add_device+0x16b/0xb30 drivers/hid/hid-core.c:2944 >>> usbhid_probe+0xc28/0x1100 drivers/hid/usbhid/hid-core.c:1369 >>> usb_probe_interface+0x35d/0x8e0 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+0x161/0x210 drivers/base/bus.c:463 >>> __device_attach+0x26e/0x3d0 drivers/base/dd.c:710 >>> device_initial_probe+0x1f/0x30 drivers/base/dd.c:757 >>> bus_probe_device+0x1eb/0x290 drivers/base/bus.c:523 >>> device_add+0xd0b/0x1660 drivers/base/core.c:1835 >>> usb_set_configuration+0x104e/0x1870 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+0x161/0x210 drivers/base/bus.c:463 >>> __device_attach+0x26e/0x3d0 drivers/base/dd.c:710 >>> device_initial_probe+0x1f/0x30 drivers/base/dd.c:757 >>> bus_probe_device+0x1eb/0x290 drivers/base/bus.c:523 >>> device_add+0xd0b/0x1660 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+0x194d/0x3740 drivers/usb/core/hub.c:5195 >>> process_one_work+0xc7f/0x1db0 kernel/workqueue.c:2119 >>> worker_thread+0x221/0x1850 kernel/workqueue.c:2253 >>> kthread+0x3a1/0x470 kernel/kthread.c:231 >>> ret_from_fork+0x2a/0x40 arch/x86/entry/entry_64.S:431 >>> >>> Allocated by task 1261: >>> 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+0x14e/0x310 mm/slub.c:3782 >>> kmalloc ./include/linux/slab.h:498 >>> usb_get_configuration+0x372/0x2a60 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+0x194d/0x3740 drivers/usb/core/hub.c:5195 >>> process_one_work+0xc7f/0x1db0 kernel/workqueue.c:2119 >>> worker_thread+0x221/0x1850 kernel/workqueue.c:2253 >>> kthread+0x3a1/0x470 kernel/kthread.c:231 >>> ret_from_fork+0x2a/0x40 arch/x86/entry/entry_64.S:431 >>> >>> Freed by task 2927: >>> 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+0xf6/0x2f0 mm/slub.c:3919 >>> free_rb_tree_fname+0x8a/0xe0 fs/ext4/dir.c:402 >>> ext4_htree_free_dir_info fs/ext4/dir.c:424 >>> ext4_release_dir+0x49/0x70 fs/ext4/dir.c:622 >>> __fput+0x33e/0x800 fs/file_table.c:210 >>> ____fput+0x1a/0x20 fs/file_table.c:244 >>> task_work_run+0x1af/0x280 kernel/task_work.c:112 >>> tracehook_notify_resume ./include/linux/tracehook.h:191 >>> exit_to_usermode_loop+0x1e1/0x220 arch/x86/entry/common.c:162 >>> prepare_exit_to_usermode arch/x86/entry/common.c:197 >>> syscall_return_slowpath+0x414/0x480 arch/x86/entry/common.c:266 >>> entry_SYSCALL_64_fastpath+0xc0/0xc2 arch/x86/entry/entry_64.S:238 >>> >>> The buggy address belongs to the object at ffff88006c5f8ea0 >>> which belongs to the cache kmalloc-64 of size 64 >>> The buggy address is located 63 bytes inside of >>> 64-byte region [ffff88006c5f8ea0, ffff88006c5f8ee0) >>> The buggy address belongs to the page: >>> page:ffffea0001b17e00 count:1 mapcount:0 mapping: (null) index:0x0 >>> flags: 0x100000000000100(slab) >>> raw: 0100000000000100 0000000000000000 0000000000000000 00000001002a002a >>> raw: ffffea0001a83000 0000001500000015 ffff88006c403800 0000000000000000 >>> page dumped because: kasan: bad access detected >>> >>> Memory state around the buggy address: >>> ffff88006c5f8d80: fb fb fb fb fb fb fb fb fc fc fc fc 00 00 00 00 >>> ffff88006c5f8e00: 00 fc fc fc fc fc fc fc fb fb fb fb fb fb fb fb >>>>ffff88006c5f8e80: fc fc fc fc 00 00 00 00 00 00 00 07 fc fc fc fc >>> ^ >>> ffff88006c5f8f00: fb fb fb fb fb fb fb fb fc fc fc fc fb fb fb fb >>> ffff88006c5f8f80: fb fb fb fb fc fc fc fc fc fc fc fc fc fc fc fc >>> ================================================================== >>> -- >>> To unsubscribe from this list: send the line "unsubscribe linux-input" in >>> the body of a message to majordomo@vger.kernel.org >>> More majordomo info at http://vger.kernel.org/majordomo-info.html >> >> -- >> 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+unsubscribe@googlegroups.com. >> For more options, visit https://groups.google.com/d/optout. Tested-by: Andrey Konovalov --- To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html diff --git a/drivers/hid/usbhid/hid-core.c b/drivers/hid/usbhid/hid-core.c index 089bad8..94c3805 100644 --- a/drivers/hid/usbhid/hid-core.c +++ b/drivers/hid/usbhid/hid-core.c @@ -974,7 +974,7 @@ static int usbhid_parse(struct hid_device *hid) u32 quirks = 0; unsigned int rsize = 0; char *rdesc; - int ret, n; + int ret; quirks = usbhid_lookup_quirk(le16_to_cpu(dev->descriptor.idVendor), le16_to_cpu(dev->descriptor.idProduct)); @@ -997,12 +997,16 @@ static int usbhid_parse(struct hid_device *hid) return -ENODEV; } + if (hdesc->bLength < sizeof(*hdesc)) { + dbg_hid("hid descriptor is too short\n"); + return -EINVAL; + } + hid->version = le16_to_cpu(hdesc->bcdHID); hid->country = hdesc->bCountryCode; - for (n = 0; n < hdesc->bNumDescriptors; n++) - if (hdesc->desc[n].bDescriptorType == HID_DT_REPORT) - rsize = le16_to_cpu(hdesc->desc[n].wDescriptorLength); + if (hdesc->desc[0].bDescriptorType == HID_DT_REPORT) + rsize = le16_to_cpu(hdesc->desc[0].wDescriptorLength); if (!rsize || rsize > HID_MAX_DESCRIPTOR_SIZE) { dbg_hid("weird size of report descriptor (%u)\n", rsize);