On Fri, Apr 27, 2018 at 01:06:49PM +0300, Аладышев Константин wrote:
Ok, I’ve finally got it working!
As I said earlier, this keyboard has interrupt endpoint size = 32 bytes. SeaBIOS doesn’t allow it ("usb_kbd_setup" function checks for “(epdesc->wMaxPacketSize != 8)").
But this fix is not enough, cause “ehci_poll_intr(struct usb_pipe *p, void *data)” use unsafe “memcpy_far” for data with size of “maxpacket”. So if maxpacket is 32, there will be 32 bytes copied. But “usb_check_key” function use “keyevent” structure address as data parameter for “ehci_poll_intr” function. And the size of this structure is only 8 bytes, so this will lead to memory corruption.
If we look at "Device Class Definition for Human Interface Devices (HID)" specification (http://www.usb.org/developers/hidpage/HID1_11.pdf - Appendix B: Boot Interface Descriptors – page 59):
“The HID Subclass 1 defines two descriptors for Boot Devices. Devices may append additional data to these boot reports, but the first 8 bytes of keyboard reports and the first 3 bytes of mouse reports must conform to the format defined by the Boot Report descriptor in order for the data to be correctly interpreted by the BIOS. The report may not exceed 8 bytes in length. The BIOS will ignore any extensions to reports. These descriptors describe reports that the BIOS expects to see.”
This is a little ambiguous for me how “Devices may append additional data” but “The report may not exceed 8 bytes in length” (maybe I misunderstand “may not” in second sentence), but it looks like it is SeaBIOS responsibility to “ignore any extensions to reports”. What do you think?
It sounds like the device is out of spec. But, I don't think there's any harm in supporting it.
I guess we could add a "datasize" parameter to usb_poll_intr() so that only the requested amount of data is transferred.
-Kevin