From 633dae9f58e9e87eda3e3158b8c3f9d3e060a6c5 Mon Sep 17 00:00:00 2001 From: miod Date: Sun, 1 Aug 2010 21:37:08 +0000 Subject: [PATCH] Be more generous when parsing the report descriptor: - parts of the report descriptor not in a format we expect are now ignored, instead of preventing attachment (e.g. hypothetical multi-bit modifiers). - modifiers beyond MAXMOD are ignored. - keycode arrays larger than MAXKEYCODE are clamped to MAXKEYCODE instead of being rejected. - multiple keycode arrays are ignored. This should allow rogue keyboards to attach and be usable up to a certain extent. Adapted from a diff sent by Loganaden Velvindron (first name at gmail), who has a keyboard which keycode array is larger than MAXKEYCODE (but, like most if not all USB keyboards out there, can only report up to three simultaneous keypresses anyway). --- sys/dev/usb/hidkbd.c | 56 +++++++++++++++++++++++++++++--------------- 1 file changed, 37 insertions(+), 19 deletions(-) diff --git a/sys/dev/usb/hidkbd.c b/sys/dev/usb/hidkbd.c index 83433eeca9c..524900cea3b 100644 --- a/sys/dev/usb/hidkbd.c +++ b/sys/dev/usb/hidkbd.c @@ -1,4 +1,4 @@ -/* $OpenBSD: hidkbd.c,v 1.2 2010/07/31 16:12:37 miod Exp $ */ +/* $OpenBSD: hidkbd.c,v 1.3 2010/08/01 21:37:08 miod Exp $ */ /* $NetBSD: ukbd.c,v 1.85 2003/03/11 16:44:00 augustss Exp $ */ /* @@ -221,7 +221,7 @@ hidkbd_detach(struct hidkbd *kbd, int flags) { int rv = 0; - DPRINTF(("hidkbd_detach: sc=%p flags=%d\n", sc, flags)); + DPRINTF(("hidkbd_detach: sc=%p flags=%d\n", kbd->sc_device, flags)); #ifdef WSDISPLAY_COMPAT_RAWKBD timeout_del(&kbd->sc_rawrepeat_ch); @@ -336,7 +336,7 @@ hidkbd_decode(struct hidkbd *kbd, struct hidkbd_data *ud) */ if (hidkbdtrace) { struct hidkbdtraceinfo *p = &hidkbdtracedata[hidkbdtraceindex]; - p->unit = kbd->sc_hdev.sc_dev.dv_unit; + p->unit = kbd->sc_device->dv_unit; microtime(&p->tv); p->ud = *ud; if (++hidkbdtraceindex >= HIDKBDTRACESIZE) @@ -597,36 +597,54 @@ hidkbd_parse_desc(struct hidkbd *kbd, int id, void *desc, int dlen) continue; DPRINTF(("hidkbd: imod=%d usage=0x%x flags=0x%x pos=%d size=%d " - "cnt=%d\n", imod, + "cnt=%d", imod, h.usage, h.flags, h.loc.pos, h.loc.size, h.loc.count)); if (h.flags & HIO_VARIABLE) { - if (h.loc.size != 1) - return ("bad modifier size"); - /* Single item */ + /* modifier reports should be one bit each */ + if (h.loc.size != 1) { + DPRINTF((": bad modifier size\n")); + continue; + } + /* single item */ if (imod < MAXMOD) { kbd->sc_modloc[imod] = h.loc; kbd->sc_mods[imod].mask = 1 << imod; kbd->sc_mods[imod].key = HID_GET_USAGE(h.usage); imod++; - } else - return ("too many modifier keys"); + } else { + /* ignore extra modifiers */ + DPRINTF((": too many modifier keys\n")); + } } else { - /* Array */ - if (h.loc.size != 8) - return ("key code size != 8"); - if (h.loc.count > MAXKEYCODE) - return ("too many key codes"); - if (h.loc.pos % 8 != 0) - return ("key codes not on byte boundary"); - if (kbd->sc_nkeycode != 0) - return ("multiple key code arrays\n"); + /* keys array should be in bytes, on a byte boundary */ + if (h.loc.size != 8) { + DPRINTF((": key code size != 8\n")); + continue; + } + if (h.loc.pos % 8 != 0) { + DPRINTF((": array not on byte boundary")); + continue; + } + if (kbd->sc_nkeycode != 0) { + DPRINTF((": ignoring multiple arrays\n")); + continue; + } kbd->sc_keycodeloc = h.loc; - kbd->sc_nkeycode = h.loc.count; + if (h.loc.count > MAXKEYCODE) { + DPRINTF((": ignoring extra key codes")); + kbd->sc_nkeycode = MAXKEYCODE; + } else + kbd->sc_nkeycode = h.loc.count; } + DPRINTF(("\n")); } kbd->sc_nmod = imod; hid_end_parse(d); + /* don't attach if no keys... */ + if (kbd->sc_nkeycode == 0) + return "no usable key codes array"; + hid_locate(desc, dlen, HID_USAGE2(HUP_LEDS, HUD_LED_NUM_LOCK), id, hid_output, &kbd->sc_numloc, NULL); hid_locate(desc, dlen, HID_USAGE2(HUP_LEDS, HUD_LED_CAPS_LOCK), -- 2.20.1