[Qemu-devel] [PULL 0/6] usb patch queue

[Qemu-devel] [PULL 0/6] usb patch queue

[Gerd Hoffmann]

  Hi,

The usb patch queue, with coverity fixes.  Also makes xhci
cancel inflight transfers on usb device unplug.

please pull,
  Gerd

The following changes since commit 63fb2590839162afdf14d7c0ee02d460766c0956:

  Merge branch 'target-arm.next' of git://git.linaro.org/people/pmaydell/qemu-arm (2013-01-12 12:47:07 +0000)

are available in the git repository at:


  git://git.kraxel.org/qemu usb.76

for you to fetch changes up to 036078475427f2562c8e505f6bb44dbf5d8cbd95:

  usb-host: Initialize dev->port the obviously safe way (2013-01-14 12:47:11 +0100)

----------------------------------------------------------------
Gerd Hoffmann (3):
      xhci: create xhci_detach_slot helper function
      xhci: call xhci_detach_slot on root port detach too
      xhci: nuke transfe5rs on detach

Markus Armbruster (3):
      ehci: Assert state machine is sane w.r.t. EHCIQueue
      usb-host: Drop superfluous null test from usb_host_auto_scan()
      usb-host: Initialize dev->port the obviously safe way

 hw/usb/hcd-ehci.c   |    4 ++++
 hw/usb/hcd-xhci.c   |   31 +++++++++++++++++++++++++------
 hw/usb/host-linux.c |    4 ++--
 3 files changed, 31 insertions(+), 8 deletions(-)

[Qemu-devel] [PATCH 1/6] xhci: create xhci_detach_slot helper function

[Gerd Hoffmann]

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 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 92f2eee..3ff8bc1 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 = {
-- 
1.7.9.7

[Qemu-devel] [PATCH 2/6] xhci: call xhci_detach_slot on root port detach too

[Gerd Hoffmann]

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 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 3ff8bc1..5b2e7f8 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);
 }
 
-- 
1.7.9.7

[Qemu-devel] [PATCH 3/6] xhci: nuke transfe5rs on detach

[Gerd Hoffmann]

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 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 5b2e7f8..5fb0c48 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;
 }
 
-- 
1.7.9.7

[Qemu-devel] [PATCH 4/6] ehci: Assert state machine is sane w.r.t. EHCIQueue

[Gerd Hoffmann]

From: Markus Armbruster <armbru@redhat.com>

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 <armbru@redhat.com>
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 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 320b7e7..7040659 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;
-- 
1.7.9.7

[Qemu-devel] [PATCH 5/6] usb-host: Drop superfluous null test from usb_host_auto_scan()

[Gerd Hoffmann]

From: Markus Armbruster <armbru@redhat.com>

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 <armbru@redhat.com>
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 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 e8e6a42..a498840 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;
         }
 
-- 
1.7.9.7

[Qemu-devel] [PATCH 6/6] usb-host: Initialize dev->port the obviously safe way

[Gerd Hoffmann]

From: Markus Armbruster <armbru@redhat.com>

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 <armbru@redhat.com>
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 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 a498840..ad75ce0 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 */
-- 
1.7.9.7

Re: [Qemu-devel] [PULL 0/6] usb patch queue

[Anthony Liguori]

Pulled, thanks.

Regards,

Anthony Liguori