Sorry for late response.

Ok, I see. I'm trying to understand the code, please correct me where I
am wrong. (It's my first time of some low level USB programming.)

So there are two similar structures, `struct keyevent` and `struct
usbkeyinfo` (if omit `u64 data` field in the latter):
{
    u8 modifiers;
    u8 reserved;
    u8 keys[6];
};

I guess they represent data that is being memcpy'ed from `pipe->buf` in
`usb_poll_intr` function. These structures are 8 bytes in size.

Earlier all keyboards with wMaxPacketSize != 8 had been ignored, so
there was no overflow, because this 8 bytes limit was sort of hardcoded.
Now when we allow 10 bytes packets, I guess `u8 keys[6]` should become
`u8 keys[8]` to allocate enough memory. I guess we also need to memset
the structure to zero before copying to it.

Is that right? If so then I'll send corrected patch.

On 6/10/19 4:07 PM, Kevin O'Connor wrote:
> On Sun, Jun 09, 2019 at 06:52:34PM +0300, Evgeny Zinoviev wrote:
>> So here's the patch for MacBooks, I hope it doesn't break anything.
> Unfortunately, the call to usb_poll_intr() in usb_check_key() will
> memcpy maxPacketSize bytes.  That would overflow 'struct usbkeyinfo'.
>
> -Kevin
>
>
>> On 6/2/19 9:01 PM, Kevin O'Connor wrote:
>>> On Sun, Jun 02, 2019 at 05:39:11PM +0300, Evgeny Zinoviev wrote:
>>>> Hi folks.
>>>>
>>>> I've recently ported coreboot on MBA 5,2 (13'' mid 2012 model) and MBP
>>>> 10,1 (15'' mid 2012 retina model). The integrated keyboard on these
>>>> models is connected as a USB device, not PS/2. I have tested GRUB,
>>>> SeaBIOS and Tianocore payloads, and the keyboard works in GRUB and
>>>> Tianocore but not in SeaBIOS.
>>>>
>>>> I was advised to send a log to this mailing list. It was created with
>>>> SEABIOS_DEBUG_LEVEL=9. I'm not yet familiar with SeaBIOS code, but with
>>>> your help perhaps I can debug and fix this.
>>>>
>>> At a quick glance, it appears multiple USB keyboards/mice were found.
>>> SeaBIOS just chooses one randomly in that case.  As a guess, it chose
>>> the wrong one.  If you want to try and debug it, you could look at
>>> src/hw/usb-hid.c:usb_kbd_setup().
>>>
>>> -Kevin
>>> _______________________________________________
>>> SeaBIOS mailing list -- seabios@seabios.org
>>> To unsubscribe send an email to seabios-le...@seabios.org
>> diff --git a/src/hw/usb-hid.c b/src/hw/usb-hid.c
>> index fa4d9a2..7d3561c 100644
>> --- a/src/hw/usb-hid.c
>> +++ b/src/hw/usb-hid.c
>> @@ -59,7 +59,7 @@ usb_kbd_setup(struct usbdevice_s *usbdev
>>          // XXX - this enables the first found keyboard (could be random)
>>          return -1;
>>  
>> -    if (epdesc->wMaxPacketSize != 8)
>> +    if (epdesc->wMaxPacketSize != 8 && epdesc->wMaxPacketSize != 10)
>>          return -1;
>>  
>>      // Enable "boot" protocol.
>> _______________________________________________
>> SeaBIOS mailing list -- seabios@seabios.org
>> To unsubscribe send an email to seabios-le...@seabios.org
> _______________________________________________
> SeaBIOS mailing list -- seabios@seabios.org
> To unsubscribe send an email to seabios-le...@seabios.org
_______________________________________________
SeaBIOS mailing list -- seabios@seabios.org
To unsubscribe send an email to seabios-le...@seabios.org

Reply via email to