[SeaBIOS] Re: [PATCH v3] hw/usb-hid: handle devices with illegal max packet size

[Kevin O'Connor]

On Sat, Dec 14, 2019 at 11:26:17AM -0600, Matt DeVillier wrote:
> Some USB keyboards report 9 or 10-byte max packet sizes, instead
> of the 8-byte max specified by the USB HID spec. Handle this by
> increasing the size of the keyevent struct to 10 bytes, and using the key
> array size of the usbkeyinfo struct as loop bounds rather than that of the
> keyevent struct (since the former will always be smaller, and within spec).
> 
> Test: built/boot on Google Pixel Slate, observe keyboard functional

Thanks.  I think it would be good if the relationship between the test
and the struct sizes was more clear.  What about the below (totally
untested)?

-Kevin


--- a/src/hw/usb-hid.c
+++ b/src/hw/usb-hid.c
@@ -49,6 +49,15 @@ set_idle(struct usb_pipe *pipe, int ms)
 #define KEYREPEATWAITMS 500
 #define KEYREPEATMS 33
 
+// Format of USB keyboard event data
+struct keyevent {
+    u8 modifiers;
+    u8 reserved;
+    u8 keys[6];
+};
+
+#define MAX_KBD_EVENT 10
+
 static int
 usb_kbd_setup(struct usbdevice_s *usbdev
               , struct usb_endpoint_descriptor *epdesc)
@@ -59,8 +68,12 @@ 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 < sizeof(struct keyevent)
+        || epdesc->wMaxPacketSize > MAX_KBD_EVENT) {
+        dprintf(1, "USB keyboard wMaxPacketSize=%d; aborting\n"
+                , epdesc->wMaxPacketSize);
         return -1;
+    }
 
     // Enable "boot" protocol.
     int ret = set_protocol(usbdev->defpipe, 0);
@@ -79,6 +92,14 @@ usb_kbd_setup(struct usbdevice_s *usbdev
     return 0;
 }
 
+// Format of USB mouse event data
+struct mouseevent {
+    u8 buttons;
+    u8 x, y;
+};
+
+#define MAX_MOUSE_EVENT 8
+
 static int
 usb_mouse_setup(struct usbdevice_s *usbdev
                 , struct usb_endpoint_descriptor *epdesc)
@@ -89,8 +110,12 @@ usb_mouse_setup(struct usbdevice_s *usbdev
         // XXX - this enables the first found mouse (could be random)
         return -1;
 
-    if (epdesc->wMaxPacketSize < 3 || epdesc->wMaxPacketSize > 8)
+    if (epdesc->wMaxPacketSize < sizeof(struct mouseevent)
+        || epdesc->wMaxPacketSize > MAX_MOUSE_EVENT) {
+        dprintf(1, "USB mouse wMaxPacketSize=%d; aborting\n"
+                , epdesc->wMaxPacketSize);
         return -1;
+    }
 
     // Enable "boot" protocol.
     int ret = set_protocol(usbdev->defpipe, 0);
@@ -163,13 +188,6 @@ static u16 ModifierToScanCode[] VAR16 = {
 
 #define RELEASEBIT 0x80
 
-// Format of USB keyboard event data
-struct keyevent {
-    u8 modifiers;
-    u8 reserved;
-    u8 keys[6];
-};
-
 // Translate data from KeyToScanCode[] to calls to process_key().
 static void
 prockeys(u16 scancode, u8 key_release, u8 mods)
@@ -309,11 +327,11 @@ usb_check_key(void)
         return;
 
     for (;;) {
-        struct keyevent data;
-        int ret = usb_poll_intr(pipe, &data);
+        u8 data[MAX_KBD_EVENT];
+        int ret = usb_poll_intr(pipe, data);
         if (ret)
             break;
-        handle_key(&data);
+        handle_key((void*)data);
     }
 }
 
@@ -349,13 +367,6 @@ usb_kbd_command(int command, u8 *param)
  * Mouse events
  ****************************************************************/
 
-// Format of USB mouse event data
-struct mouseevent {
-    u8 buttons;
-    u8 x, y;
-    u8 reserved[5];
-};
-
 // Process USB mouse data.
 static void
 handle_mouse(struct mouseevent *data)
@@ -381,11 +392,11 @@ usb_check_mouse(void)
         return;
 
     for (;;) {
-        struct mouseevent data;
-        int ret = usb_poll_intr(pipe, &data);
+        u8 data[MAX_MOUSE_EVENT];
+        int ret = usb_poll_intr(pipe, data);
         if (ret)
             break;
-        handle_mouse(&data);
+        handle_mouse((void*)data);
     }
 }
 
_______________________________________________
SeaBIOS mailing list -- seabios@seabios.org
To unsubscribe send an email to seabios-le...@seabios.org