On Sun, 10 Jun 2007, Craig W. Nadler wrote:

> From: Craig W. Nadler <[EMAIL PROTECTED]> 
> 
> EHCI_HUB_CTRL: Fixes a bug with a pointer to a port_status register.
> 
> The ehci_hub_control() function in driver/usb/host/ehci-hub.c takes a
> u16 parameter called wIndex. The least significant byte of this
> parameter holds the port number relevant to the function call. The other
> byte in wIndex is sometimes used to hold another value. A pointer to the
> port_status register for the relevant port was being set using wIndex as
> a subscript to an array without masking off the other byte. This patch
> adds a local u8 variable called port_num which is set to wIndex&0xFF.
> The pointer for the port_status register is then set using port_num as
> the subscript. 
> 
> 
> Signed-off-by: Craig W. Nadler <[EMAIL PROTECTED]> 
> 
> ---
> 
> diff -uprN a/drivers/usb/host/ehci-hub.c b/drivers/usb/host/ehci-hub.c
> --- a/drivers/usb/host/ehci-hub.c 2007-06-07 17:27:31.000000000 -0400
> +++ b/drivers/usb/host/ehci-hub.c 2007-06-10 13:45:14.000000000 -0400
> @@ -444,7 +444,8 @@ static int ehci_hub_control (
> ) {
> struct ehci_hcd *ehci = hcd_to_ehci (hcd);
> int ports = HCS_N_PORTS (ehci->hcs_params);
> - u32 __iomem *status_reg = &ehci->regs->port_status[wIndex - 1];
> + u8 port_num = wIndex&0xFF;
> + u32 __iomem *status_reg = &ehci->regs->port_status[port_num - 1];
> u32 temp, status;
> unsigned long flags;
> int retval = 0;


I have some comments on the contents of this patch -- others may
comment on its formatting (or lack thereof!).

First, there's no reason to make port_num u8.  Compilers and CPUs work
better with word-sized types; make it unsigned.

Second, you didn't fix enough.  Although the indexing is now correct, 
the code in the ClearPortFeature and related cases still checks the 
value of wIndex, not the value of port_num.

Third, exactly the same bug is present in all of the HCDs.  Why change
only ehci-hcd?  Please fix all of them.  And don't forget
drivers/usb/gadget/dummy-hcd.c!

Alan Stern


-------------------------------------------------------------------------
This SF.net email is sponsored by DB2 Express
Download DB2 Express C - the FREE version of DB2 express and take
control of your XML. No limits. Just data. Click to get it now.
http://sourceforge.net/powerbar/db2/
_______________________________________________
linux-usb-devel@lists.sourceforge.net
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel

Reply via email to