From 8125184178de05d762e39ee07f44ada6006e87bd Mon Sep 17 00:00:00 2001 From: Gerd Hoffmann Date: Tue, 8 Jan 2013 13:06:16 +0100 Subject: [PATCH 1/6] xhci: create xhci_detach_slot helper function Signed-off-by: Gerd Hoffmann --- hw/usb/hcd-xhci.c | 24 ++++++++++++++++++------ 1 file changed, 18 insertions(+), 6 deletions(-) diff --git a/hw/usb/hcd-xhci.c b/hw/usb/hcd-xhci.c index 92f2eee3b..3ff8bc1f8 100644 --- a/hw/usb/hcd-xhci.c +++ b/hw/usb/hcd-xhci.c @@ -2198,6 +2198,23 @@ static unsigned int xhci_get_slot(XHCIState *xhci, XHCIEvent *event, XHCITRB *tr return slotid; } +/* cleanup slot state on usb device detach */ +static void xhci_detach_slot(XHCIState *xhci, USBPort *uport) +{ + int slot; + + for (slot = 0; slot < xhci->numslots; slot++) { + if (xhci->slots[slot].uport == uport) { + break; + } + } + if (slot == xhci->numslots) { + return; + } + + xhci->slots[slot].uport = NULL; +} + static TRBCCode xhci_get_port_bandwidth(XHCIState *xhci, uint64_t pctx) { dma_addr_t ctx; @@ -2971,13 +2988,8 @@ static void xhci_child_detach(USBPort *uport, USBDevice *child) { USBBus *bus = usb_bus_from_device(child); XHCIState *xhci = container_of(bus, XHCIState, bus); - int i; - for (i = 0; i < xhci->numslots; i++) { - if (xhci->slots[i].uport == uport) { - xhci->slots[i].uport = NULL; - } - } + xhci_detach_slot(xhci, uport); } static USBPortOps xhci_uport_ops = { From f3dcf6384cc94b6a688f3a366c20642f36247b68 Mon Sep 17 00:00:00 2001 From: Gerd Hoffmann Date: Tue, 8 Jan 2013 13:06:57 +0100 Subject: [PATCH 2/6] xhci: call xhci_detach_slot on root port detach too Signed-off-by: Gerd Hoffmann --- hw/usb/hcd-xhci.c | 1 + 1 file changed, 1 insertion(+) diff --git a/hw/usb/hcd-xhci.c b/hw/usb/hcd-xhci.c index 3ff8bc1f8..5b2e7f89e 100644 --- a/hw/usb/hcd-xhci.c +++ b/hw/usb/hcd-xhci.c @@ -2957,6 +2957,7 @@ static void xhci_detach(USBPort *usbport) XHCIState *xhci = usbport->opaque; XHCIPort *port = xhci_lookup_port(xhci, usbport); + xhci_detach_slot(xhci, usbport); xhci_port_update(port, 1); } From 0cb41e2c5ebc1f8fa180a1726981416fee9abad1 Mon Sep 17 00:00:00 2001 From: Gerd Hoffmann Date: Tue, 8 Jan 2013 14:06:51 +0100 Subject: [PATCH 3/6] xhci: nuke transfe5rs on detach Signed-off-by: Gerd Hoffmann --- hw/usb/hcd-xhci.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/hw/usb/hcd-xhci.c b/hw/usb/hcd-xhci.c index 5b2e7f89e..5fb0c488e 100644 --- a/hw/usb/hcd-xhci.c +++ b/hw/usb/hcd-xhci.c @@ -1197,6 +1197,7 @@ static int xhci_ep_nuke_xfers(XHCIState *xhci, unsigned int slotid, ep = epctx->transfers[xferi].packet.ep; } killed += xhci_ep_nuke_one_xfer(&epctx->transfers[xferi]); + epctx->transfers[xferi].packet.ep = NULL; xferi = (xferi + 1) % TD_QUEUE; } if (ep) { @@ -2201,7 +2202,7 @@ static unsigned int xhci_get_slot(XHCIState *xhci, XHCIEvent *event, XHCITRB *tr /* cleanup slot state on usb device detach */ static void xhci_detach_slot(XHCIState *xhci, USBPort *uport) { - int slot; + int slot, ep; for (slot = 0; slot < xhci->numslots; slot++) { if (xhci->slots[slot].uport == uport) { @@ -2212,6 +2213,11 @@ static void xhci_detach_slot(XHCIState *xhci, USBPort *uport) return; } + for (ep = 0; ep < 31; ep++) { + if (xhci->slots[slot].eps[ep]) { + xhci_ep_nuke_xfers(xhci, slot+1, ep+1); + } + } xhci->slots[slot].uport = NULL; } From cc8d2b65c7e5f44172bf3ec300407522162e9a7f Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Thu, 10 Jan 2013 14:33:23 +0100 Subject: [PATCH 4/6] ehci: Assert state machine is sane w.r.t. EHCIQueue Coverity worries the EHCIQueue pointer could be null when we pass it to functions that reference it. The state machine ensures it can't be null then. Assert that, to hush the checker. Signed-off-by: Markus Armbruster Signed-off-by: Gerd Hoffmann --- hw/usb/hcd-ehci.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/hw/usb/hcd-ehci.c b/hw/usb/hcd-ehci.c index 320b7e723..70406592e 100644 --- a/hw/usb/hcd-ehci.c +++ b/hw/usb/hcd-ehci.c @@ -2092,18 +2092,22 @@ static void ehci_advance_state(EHCIState *ehci, int async) break; case EST_ADVANCEQUEUE: + assert(q != NULL); again = ehci_state_advqueue(q); break; case EST_FETCHQTD: + assert(q != NULL); again = ehci_state_fetchqtd(q); break; case EST_HORIZONTALQH: + assert(q != NULL); again = ehci_state_horizqh(q); break; case EST_EXECUTE: + assert(q != NULL); again = ehci_state_execute(q); if (async) { ehci->async_stepdown = 0; From 4663530898a15944706d51b523d1f1545e32e46a Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Thu, 10 Jan 2013 14:33:24 +0100 Subject: [PATCH 5/6] usb-host: Drop superfluous null test from usb_host_auto_scan() Coverity points out that port is later passed to usb_host_open(), which dereferences it. It actually can't be null: it always points to usb_host_scan()'s auto port[]. Drop the superfluous port == NULL test. Signed-off-by: Markus Armbruster Signed-off-by: Gerd Hoffmann --- hw/usb/host-linux.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/usb/host-linux.c b/hw/usb/host-linux.c index e8e6a42fb..a498840e0 100644 --- a/hw/usb/host-linux.c +++ b/hw/usb/host-linux.c @@ -1760,7 +1760,7 @@ static int usb_host_auto_scan(void *opaque, int bus_num, if (f->addr > 0 && f->addr != addr) { continue; } - if (f->port != NULL && (port == NULL || strcmp(f->port, port) != 0)) { + if (f->port != NULL && strcmp(f->port, port) != 0) { continue; } From 036078475427f2562c8e505f6bb44dbf5d8cbd95 Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Thu, 10 Jan 2013 14:33:25 +0100 Subject: [PATCH 6/6] usb-host: Initialize dev->port the obviously safe way Coverity worries the strcpy() could overrun the destination. It can't, because the source always points to usb_host_scan()'s auto port[], which has the same size. Use pstrcpy() anyway, to hush the checker. Signed-off-by: Markus Armbruster Signed-off-by: Gerd Hoffmann --- hw/usb/host-linux.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/usb/host-linux.c b/hw/usb/host-linux.c index a498840e0..ad75ce070 100644 --- a/hw/usb/host-linux.c +++ b/hw/usb/host-linux.c @@ -1314,7 +1314,7 @@ static int usb_host_open(USBHostDevice *dev, int bus_num, dev->bus_num = bus_num; dev->addr = addr; - strcpy(dev->port, port); + pstrcpy(dev->port, sizeof(dev->port), port); dev->fd = fd; /* read the device description */