[Qemu-devel] [PULL for-2.6 0/2] ehci: fix (s)iTD looping issue (CVE-2015-8558) in a different way.

[Qemu-devel] [PULL for-2.6 0/2] ehci: fix (s)iTD looping issue (CVE-2015-8558) in a different way.

[Gerd Hoffmann]

  Hi,

CVE-2015-8558 fix turned out to break FreeBSD and being
incomplete -- go for a different approach to fix things.

please pull,
  Gerd

The following changes since commit c6c598ca5fba68fbd6612f3330c4015142f2f86a:

  Merge remote-tracking branch 'remotes/weil/tags/pull-wxx-20160415' into staging (2016-04-18 09:55:16 +0100)

are available in the git repository at:


  git://git.kraxel.org/qemu tags/pull-usb-20160419-1

for you to fetch changes up to a49923d2837d20510d645d3758f1ad87c32d0730:

  Revert "ehci: make idt processing more robust" (2016-04-19 08:20:56 +0200)

----------------------------------------------------------------
ehci: fix (s)iTD looping issue (CVE-2015-8558) in a different way.

----------------------------------------------------------------
Gerd Hoffmann (2):
      ehci: apply limit to iTD/sidt descriptors
      Revert "ehci: make idt processing more robust"

 hw/usb/hcd-ehci.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

[Qemu-devel] [PULL 1/2] ehci: apply limit to iTD/sidt descriptors

[Gerd Hoffmann]

Commit "156a2e4 ehci: make idt processing more robust" tries to avoid a
DoS by the guest (create a circular iTD queue and let qemu ehci
emulation run in circles forever).  Unfortunately this has two problems:
First it misses the case of siTDs, and second it reportedly breaks
FreeBSD.

So lets go for a different approach: just count the number of iTDs and
siTDs we have seen per frame and apply a limit.  That should really
catch all cases now.

Reported-by: 杜少博 <dushaobo@360.cn>
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/usb/hcd-ehci.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/hw/usb/hcd-ehci.c b/hw/usb/hcd-ehci.c
index 159f58d..d5c0e1c 100644
--- a/hw/usb/hcd-ehci.c
+++ b/hw/usb/hcd-ehci.c
@@ -2011,6 +2011,7 @@ static int ehci_state_writeback(EHCIQueue *q)
 static void ehci_advance_state(EHCIState *ehci, int async)
 {
     EHCIQueue *q = NULL;
+    int itd_count = 0;
     int again;
 
     do {
@@ -2035,10 +2036,12 @@ static void ehci_advance_state(EHCIState *ehci, int async)
 
         case EST_FETCHITD:
             again = ehci_state_fetchitd(ehci, async);
+            itd_count++;
             break;
 
         case EST_FETCHSITD:
             again = ehci_state_fetchsitd(ehci, async);
+            itd_count++;
             break;
 
         case EST_ADVANCEQUEUE:
@@ -2087,7 +2090,8 @@ static void ehci_advance_state(EHCIState *ehci, int async)
             break;
         }
 
-        if (again < 0) {
+        if (again < 0 || itd_count > 16) {
+            /* TODO: notify guest (raise HSE irq?) */
             fprintf(stderr, "processing error - resetting ehci HC\n");
             ehci_reset(ehci);
             again = 0;
-- 
1.8.3.1

[Qemu-devel] [PULL 2/2] Revert "ehci: make idt processing more robust"

[Gerd Hoffmann]

This reverts commit 156a2e4dbffa85997636a7a39ef12da6f1b40254.

Breaks FreeBSD.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/usb/hcd-ehci.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/hw/usb/hcd-ehci.c b/hw/usb/hcd-ehci.c
index d5c0e1c..43a8f7a 100644
--- a/hw/usb/hcd-ehci.c
+++ b/hw/usb/hcd-ehci.c
@@ -1397,7 +1397,7 @@ static int ehci_process_itd(EHCIState *ehci,
 {
     USBDevice *dev;
     USBEndpoint *ep;
-    uint32_t i, len, pid, dir, devaddr, endp, xfers = 0;
+    uint32_t i, len, pid, dir, devaddr, endp;
     uint32_t pg, off, ptr1, ptr2, max, mult;
 
     ehci->periodic_sched_active = PERIODIC_ACTIVE;
@@ -1489,10 +1489,9 @@ static int ehci_process_itd(EHCIState *ehci,
                 ehci_raise_irq(ehci, USBSTS_INT);
             }
             itd->transact[i] &= ~ITD_XACT_ACTIVE;
-            xfers++;
         }
     }
-    return xfers ? 0 : -1;
+    return 0;
 }
 
 
-- 
1.8.3.1

Re: [Qemu-devel] [PULL for-2.6 0/2] ehci: fix (s)iTD looping issue (CVE-2015-8558) in a different way.

[Peter Maydell]

On 19 April 2016 at 07:24, Gerd Hoffmann <kraxel@redhat.com> wrote:
>   Hi,
>
> CVE-2015-8558 fix turned out to break FreeBSD and being
> incomplete -- go for a different approach to fix things.
>
> please pull,
>   Gerd
>
> The following changes since commit c6c598ca5fba68fbd6612f3330c4015142f2f86a:
>
>   Merge remote-tracking branch 'remotes/weil/tags/pull-wxx-20160415' into staging (2016-04-18 09:55:16 +0100)
>
> are available in the git repository at:
>
>
>   git://git.kraxel.org/qemu tags/pull-usb-20160419-1
>
> for you to fetch changes up to a49923d2837d20510d645d3758f1ad87c32d0730:
>
>   Revert "ehci: make idt processing more robust" (2016-04-19 08:20:56 +0200)
>
> ----------------------------------------------------------------
> ehci: fix (s)iTD looping issue (CVE-2015-8558) in a different way.

Applied, thanks.

-- PMM