linux-kernel - Re: [PATCH] HID: usbhid: fix out-of-bounds bug

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Thu, 28 Sep 2017 17:44:22 +0900
From:   Jaejoong Kim <climbbb.kim@...il.com>
To:     Alan Stern <stern@...land.harvard.edu>
Cc:     Michel Hermier <michel.hermier@...il.com>,
        Kostya Serebryany <kcc@...gle.com>,
        syzkaller <syzkaller@...glegroups.com>,
        Jiri Kosina <jikos@...nel.org>,
        USB list <linux-usb@...r.kernel.org>,
        Andrey Konovalov <andreyknvl@...gle.com>,
        Benjamin Tissoires <benjamin.tissoires@...hat.com>,
        Dmitry Vyukov <dvyukov@...gle.com>,
        linux-input@...r.kernel.org, LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] HID: usbhid: fix out-of-bounds bug

2017-09-27 23:29 GMT+09:00 Alan Stern <stern@...land.harvard.edu>:
> On Wed, 27 Sep 2017, Michel Hermier wrote:
>
>> Le 27 sept. 2017 07:42, "Alan Stern" <stern@...land.harvard.edu> a écrit :
>
>> > -       for (n = 0; n < hdesc->bNumDescriptors; n++)
>> > +       num_descriptors = min_t(int, hdesc->bNumDescriptors,
>> > +                               (hdesc->bLength - 6) / 3);
>> > +       for (n = 0; n < num_descriptors; n++)
>> >                 if (hdesc->desc[n].bDescriptorType == HID_DT_REPORT)
>> >                         rsize = le16_to_cpu(hdesc->desc[n].
>> wDescriptorLength);
>>
>> Yes, this is a lot better.

OK.

>>
>>
>> Is it possible to explicit the magic number 6 and 3 in the code. Currently,
>> it looks like it comes from no where.

I gree with you.

>
> Yes, it is possible.  The 6 is equal to
>
>         offsetof(struct hid_descriptor, desc)
>
> and the 3 is equal to
>
>         sizeof(struct hid_class_descriptor)
>
> (at least, I think it is -- the structure is marked as packed so its
> size should be 3).
>
> In this case I found the numbers to be more readable, but other people
> may have different opinions.

I will post V2 shortly.

>
>> I'm also wondering if this change will not affect some devices in the wild,
>> by rejecting hid descriptors with num descriptors == 0 ?
>
> It's possible, but I doubt it.  If such devices do exist, they should
> never have worked in the first place.  Certainly they would generate
> warnings or errors during enumeration because of their invalid
> descriptors.
>
> Alan Stern
>

Jaejoong