From owner-freebsd-bugs@FreeBSD.ORG Sat Feb 21 06:40:14 2004 Return-Path: Delivered-To: freebsd-bugs@hub.freebsd.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id DA48316A4CE for ; Sat, 21 Feb 2004 06:40:14 -0800 (PST) Received: from freefall.freebsd.org (freefall.freebsd.org [216.136.204.21]) by mx1.FreeBSD.org (Postfix) with ESMTP id D134743D1F for ; Sat, 21 Feb 2004 06:40:14 -0800 (PST) (envelope-from gnats@FreeBSD.org) Received: from freefall.freebsd.org (gnats@localhost [127.0.0.1]) i1LEeEbv099258 for ; Sat, 21 Feb 2004 06:40:14 -0800 (PST) (envelope-from gnats@freefall.freebsd.org) Received: (from gnats@localhost) by freefall.freebsd.org (8.12.10/8.12.10/Submit) id i1LEeETR099257; Sat, 21 Feb 2004 06:40:14 -0800 (PST) (envelope-from gnats) Resent-Date: Sat, 21 Feb 2004 06:40:14 -0800 (PST) Resent-Message-Id: <200402211440.i1LEeETR099257@freefall.freebsd.org> Resent-From: FreeBSD-gnats-submit@FreeBSD.org (GNATS Filer) Resent-To: freebsd-bugs@FreeBSD.org Resent-Reply-To: FreeBSD-gnats-submit@FreeBSD.org, Brian Candler Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id C260E16A4CE for ; Sat, 21 Feb 2004 06:35:31 -0800 (PST) Received: from mk-smarthost-3.mail.uk.tiscali.com (mk-smarthost-3.mail.uk.tiscali.com [212.74.114.39]) by mx1.FreeBSD.org (Postfix) with ESMTP id 7086D43D1D for ; Sat, 21 Feb 2004 06:35:31 -0800 (PST) (envelope-from b.candler@pobox.com) Received: from ppp-1-179.lond-a-1.access.uk.tiscali.com ([80.225.197.179]:2051 helo=vaio.linnet.org) by mk-smarthost-3.mail.uk.tiscali.com with esmtp (Exim 4.24) id 1AuYE4-000NDe-9h for FreeBSD-gnats-submit@freebsd.org; Sat, 21 Feb 2004 14:35:28 +0000 Received: from brian by vaio.linnet.org with local (Exim 4.30) id 1AuYE3-00006b-3x for FreeBSD-gnats-submit@freebsd.org; Sat, 21 Feb 2004 14:35:27 +0000 Message-Id: Date: Sat, 21 Feb 2004 14:35:27 +0000 From: Brian Candler To: FreeBSD-gnats-submit@FreeBSD.org X-Send-Pr-Version: 3.113 Subject: kern/63171: USB keyboard rollover error [ukbd.c patch] X-BeenThere: freebsd-bugs@freebsd.org X-Mailman-Version: 2.1.1 Precedence: list Reply-To: Brian Candler List-Id: Bug reports List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 21 Feb 2004 14:40:15 -0000 >Number: 63171 >Category: kern >Synopsis: USB keyboard rollover error [ukbd.c patch] >Confidential: no >Severity: serious >Priority: low >Responsible: freebsd-bugs >State: open >Quarter: >Keywords: >Date-Required: >Class: sw-bug >Submitter-Id: current-users >Arrival-Date: Sat Feb 21 06:40:14 PST 2004 >Closed-Date: >Last-Modified: >Originator: Brian Candler >Release: FreeBSD 4.8-RELEASE i386 >Organization: >Environment: System: 4.8-RELEASE FreeBSD 4.8-RELEASE #5: Sat Feb 21 13:49:32 GMT 2004 root@vaio.linnet.org:/usr/src/sys/compile/VAIO i386 Sony Vaio PCG-C1F USB Keyboard (Packard Bell PB-KB400) >Description: N-key rollover fails to work properly with a USB keyboard, giving duplicate characters. Thisi is a prorbleme when youo a rer typing veryr quiuckcly! A second, unrelated issue is that there is a wrong pointer type being passed to usbd_set_polling, which manifests itself as a compiler warning but could cause memory corruption. The attached patch corrects both problems. >How-To-Repeat: Plug in a USB keyboard. Activate it using #!/bin/sh kbdcontrol -k /dev/kbd1 Fix: The keyboard driver keeps a list of keys held (ks_ndata and ks_odata) and looks for differences between them to identify key presses and releases. However it makes the erroneous assumption that the first zero entry seen indicates the end of the list. In fact, when you press the key sequence above, you get (1) _ _ _ _ _ _ Initial empty list (2) a _ _ _ _ _ "a" pressed (3) a s _ _ _ _ "s" pressed (4) _ s _ _ _ _ "a" released (5) d s _ _ _ _ "d" pressed At least this is true with my USB keyboard; I haven't checked the HID spec, but given that my keyboard works correctly under Windows98 with no rollover problems I think it is entitled to behave in this way. The fix is very straightforward, changing 'break' to 'continue' in a few places. The pointer error looks like an oversight, and the last part of the patch makes what seems to be the obvious correction, although I am not intimate with the USB data structures so this ought to be checked by someone who is. (usbd_set_polling requires a usbd_interface_handle, not a usbd_device_handle, as its first parameter). Regards, Brian Candler. --- sys/dev/usb/ukbd.c-1.46 Sat Feb 21 12:17:38 2004 +++ sys/dev/usb/ukbd.c Sat Feb 21 13:47:56 2004 @@ -43,7 +43,7 @@ __FBSDID("$FreeBSD: /repoman/r/ncvs/src/sys/dev/usb/ukbd.c,v 1.46 2004/01/03 15:01:04 sanpei Exp $"); /* - * HID spec: http://www.usb.org/developers/data/devclass/hid1_1.pdf + * HID spec: http://www.usb.org/developers/devclass_docs/HID1_11.pdf */ #include "opt_kbd.h" @@ -746,10 +746,10 @@ for (i = 0; i < NKEYCODE; i++) { key = state->ks_odata.keycode[i]; if (key == 0) - break; + continue; for (j = 0; j < NKEYCODE; j++) { if (ud->keycode[j] == 0) - break; + continue; if (key == ud->keycode[j]) goto rfound; } @@ -762,11 +762,11 @@ for (i = 0; i < NKEYCODE; i++) { key = ud->keycode[i]; if (key == 0) - break; + continue; state->ks_ntime[i] = now + kbd->kb_delay1; for (j = 0; j < NKEYCODE; j++) { if (state->ks_odata.keycode[j] == 0) - break; + continue; if (key == state->ks_odata.keycode[j]) { state->ks_ntime[i] = state->ks_otime[j]; if (state->ks_otime[j] > now) @@ -1327,21 +1327,19 @@ ukbd_poll(keyboard_t *kbd, int on) { ukbd_state_t *state; - usbd_device_handle dev; int s; state = (ukbd_state_t *)kbd->kb_data; - usbd_interface2device_handle(state->ks_iface, &dev); s = splusb(); if (on) { if (state->ks_polling == 0) - usbd_set_polling(dev, on); + usbd_set_polling(state->ks_iface, on); ++state->ks_polling; } else { --state->ks_polling; if (state->ks_polling == 0) - usbd_set_polling(dev, on); + usbd_set_polling(state->ks_iface, on); } splx(s); return 0; >Release-Note: >Audit-Trail: >Unformatted: