From 5d369bd44cf2611f47fec52c7402472cd2436b4a Mon Sep 17 00:00:00 2001

From: Hans de Goede <hdegoede@redhat.com>

Date: Mon, 3 Sep 2012 11:01:13 +0200

Subject: [PATCH] ehci: Fix memory leak in handling of NAK-ed packets

Currently each time we try to execute a NAK-ed packet we redo

ehci_init_transfer, and usb_packet_map, re-allocing (without freeing) the

sg list every time.

This patch fixes this, it does this by introducing another async state, so

that we also properly cleanup a NAK-ed packet on cancel.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>

(cherry picked from commit ef5b234477df80700b128f561f5877a0688a70c8)

Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>

---

 hw/usb/hcd-ehci.c | 38 +++++++++++++++++++++++++++-----------

 1 file changed, 27 insertions(+), 11 deletions(-)

diff --git a/hw/usb/hcd-ehci.c b/hw/usb/hcd-ehci.c

index 5a88268..d87aca8 100644

--- a/hw/usb/hcd-ehci.c

+++ b/hw/usb/hcd-ehci.c

@@ -345,6 +345,7 @@ typedef struct EHCIState EHCIState;

 enum async_state {

     EHCI_ASYNC_NONE = 0,

+    EHCI_ASYNC_INITIALIZED,

     EHCI_ASYNC_INFLIGHT,

     EHCI_ASYNC_FINISHED,

 };

@@ -764,6 +765,10 @@ static void ehci_free_packet(EHCIPacket *p)

         return;

     }

     trace_usb_ehci_packet_action(p->queue, p, "free");

+    if (p->async == EHCI_ASYNC_INITIALIZED) {

+        usb_packet_unmap(&p->packet, &p->sgl);

+        qemu_sglist_destroy(&p->sgl);

+    }

     if (p->async == EHCI_ASYNC_INFLIGHT) {

         usb_cancel_packet(&p->packet);

         usb_packet_unmap(&p->packet, &p->sgl);

@@ -1485,8 +1490,8 @@ static void ehci_execute_complete(EHCIQueue *q)

     assert(p != NULL);

     assert(p->qtdaddr == q->qtdaddr);

-    assert(p->async != EHCI_ASYNC_INFLIGHT);

-    p->async = EHCI_ASYNC_NONE;

+    assert(p->async == EHCI_ASYNC_INITIALIZED ||

+           p->async == EHCI_ASYNC_FINISHED);

     DPRINTF("execute_complete: qhaddr 0x%x, next %x, qtdaddr 0x%x, status %d\n",

             q->qhaddr, q->qh.next, q->qtdaddr, q->usb_status);

@@ -1531,6 +1536,7 @@ static void ehci_execute_complete(EHCIQueue *q)

     ehci_finish_transfer(q, p->usb_status);

     usb_packet_unmap(&p->packet, &p->sgl);

     qemu_sglist_destroy(&p->sgl);

+    p->async = EHCI_ASYNC_NONE;

     q->qh.token ^= QTD_TOKEN_DTOGGLE;

     q->qh.token &= ~QTD_TOKEN_ACTIVE;

@@ -1548,6 +1554,9 @@ static int ehci_execute(EHCIPacket *p, const char *action)

     int ret;

     int endp;

+    assert(p->async == EHCI_ASYNC_NONE ||

+           p->async == EHCI_ASYNC_INITIALIZED);

+

     if (!(p->qtd.token & QTD_TOKEN_ACTIVE)) {

         fprintf(stderr, "Attempting to execute inactive qtd\n");

         return USB_RET_PROCERR;

@@ -1576,15 +1585,18 @@ static int ehci_execute(EHCIPacket *p, const char *action)

         break;

     }

-    if (ehci_init_transfer(p) != 0) {

-        return USB_RET_PROCERR;

-    }

-

     endp = get_field(p->queue->qh.epchar, QH_EPCHAR_EP);

     ep = usb_ep_get(p->queue->dev, p->pid, endp);

-    usb_packet_setup(&p->packet, p->pid, ep, p->qtdaddr);

-    usb_packet_map(&p->packet, &p->sgl);

+    if (p->async == EHCI_ASYNC_NONE) {

+        if (ehci_init_transfer(p) != 0) {

+            return USB_RET_PROCERR;

+        }

+

+        usb_packet_setup(&p->packet, p->pid, ep, p->qtdaddr);

+        usb_packet_map(&p->packet, &p->sgl);

+        p->async = EHCI_ASYNC_INITIALIZED;

+    }

     trace_usb_ehci_packet_action(p->queue, p, action);

     ret = usb_handle_packet(p->queue->dev, &p->packet);

@@ -2021,11 +2033,15 @@ static int ehci_state_fetchqtd(EHCIQueue *q)

     } else if (p != NULL) {

         switch (p->async) {

         case EHCI_ASYNC_NONE:

+            /* Should never happen packet should at least be initialized */

+            assert(0);

+            break;

+        case EHCI_ASYNC_INITIALIZED:

             /* Previously nacked packet (likely interrupt ep) */

-           ehci_set_state(q->ehci, q->async, EST_EXECUTE);

-           break;

+            ehci_set_state(q->ehci, q->async, EST_EXECUTE);

+            break;

         case EHCI_ASYNC_INFLIGHT:

-            /* Unfinyshed async handled packet, go horizontal */

+            /* Unfinished async handled packet, go horizontal */

             ehci_set_state(q->ehci, q->async, EST_HORIZONTALQH);

             break;

         case EHCI_ASYNC_FINISHED:

-- 

1.7.12.1