From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:42353) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1T9WGz-0004lv-05 for qemu-devel@nongnu.org; Thu, 06 Sep 2012 03:13:28 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1T9WGq-0006NR-Dg for qemu-devel@nongnu.org; Thu, 06 Sep 2012 03:13:08 -0400 Received: from mx1.redhat.com ([209.132.183.28]:5513) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1T9WGq-0006Mh-5K for qemu-devel@nongnu.org; Thu, 06 Sep 2012 03:13:00 -0400 Received: from int-mx11.intmail.prod.int.phx2.redhat.com (int-mx11.intmail.prod.int.phx2.redhat.com [10.5.11.24]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id q867CxV7019286 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Thu, 6 Sep 2012 03:12:59 -0400 From: Gerd Hoffmann Date: Thu, 6 Sep 2012 09:12:15 +0200 Message-Id: <1346915575-12369-15-git-send-email-kraxel@redhat.com> In-Reply-To: <1346915575-12369-1-git-send-email-kraxel@redhat.com> References: <1346915575-12369-1-git-send-email-kraxel@redhat.com> Subject: [Qemu-devel] [PATCH 14/54] ehci: Fix memory leak in handling of NAK-ed packets List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: qemu-devel@nongnu.org Cc: Hans de Goede , Gerd Hoffmann From: Hans de Goede 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 Signed-off-by: Gerd Hoffmann --- hw/usb/hcd-ehci.c | 38 +++++++++++++++++++++++++++----------- 1 files 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.1