On Mon, Mar 18, 2013 at 09:34:51AM +0800, Peter Chen wrote:
> I agree with you. How about below version:
>
> drivers/usb/host/xhci-ring.c | 39 ++++++++++++++++++++-------------------
> 1 files changed, 20 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
> index 8828754..6138af2 100644
> --- a/drivers/usb/host/xhci-ring.c
> +++ b/drivers/usb/host/xhci-ring.c
> @@ -1588,25 +1588,18 @@ static void handle_port_status(struct xhci_hcd *xhci,
> __le32 __iomem **port_array;
> bool bogus_port_status = false;
>
> - /* 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.
> + /* 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",
> @@ -1621,6 +1614,18 @@ static void handle_port_status(struct xhci_hcd *xhci,
> bogus_port_status = true;
> goto cleanup;
> }
> + /* 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;
> + }
> +
> + 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;
> + }
>
> /*
> * Hardware port IDs reported by a Port Status Change Event include USB
> @@ -1629,10 +1634,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;
>
> --
Sarah Sharp
--
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
