• Christian Krause's avatar
    [PATCH] USB: fix bug in handling of highspeed usb HID devices · 13b58ee5
    Christian Krause authored
    During the development of an USB device I found a bug in the handling of
    Highspeed HID devices in the kernel.
    
    What happened?
    
    Highspeed HID devices are correctly recognized and enumerated by the
    kernel. But even if usbhid kernel module is loaded, no HID reports are
    received by the kernel.
    
    The output of the hardware USB analyzer told me that the host doesn't
    even poll for interrupt IN transfers (even the "interrupt in" USB
    transfer are polled by the host).
    
    After some debugging in hid-core.c I've found the reason.
    
    In case of a highspeed device, the endpoint interval is re-calculated in
    driver/usb/input/hid-core.c:
    
    line 1669:
                 /* handle potential highspeed HID correctly */
                 interval = endpoint->bInterval;
                 if (dev->speed == USB_SPEED_HIGH)
                       interval = 1 << (interval - 1);
    
    Basically this calculation is correct (refer to USB 2.0 spec, 9.6.6).
    This new calculated value of "interval" is used as input for
    usb_fill_int_urb:
    
    line 1685:
    
                usb_fill_int_urb(hid->urbin, dev, pipe, hid->inbuf, 0,
                       hid_irq_in, hid, interval);
    
    Unfortunately the same calculation as above is done a second time in
    usb_fill_int_urb in the file include/linux/usb.h:
    
    line 933:
            if (dev->speed == USB_SPEED_HIGH)
                    urb->interval = 1 << (interval - 1);
            else
                    urb->interval = interval;
    
    This means, that if the endpoint descriptor (of a high speed device)
    specifies e.g. bInterval = 7, the urb->interval gets the value:
    
    hid-core.c: interval = 1 << (7-1) = 0x40 = 64
    urb->interval = 1 << (interval -1) = 1 << (63) = integer overflow
    
    Because of this the value of urb->interval is sometimes negative and is
    rejected in core/urb.c:
    line 353:
                    /* too small? */
                    if (urb->interval <= 0)
                            return -EINVAL;
    
    The conclusion is, that the recalculaton of the interval (which is
    necessary for highspeed) should not be made twice, because this is
    simply wrong. ;-)
    
    Re-calculation in usb_fill_int_urb makes more sense, because it is the
    most general approach. So it would make sense to remove it from
    hid-core.c.
    
    Because in hid-core.c the interval variable is only used for calling
    usb_fill_int_urb, it is no problem to remove the highspeed
    re-calculation in this file.
    Signed-off-by: default avatarChristian Krause <chkr@plauener.de>
    Signed-off-by: default avatarGreg Kroah-Hartman <gregkh@suse.de>
    Signed-off-by: default avatarLinus Torvalds <torvalds@osdl.org>
    13b58ee5
hid-core.c 53.5 KB