From patchwork Mon Aug 24 21:14:57 2009 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jim Paris X-Patchwork-Id: 31994 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from lists.gnu.org (lists.gnu.org [199.232.76.165]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (Client did not present a certificate) by bilbo.ozlabs.org (Postfix) with ESMTPS id 6539EB70B3 for ; Tue, 25 Aug 2009 07:15:39 +1000 (EST) Received: from localhost ([127.0.0.1]:55037 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1Mfgt5-0002vz-TI for incoming@patchwork.ozlabs.org; Mon, 24 Aug 2009 17:15:35 -0400 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1MfgsY-0002vo-W3 for qemu-devel@nongnu.org; Mon, 24 Aug 2009 17:15:03 -0400 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1MfgsX-0002vc-CZ for qemu-devel@nongnu.org; Mon, 24 Aug 2009 17:15:01 -0400 Received: from [199.232.76.173] (port=56853 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1MfgsX-0002vZ-9N for qemu-devel@nongnu.org; Mon, 24 Aug 2009 17:15:01 -0400 Received: from jim.sh ([75.150.123.25]:49801 helo=psychosis.jim.sh) by monty-python.gnu.org with esmtps (TLS-1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.60) (envelope-from ) id 1MfgsW-0003lk-TL for qemu-devel@nongnu.org; Mon, 24 Aug 2009 17:15:01 -0400 Received: from psychosis.jim.sh (localhost [127.0.0.1]) by psychosis.jim.sh (8.14.3/8.14.3/Debian-5) with ESMTP id n7OLEvL8000994 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=FAIL) for ; Mon, 24 Aug 2009 17:14:58 -0400 Received: (from jim@localhost) by psychosis.jim.sh (8.14.3/8.14.3/Submit) id n7OLEvcg000993 for qemu-devel@nongnu.org; Mon, 24 Aug 2009 17:14:57 -0400 Date: Mon, 24 Aug 2009 17:14:57 -0400 From: Jim Paris To: qemu-devel@nongnu.org Message-ID: <20090824211457.GA761@psychosis.jim.sh> MIME-Version: 1.0 Content-Disposition: inline User-Agent: Mutt/1.5.18 (2008-05-17) X-Virus-Scanned: ClamAV 0.94.2/9733/Mon Aug 24 16:40:23 2009 on psychosis.jim.sh X-Virus-Status: Clean X-detected-operating-system: by monty-python.gnu.org: GNU/Linux 2.6 (newer, 3) Subject: [Qemu-devel] [PATCH] usb-linux.c: fix buffer overflow X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org From 0865ed67a30a1bde463a8db2baab8b6dbb0fcff5 Mon Sep 17 00:00:00 2001 From: Jim Paris Date: Mon, 24 Aug 2009 14:56:12 -0400 Subject: [PATCH] usb-linux.c: fix buffer overflow In usb-linux.c:usb_host_handle_control, we pass a 1024-byte buffer and length to the kernel. However, the length was provided by the caller of dev->handle_packet, and is not checked, so the kernel might provide too much data and overflow our buffer. For example, hw/usb-uhci.c could set the length to 2047. hw/usb-ohci.c looks like it might go up to 4096 or 8192. This causes a qemu crash, as reported here: http://www.mail-archive.com/kvm@vger.kernel.org/msg18447.html This patch increases the usb-linux.c buffer size to 2048 to fix the specific device reported, and adds a check to avoid the overflow in any case. Signed-off-by: Jim Paris --- Increasing the buffer to 2048 is all that's necessary to fix the original bug report. It might make sense to increase the buffer size more (hw/usb-musb.c has some comments about 32k buffers). I don't understand all of the USB code too well, so the rest of the patch might not be optimal -- maybe it's OK to just cap the length before we pass it to the kernel, and then let async_complete_control notice that the transferred length was less than requested, rather than failing the request completely? usb-linux.c | 12 ++++++++++-- 1 files changed, 10 insertions(+), 2 deletions(-) diff --git a/usb-linux.c b/usb-linux.c index 043f6b6..eb1c5f0 100644 --- a/usb-linux.c +++ b/usb-linux.c @@ -115,7 +115,7 @@ struct ctrl_struct { uint16_t offset; uint8_t state; struct usb_ctrlrequest req; - uint8_t buffer[1024]; + uint8_t buffer[2048]; }; typedef struct USBHostDevice { @@ -552,6 +552,7 @@ static int usb_host_handle_control(USBHostDevice *s, USBPacket *p) struct usbdevfs_urb *urb; AsyncURB *aurb; int ret, value, index; + int buffer_len; /* * Process certain standard device requests. @@ -580,6 +581,13 @@ static int usb_host_handle_control(USBHostDevice *s, USBPacket *p) /* The rest are asynchronous */ + buffer_len = 8 + s->ctrl.len; + if (buffer_len > sizeof(s->ctrl.buffer)) { + fprintf(stderr, "husb: ctrl buffer too small (%u > %lu)\n", + buffer_len, sizeof(s->ctrl.buffer)); + return USB_RET_STALL; + } + aurb = async_alloc(); aurb->hdev = s; aurb->packet = p; @@ -596,7 +604,7 @@ static int usb_host_handle_control(USBHostDevice *s, USBPacket *p) urb->endpoint = p->devep; urb->buffer = &s->ctrl.req; - urb->buffer_length = 8 + s->ctrl.len; + urb->buffer_length = buffer_len; urb->usercontext = s;