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 [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html