On Mon, Mar 18, 2013 at 08:48:15AM -0700, Sarah Sharp wrote:
> >  
> > -   /* Port status change events always have a successful completion code */
> > -   if (GET_COMP_CODE(le32_to_cpu(event->generic.field[2])) != 
> > COMP_SUCCESS) {
> > -           xhci_warn(xhci, "WARN: xHC returned failed port status 
> > event\n");
> > -           xhci->error_bitmask |= 1 << 8;
> > -   }
> > -   port_id = GET_PORT_ID(le32_to_cpu(event->generic.field[0]));
> > -   xhci_dbg(xhci, "Port Status Change Event for port %d\n", port_id);
> > -
> > -   max_ports = HCS_MAX_PORTS(xhci->hcs_params1);
> > -   if ((port_id <= 0) || (port_id > max_ports)) {
> > -           xhci_warn(xhci, "Invalid port id %d\n", port_id);
> > -           bogus_port_status = true;
> > -           goto cleanup;
> > -   }
> > -
> >     /* Figure out which usb_hcd this port is attached to:
> >      * is it a USB 3.0 port or a USB 2.0/1.1 port?
> >      */
> > +   port_id = GET_PORT_ID(le32_to_cpu(event->generic.field[0]));
> > +   xhci_dbg(xhci, "Port Status Change Event for port %d\n", port_id);
> > +
> >     major_revision = xhci->port_array[port_id - 1];
> 
> You moved the check for out-of-range port_id further down in the code,
> and it really needs to be before the line above.  Otherwise the host
> could give us a garbage port number and the kernel will do an
> out-of-bounds array access.
> 

How about below version:

 drivers/usb/host/xhci-ring.c |   14 ++++++++------
 1 files changed, 8 insertions(+), 6 deletions(-)

diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index 8828754..ec26819 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -1599,14 +1599,20 @@ static void handle_port_status(struct xhci_hcd *xhci,
        max_ports = HCS_MAX_PORTS(xhci->hcs_params1);
        if ((port_id <= 0) || (port_id > max_ports)) {
                xhci_warn(xhci, "Invalid port id %d\n", port_id);
-               bogus_port_status = true;
-               goto cleanup;
+               inc_deq(xhci, xhci->event_ring);
+               return;
        }
 
        /* Figure out which usb_hcd this port is attached to:
         * is it a USB 3.0 port or a USB 2.0/1.1 port?
         */
        major_revision = xhci->port_array[port_id - 1];
+
+       /* Find the right roothub. */
+       hcd = xhci_to_hcd(xhci);
+       if ((major_revision == 0x03) != (hcd->speed == HCD_USB3))
+               hcd = xhci->shared_hcd;
+
        if (major_revision == 0) {
                xhci_warn(xhci, "Event for port %u not in "
                                "Extended Capabilities, ignoring.\n",
@@ -1629,10 +1635,6 @@ static void handle_port_status(struct xhci_hcd *xhci,
         * into the index into the ports on the correct split roothub, and the
         * correct bus_state structure.
         */
-       /* Find the right roothub. */
-       hcd = xhci_to_hcd(xhci);
-       if ((major_revision == 0x03) != (hcd->speed == HCD_USB3))
-               hcd = xhci->shared_hcd;
        bus_state = &xhci->bus_state[hcd_index(hcd)];
        if (hcd->speed == HCD_USB3)
                port_array = xhci->usb3_ports;

-- 

Best Regards,
Peter Chen

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to