From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:56625) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aYAcT-0002tW-H0 for qemu-devel@nongnu.org; Tue, 23 Feb 2016 05:55:06 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aYAcS-000251-8G for qemu-devel@nongnu.org; Tue, 23 Feb 2016 05:55:05 -0500 Received: from mx1.redhat.com ([209.132.183.28]:33558) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aYAcR-00024m-VL for qemu-devel@nongnu.org; Tue, 23 Feb 2016 05:55:04 -0500 From: Gerd Hoffmann Date: Tue, 23 Feb 2016 11:54:57 +0100 Message-Id: <1456224898-23270-6-git-send-email-kraxel@redhat.com> In-Reply-To: <1456224898-23270-1-git-send-email-kraxel@redhat.com> References: <1456224898-23270-1-git-send-email-kraxel@redhat.com> Subject: [Qemu-devel] [PULL 5/6] usb: add pid check at the first of uhci_handle_td() List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: qemu-devel@nongnu.org Cc: Gonglei , Gerd Hoffmann From: 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. https://bugzilla.redhat.com/show_bug.cgi?id=1070027 Signed-off-by: Gonglei Message-id: 1455867238-4720-1-git-send-email-arei.gonglei@huawei.com [ applied minor codestyle fix ] Signed-off-by: Gerd Hoffmann --- 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..c370240 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.3.1