Skip to content

Commit

Permalink
USB: EHCI: avoid undefined pointer arithmetic and placate UBSAN
Browse files Browse the repository at this point in the history
Several people have reported that UBSAN doesn't like the pointer
arithmetic in ehci_hub_control():

	u32 __iomem	*status_reg = &ehci->regs->port_status[
				(wIndex & 0xff) - 1];
	u32 __iomem	*hostpc_reg = &ehci->regs->hostpc[(wIndex & 0xff) - 1];

If wIndex is 0 (and it often is), these calculations underflow and
UBSAN complains.

According to the C standard, pointer computations leading to locations
outside the bounds of an array object (other than 1 position past the
end) are undefined.  In this case, the compiler would be justified in
concluding the wIndex can never be 0 and then optimizing away the
tests for !wIndex that occur later in the subroutine.  (Although,
since ehci->regs->port_status and ehci->regs->hostpc are both 0-length
arrays and are thus GCC extensions to the C standard, it's not clear
what the compiler is really allowed to do.)

At any rate, we can avoid all these difficulties, at the cost of
making the code slightly longer, by not decrementing the index when it
is equal to 0.  The runtime effect is minimal, and anyway
ehci_hub_control() is not on a hot path.

Signed-off-by: Alan Stern <stern@rowland.harvard.edu>
Reported-by: Valdis Kletnieks <Valdis.Kletnieks@vt.edu>
Reported-by: Meelis Roos <mroos@linux.ee>
Reported-by: Martin_MOKREJÅ <mmokrejs@gmail.com>
Reported-by: "Navin P.S" <navinp1912@gmail.com>
CC: Andrey Ryabinin <ryabinin.a.a@gmail.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
  • Loading branch information
Alan Stern authored and Greg Kroah-Hartman committed Jun 1, 2016
1 parent 593224e commit 85e3990
Showing 1 changed file with 11 additions and 3 deletions.
14 changes: 11 additions & 3 deletions drivers/usb/host/ehci-hub.c
Original file line number Diff line number Diff line change
Expand Up @@ -872,14 +872,22 @@ 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 & 0xff) - 1];
u32 __iomem *hostpc_reg = &ehci->regs->hostpc[(wIndex & 0xff) - 1];
u32 __iomem *status_reg, *hostpc_reg;
u32 temp, temp1, status;
unsigned long flags;
int retval = 0;
unsigned selector;

/*
* Avoid underflow while calculating (wIndex & 0xff) - 1.
* The compiler might deduce that wIndex can never be 0 and then
* optimize away the tests for !wIndex below.
*/
temp = wIndex & 0xff;
temp -= (temp > 0);
status_reg = &ehci->regs->port_status[temp];
hostpc_reg = &ehci->regs->hostpc[temp];

/*
* FIXME: support SetPortFeatures USB_PORT_FEAT_INDICATOR.
* HCS_INDICATOR may say we can change LEDs to off/amber/green.
Expand Down

0 comments on commit 85e3990

Please sign in to comment.