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

[Jaejoong Kim]

2017-09-27 23:29 GMT+09:00 Alan Stern <stern@xxxxxxxxxxxxxxxxxxx>:
> On Wed, 27 Sep 2017, Michel Hermier wrote:
>
>> Le 27 sept. 2017 07:42, "Alan Stern" <stern@xxxxxxxxxxxxxxxxxxx> 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