Dear Matt,
On 2019-12-13 04:51, 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, zeroizing
> it before use, and using the key array size of the usbkeyinfo
This could have been a separate commit.
> 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
>
> Signed-off-by: Matt DeVillier <matt.devill...@gmail.com>
> ---
> src/hw/usb-hid.c | 17 ++++++++++++-----
> 1 file changed, 12 insertions(+), 5 deletions(-)
>
> diff --git a/src/hw/usb-hid.c b/src/hw/usb-hid.c
> index fa4d9a2..cedec0b 100644
> --- a/src/hw/usb-hid.c
> +++ b/src/hw/usb-hid.c
> @@ -8,6 +8,7 @@
> #include "config.h" // CONFIG_*
> #include "output.h" // dprintf
> #include "ps2port.h" // ATKBD_CMD_GETID
> +#include <string.h> //memset
> #include "usb.h" // usb_ctrlrequest
> #include "usb-hid.h" // usb_keyboard_setup
> #include "util.h" // process_key
> @@ -59,7 +60,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 > 10)
A debug message would be nice, so it’s easier to spot in the future.
Could be a follow-up commit though.
> return -1;
>
> // Enable "boot" protocol.
> @@ -163,11 +164,15 @@ static u16 ModifierToScanCode[] VAR16 = {
>
> #define RELEASEBIT 0x80
>
> -// Format of USB keyboard event data
> +// Format of USB keyboard event data.
> +// Some keyboards use a 9/10 byte packet size,
> +// so account for that here to prevent buffer
> +// overflow. We'll ignore the 9th/10th bytes
> +// as it's out of spec.
> struct keyevent {
> u8 modifiers;
> u8 reserved;
> - u8 keys[6];
> + u8 keys[8];
> };
>
> // Translate data from KeyToScanCode[] to calls to process_key().
> @@ -253,7 +258,7 @@ handle_key(struct keyevent *data)
> break;
> int j;
> for (j=0;; j++) {
> - if (j>=ARRAY_SIZE(data->keys)) {
> + if (j>=ARRAY_SIZE(old.keys)) {
> // Key released.
> procscankey(key, RELEASEBIT, data->modifiers);
> if (i+1 >= ARRAY_SIZE(old.keys) || !old.keys[i+1])
> @@ -274,7 +279,7 @@ handle_key(struct keyevent *data)
> // Process new keys
> procmodkey(data->modifiers & ~old.modifiers, 0);
> old.modifiers = data->modifiers;
> - for (i=0; i<ARRAY_SIZE(data->keys); i++) {
> + for (i=0; i<ARRAY_SIZE(old.keys); i++) {
> u8 key = data->keys[i];
> if (!key)
> continue;
> @@ -310,6 +315,8 @@ usb_check_key(void)
>
> for (;;) {
> struct keyevent data;
> + //zeroize struct as most keyboards won't fill it
Please add a space after the comment characters.
> + memset(&data, 0, sizeof(data));
> int ret = usb_poll_intr(pipe, &data);
> if (ret)
> break;
Kind regards,
Paul
smime.p7s
Description: S/MIME Cryptographic Signature
_______________________________________________ SeaBIOS mailing list -- seabios@seabios.org To unsubscribe send an email to seabios-le...@seabios.org
