[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Qemu-devel] [RESEND] [PATCH] usb-linux.c: fix buffer overflow
From: |
Jim Paris |
Subject: |
[Qemu-devel] [RESEND] [PATCH] usb-linux.c: fix buffer overflow |
Date: |
Wed, 9 Sep 2009 14:42:34 -0400 |
User-agent: |
Mutt/1.5.18 (2008-05-17) |
From 0865ed67a30a1bde463a8db2baab8b6dbb0fcff5 Mon Sep 17 00:00:00 2001
From: Jim Paris <address@hidden>
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/address@hidden/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 <address@hidden>
---
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;
--
1.6.1.3
[Prev in Thread] |
Current Thread |
[Next in Thread] |
- [Qemu-devel] [RESEND] [PATCH] usb-linux.c: fix buffer overflow,
Jim Paris <=