]> git.kernelconcepts.de Git - karo-tx-linux.git/commitdiff
USB: EHCI: avoid undefined pointer arithmetic and placate UBSAN
authorAlan Stern <stern@rowland.harvard.edu>
Thu, 19 May 2016 20:29:50 +0000 (16:29 -0400)
committerGreg Kroah-Hartman <gregkh@linuxfoundation.org>
Wed, 1 Jun 2016 21:56:24 +0000 (14:56 -0700)
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>
drivers/usb/host/ehci-hub.c

index ffc90295a95f284e7e234f121429b5f957dedc07..74f62d68f01362d144e8de21bd188c06e31a7945 100644 (file)
@@ -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.