[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAL6iAaJQZ4JB6udi3UFyGM0uYYE4NECZ5fK8ujb+ANfwe9tYDw@mail.gmail.com>
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
Powered by blists - more mailing lists