[Qemu-devel] [PATCH 0/3] Minor USB cleanups

[Qemu-devel] [PATCH 0/3] Minor USB cleanups

[Markus Armbruster]

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/host-linux.c | 4 ++--
 2 files changed, 6 insertions(+), 2 deletions(-)

-- 
1.7.11.7

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

[Markus Armbruster]

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>
---
 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.11.7

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

[Markus Armbruster]

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>
---
 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 669fbd2..ea2179f 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.11.7

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

[Markus Armbruster]

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>
---
 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 ea2179f..fc7c5ca 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.11.7

Re: [Qemu-devel] [PATCH 0/3] Minor USB cleanups

[Gerd Hoffmann]

On 01/10/13 14:33, Markus Armbruster wrote:
> 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/host-linux.c | 4 ++--
>  2 files changed, 6 insertions(+), 2 deletions(-)
> 

Patch added to usb patch queue.

thanks,
  Gerd