[Qemu-devel] [PATCH v3] usb: add pid check at the first of uhci_handle_td()

[Qemu-devel] [PATCH v3] usb: add pid check at the first of uhci_handle_td()

[Gonglei]

pid can be gotten from uhci device memory in uhci_handle_td(),
so the guest can trigger assert qemu if we get an invalid pid.
And the uhci spec 2.1.2 tells us The Host Controller sets Host
Controller Process Error bit to 1 when it detects a fatal error
and indicates that the Host Controller suffered a consistency
check failure while processing a Transfer Descriptor. An example
of a consistency check failure would be finding an illegal PID
field while processing the packet header portion of the TD.
When this error occurs, the Host Controller clears the Run/Stop
bit in the Command register to prevent further schedule execution.

We'd better to set UHCI_STS_HCPERR and kick an interrupt, check
the pid value at the first of uhci_handle_td function.

[Also fixed BZ 1070027]

Signed-off-by: Gonglei <arei.gonglei@huawei.com>
---
 v3:  checking whenever the pid is valid as very first
      thing in uhci_handle_td.  (As Gerd's suggestion, thanks)

 hw/usb/hcd-uhci.c | 22 ++++++++++++++++------
 1 file changed, 16 insertions(+), 6 deletions(-)

diff --git a/hw/usb/hcd-uhci.c b/hw/usb/hcd-uhci.c
index 5ccfb83..03fe599 100644
--- a/hw/usb/hcd-uhci.c
+++ b/hw/usb/hcd-uhci.c
@@ -773,8 +773,22 @@ static int uhci_handle_td(UHCIState *s, UHCIQueue *q, uint32_t qh_addr,
     bool spd;
     bool queuing = (q != NULL);
     uint8_t pid = td->token & 0xff;
-    UHCIAsync *async = uhci_async_find_td(s, td_addr);
+    UHCIAsync *async;
 
+    switch(pid) {
+    case USB_TOKEN_OUT:
+    case USB_TOKEN_SETUP:
+    case USB_TOKEN_IN:
+        break;
+    default:
+        /* invalid pid : frame interrupted */
+        s->status |= UHCI_STS_HCPERR;
+        s->cmd &= ~UHCI_CMD_RS;
+        uhci_update_irq(s);
+        return TD_RESULT_STOP_FRAME;
+    }
+
+    async = uhci_async_find_td(s, td_addr);
     if (async) {
         if (uhci_queue_verify(async->queue, qh_addr, td, td_addr, queuing)) {
             assert(q == NULL || q == async->queue);
@@ -880,11 +894,7 @@ static int uhci_handle_td(UHCIState *s, UHCIQueue *q, uint32_t qh_addr,
         break;
 
     default:
-        /* invalid pid : frame interrupted */
-        uhci_async_free(async);
-        s->status |= UHCI_STS_HCPERR;
-        uhci_update_irq(s);
-        return TD_RESULT_STOP_FRAME;
+        abort(); /* Never to execute */
     }
 
     if (async->packet.status == USB_RET_ASYNC) {
-- 
1.8.5.2

Re: [Qemu-devel] [PATCH v3] usb: add pid check at the first of uhci_handle_td()

[Gerd Hoffmann]

  Hi,

> [Also fixed BZ 1070027]

Which bugzilla instance is this?

Better cut+paste the full bug URL into the commit message.

Patch added to usb queue.

thanks,
  Gerd

Re: [Qemu-devel] [PATCH v3] usb: add pid check at the first of uhci_handle_td()

[Gonglei (Arei)]

> -----Original Message-----
> From: Gerd Hoffmann [mailto:kraxel@redhat.com]
> Sent: Monday, February 22, 2016 4:55 PM
> 
>   Hi,
> 
> > [Also fixed BZ 1070027]
> 
> Which bugzilla instance is this?
> 

https://bugzilla.redhat.com/show_bug.cgi?id=1070027

> Better cut+paste the full bug URL into the commit message.
> 
Yes, can you rebase and change the commit message?

> Patch added to usb queue.
> 
Thanks!

> thanks,
>   Gerd


Regards,
-Gonglei

Re: [Qemu-devel] [PATCH v3] usb: add pid check at the first of uhci_handle_td()

[Gerd Hoffmann]

  Hi,

> https://bugzilla.redhat.com/show_bug.cgi?id=1070027
> 
> Yes, can you rebase and change the commit message?

Done.

cheers,
  Gerd