[Qemu-devel] Guest OS hangs on usb_add

[Qemu-devel] Guest OS hangs on usb_add

[Timothy Jones]

I am trying to attach universal remote control (URC MX-950) to Windows
XP guest as follows:

==============================
(qemu) info usbhost
  Device 1.1, speed 480 Mb/s
    Hub: USB device 1d6b:0002, EHCI Host Controller
  Device 2.1, speed 480 Mb/s
    Hub: USB device 1d6b:0002, EHCI Host Controller
  Device 1.2, speed 480 Mb/s
    Hub: USB device 8087:0020
  Device 2.2, speed 480 Mb/s
    Hub: USB device 8087:0020
  Device 1.3, speed 480 Mb/s
    Class ef: USB device 0c45:6416, Laptop_Integrated_Webcam_2M
  Device 2.5, speed 12 Mb/s
    Vendor Specific: USB device 4647:3000     <-------- MX-950 USB remote
  Device 2.4, speed 1.5 Mb/s
    Class 00: USB device 05ac:0304, Apple Optical USB Mouse

(qemu) usb_add host:4647:3000

usb_create: no bus specified, using "usb.0" for "usb-host"
husb: open device 2.5
==============================

The guest just freezes including the monitor and pegs one of the cores
on host OS at 100%.

I tried the same with 2 other devices on the above list (Apple Mouse
and Webcam) and both worked fine.

I am running: (qemu) info version
0.12.50 (qemu-kvm-devel)

Host: Linux studio 2.6.34-gentoo-r1 #2 SMP Tue Jun 22 23:21:18 EDT
2010 x86_64 Intel(R) Core(TM) i5 CPU M 520 @ 2.40GHz GenuineIntel
GNU/Linux

Guest: Windows XP Pro SP3

FWIW, I tried the above with and without kvm and kvm-intel modules loaded.

Anybody know what could be causing this?

Thank you.

-TJ

[Qemu-devel] Guest OS hangs on usb_add

[Timothy Jones]

With some digging around I found out that the qemu hangs in
usb_host_claim_interfaces, which is caused by screwed up usb
descriptor. The device reports the following:

(gdb) p dev->descr_len
$21 = 50
(gdb) p /x dev->descr[0]@50
$23 = {0x18, 0x1, 0x0, 0x1, 0xff, 0xff, 0xff, 0x8, 0x47, 0x46, 0x0,
0x30, 0x0, 0x0, 0x0, 0x0, 0x0, 0x1, 0x9, 0x2, 0x20,
  0x0, 0x1, 0x1, 0x0, 0x80, 0x19, 0x9, 0x4, 0x0, 0x0, 0x2, 0xff, 0xff,
0xff, 0x0, 0x7, 0x5, 0x81, 0x2, 0x40, 0x0, 0x0,
  0x7, 0x5, 0x3, 0x2, 0x10, 0x0, 0x0}

The first 0x18 (Device Descriptor bLength) is supposed to be decimal
18, not hex! According to USB spec, if the device reports size greater
than expected, the host is supposed ignore the extra bytes. So qemu
behaves correctly here. However, with this length, the following
Configuration Descriptor length falls on a 0x0 and so the qemu spins
in an endless loop. (This is prolly something that should be detected
and reported as error by qemu.)

My question is: This 0x18 -- is this something that comes from the
device itself (ie, firmware bug)? Or does it come from the USB
subsystem?

I don't mind writing a small patch to make descriptor parsing a bit
more intelligent, but I am very unfamiliar with the code, so I might
botch things up. Or is the above data sufficient for one of the devs
to take a look at the code and improve it?

Thank you.

-TJ


---------- Forwarded message ----------
From: Timothy Jones <one.timothy.jones@gmail.com>
Date: Wed, Jun 23, 2010 at 2:21 PM
Subject: Guest OS hangs on usb_add
To: qemu-devel@nongnu.org


I am trying to attach universal remote control (URC MX-950) to Windows
XP guest as follows:

==============================
(qemu) info usbhost
  Device 1.1, speed 480 Mb/s
    Hub: USB device 1d6b:0002, EHCI Host Controller
  Device 2.1, speed 480 Mb/s
    Hub: USB device 1d6b:0002, EHCI Host Controller
  Device 1.2, speed 480 Mb/s
    Hub: USB device 8087:0020
  Device 2.2, speed 480 Mb/s
    Hub: USB device 8087:0020
  Device 1.3, speed 480 Mb/s
    Class ef: USB device 0c45:6416, Laptop_Integrated_Webcam_2M
  Device 2.5, speed 12 Mb/s
    Vendor Specific: USB device 4647:3000     <-------- MX-950 USB remote
  Device 2.4, speed 1.5 Mb/s
    Class 00: USB device 05ac:0304, Apple Optical USB Mouse

(qemu) usb_add host:4647:3000

usb_create: no bus specified, using "usb.0" for "usb-host"
husb: open device 2.5
==============================

The guest just freezes including the monitor and pegs one of the cores
on host OS at 100%.

I tried the same with 2 other devices on the above list (Apple Mouse
and Webcam) and both worked fine.

I am running: (qemu) info version
0.12.50 (qemu-kvm-devel)

Host: Linux studio 2.6.34-gentoo-r1 #2 SMP Tue Jun 22 23:21:18 EDT
2010 x86_64 Intel(R) Core(TM) i5 CPU M 520 @ 2.40GHz GenuineIntel
GNU/Linux

Guest: Windows XP Pro SP3

FWIW, I tried the above with and without kvm and kvm-intel modules loaded.

Anybody know what could be causing this?

Thank you.

-TJ

[Qemu-devel] Guest OS hangs on usb_add

[TJ]

[-- Attachment #1: Type: text/plain, Size: 2048 bytes --]


> ---------- Forwarded message ----------
> From: Timothy Jones <one.timothy.jones@gmail.com>
> Date: Wed, Jun 23, 2010 at 9:07 PM
> Subject: Guest OS hangs on usb_add
> To: qemu-devel@nongnu.org
> 
> 
> With some digging around I found out that the qemu hangs in
> usb_host_claim_interfaces, which is caused by screwed up usb
> descriptor. The device reports the following:
> 
> (gdb) p dev->descr_len
> $21 = 50
> (gdb) p /x dev->descr[0]@50
> $23 = {0x18, 0x1, 0x0, 0x1, 0xff, 0xff, 0xff, 0x8, 0x47, 0x46, 0x0,
> 0x30, 0x0, 0x0, 0x0, 0x0, 0x0, 0x1, 0x9, 0x2, 0x20,
>  0x0, 0x1, 0x1, 0x0, 0x80, 0x19, 0x9, 0x4, 0x0, 0x0, 0x2, 0xff, 0xff,
> 0xff, 0x0, 0x7, 0x5, 0x81, 0x2, 0x40, 0x0, 0x0,
>  0x7, 0x5, 0x3, 0x2, 0x10, 0x0, 0x0}
> 
> The first 0x18 (Device Descriptor bLength) is supposed to be decimal
> 18, not hex! According to USB spec, if the device reports size greater
> than expected, the host is supposed ignore the extra bytes. So qemu
> behaves correctly here. However, with this length, the following
> Configuration Descriptor length falls on a 0x0 and so the qemu spins
> in an endless loop. (This is prolly something that should be detected
> and reported as error by qemu.)
> 
> My question is: This 0x18 -- is this something that comes from the
> device itself (ie, firmware bug)? Or does it come from the USB
> subsystem?
> 
> I don't mind writing a small patch to make descriptor parsing a bit
> more intelligent, but I am very unfamiliar with the code, so I might
> botch things up. Or is the above data sufficient for one of the devs
> to take a look at the code and improve it?
> 
> Thank you.
> 
> -TJ
> 

Here is small patch that fixed my problem.

In looking at the USB spec, it seems pretty clear cut about the whole
device/config/interface/endpoint descriptor hierarchy, so the
usb_host_claim_interfaces can be optimized instead of parsing through each
descriptor to skip through config descriptors using wTotalLength field. And
again, some checks can be done for descriptor types and/or sizes.

Just my 2 cents.

-TJ



[-- Attachment #2: hex-dev-descr-len.patch --]
[-- Type: text/plain, Size: 705 bytes --]

--- hw/usb.h.orig	2010-06-23 22:55:27.649182672 -0400
+++ hw/usb.h	2010-06-23 22:56:09.937910171 -0400
@@ -117,6 +117,8 @@
 #define USB_DT_INTERFACE		0x04
 #define USB_DT_ENDPOINT			0x05
 
+#define USB_DT_DEVICE_LEN		18
+
 #define USB_ENDPOINT_XFER_CONTROL	0
 #define USB_ENDPOINT_XFER_ISOC		1
 #define USB_ENDPOINT_XFER_BULK		2
--- usb-linux.c.orig	2010-06-23 22:56:23.191339634 -0400
+++ usb-linux.c	2010-06-24 00:08:19.437515669 -0400
@@ -299,6 +299,9 @@
 
     i = 0;
     dev_descr_len = dev->descr[0];
+    if (dev_descr_len == 0x18)
+        dev_descr_len = USB_DT_DEVICE_LEN; /* for buggy device(s) reporting len in hex */
+
     if (dev_descr_len > dev->descr_len) {
         goto fail;
     }

Re: [Qemu-devel] Guest OS hangs on usb_add

[David S. Ahern]

On 06/23/10 22:45, TJ wrote:
> 
>> ---------- Forwarded message ----------
>> From: Timothy Jones <one.timothy.jones@gmail.com>
>> Date: Wed, Jun 23, 2010 at 9:07 PM
>> Subject: Guest OS hangs on usb_add
>> To: qemu-devel@nongnu.org
>>
>>
>> With some digging around I found out that the qemu hangs in
>> usb_host_claim_interfaces, which is caused by screwed up usb
>> descriptor. The device reports the following:
>>
>> (gdb) p dev->descr_len
>> $21 = 50
>> (gdb) p /x dev->descr[0]@50
>> $23 = {0x18, 0x1, 0x0, 0x1, 0xff, 0xff, 0xff, 0x8, 0x47, 0x46, 0x0,
>> 0x30, 0x0, 0x0, 0x0, 0x0, 0x0, 0x1, 0x9, 0x2, 0x20,
>>  0x0, 0x1, 0x1, 0x0, 0x80, 0x19, 0x9, 0x4, 0x0, 0x0, 0x2, 0xff, 0xff,
>> 0xff, 0x0, 0x7, 0x5, 0x81, 0x2, 0x40, 0x0, 0x0,
>>  0x7, 0x5, 0x3, 0x2, 0x10, 0x0, 0x0}
>>
>> The first 0x18 (Device Descriptor bLength) is supposed to be decimal
>> 18, not hex! According to USB spec, if the device reports size greater
>> than expected, the host is supposed ignore the extra bytes. So qemu
>> behaves correctly here. However, with this length, the following
>> Configuration Descriptor length falls on a 0x0 and so the qemu spins
>> in an endless loop. (This is prolly something that should be detected
>> and reported as error by qemu.)
>>
>> My question is: This 0x18 -- is this something that comes from the
>> device itself (ie, firmware bug)? Or does it come from the USB
>> subsystem?

What kind of device is this?

David

Re: [Qemu-devel] Guest OS hangs on usb_add

[TJ]

On 06/24/10 13:59, David S. Ahern wrote:
> 
> 
> On 06/23/10 22:45, TJ wrote:
>>
>>> ---------- Forwarded message ----------
>>> From: Timothy Jones <one.timothy.jones@gmail.com>
>>> Date: Wed, Jun 23, 2010 at 9:07 PM
>>> Subject: Guest OS hangs on usb_add
>>> To: qemu-devel@nongnu.org
>>>
>>>
>>> With some digging around I found out that the qemu hangs in
>>> usb_host_claim_interfaces, which is caused by screwed up usb
>>> descriptor. The device reports the following:
>>>
>>> (gdb) p dev->descr_len
>>> $21 = 50
>>> (gdb) p /x dev->descr[0]@50
>>> $23 = {0x18, 0x1, 0x0, 0x1, 0xff, 0xff, 0xff, 0x8, 0x47, 0x46, 0x0,
>>> 0x30, 0x0, 0x0, 0x0, 0x0, 0x0, 0x1, 0x9, 0x2, 0x20,
>>>  0x0, 0x1, 0x1, 0x0, 0x80, 0x19, 0x9, 0x4, 0x0, 0x0, 0x2, 0xff, 0xff,
>>> 0xff, 0x0, 0x7, 0x5, 0x81, 0x2, 0x40, 0x0, 0x0,
>>>  0x7, 0x5, 0x3, 0x2, 0x10, 0x0, 0x0}
>>>
>>> The first 0x18 (Device Descriptor bLength) is supposed to be decimal
>>> 18, not hex! According to USB spec, if the device reports size greater
>>> than expected, the host is supposed ignore the extra bytes. So qemu
>>> behaves correctly here. However, with this length, the following
>>> Configuration Descriptor length falls on a 0x0 and so the qemu spins
>>> in an endless loop. (This is prolly something that should be detected
>>> and reported as error by qemu.)
>>>
>>> My question is: This 0x18 -- is this something that comes from the
>>> device itself (ie, firmware bug)? Or does it come from the USB
>>> subsystem?
> 
> What kind of device is this?
> 
> David
> 

Universal Remote Control MX-950

http://www.universalremote.com/product_detail.php?model=35

-TJ

Re: [Qemu-devel] Guest OS hangs on usb_add

[Gianni Tedesco]

On Thu, 2010-06-24 at 05:45 +0100, TJ wrote:
> Here is small patch that fixed my problem.
> 
> In looking at the USB spec, it seems pretty clear cut about the whole
> device/config/interface/endpoint descriptor hierarchy, so the
> usb_host_claim_interfaces can be optimized instead of parsing through each
> descriptor to skip through config descriptors using wTotalLength field. And
> again, some checks can be done for descriptor types and/or sizes.

A device MAY provide extended descriptors in 2 ways mentioned in the
spec, but ISTR finding at least one device in the wild with standard
descriptors extended which were not so much used by the "host" but by
application software. So not sure about your patch, a quirks blacklist
based on idDevice/idProduct might be the better fix here.

However the more serious problem is spinning on zero length descriptor
when truncated descriptors are not valid and zero length (in fact < 2)
is totally unacceptable. Following patch checks for truncation.

diff --git a/hw/usb.h b/hw/usb.h
index 00d2802..efd4a65 100644
--- a/hw/usb.h
+++ b/hw/usb.h
@@ -117,6 +117,14 @@
 #define USB_DT_INTERFACE		0x04
 #define USB_DT_ENDPOINT			0x05
 
+/*
+ * Descriptor sizes per descriptor type
+ */
+#define USB_DT_DEVICE_SIZE		18
+#define USB_DT_CONFIG_SIZE		9
+#define USB_DT_INTERFACE_SIZE		9
+#define USB_DT_ENDPOINT_SIZE		7
+
 #define USB_ENDPOINT_XFER_CONTROL	0
 #define USB_ENDPOINT_XFER_ISOC		1
 #define USB_ENDPOINT_XFER_BULK		2
diff --git a/usb-linux.c b/usb-linux.c
index 88273ff..d259290 100644
--- a/usb-linux.c
+++ b/usb-linux.c
@@ -299,7 +299,7 @@ static int usb_host_claim_interfaces(USBHostDevice *dev, int configuration)
 
     i = 0;
     dev_descr_len = dev->descr[0];
-    if (dev_descr_len > dev->descr_len) {
+    if ( dev_descr_len < USB_DT_DEVICE_SIZE || dev_descr_len > dev->descr_len) {
         goto fail;
     }
 
@@ -314,6 +314,8 @@ static int usb_host_claim_interfaces(USBHostDevice *dev, int configuration)
             continue;
         }
         config_descr_len = dev->descr[i];
+        if ( config_descr_len < USB_DT_CONFIG_SIZE )
+            goto fail;
 
         printf("husb: config #%d need %d\n", dev->descr[i + 5], configuration);
 

Re: [Qemu-devel] Guest OS hangs on usb_add

[TJ]

On 06/25/10 12:32, Gianni Tedesco wrote:
> A device MAY provide extended descriptors in 2 ways mentioned in the
> spec, but ISTR finding at least one device in the wild with standard
> descriptors extended which were not so much used by the "host" but by
> application software. So not sure about your patch, a quirks blacklist
> based on idDevice/idProduct might be the better fix here.

Makes sense. I should add vend/prod id check.

> However the more serious problem is spinning on zero length descriptor
> when truncated descriptors are not valid and zero length (in fact < 2)
> is totally unacceptable. Following patch checks for truncation.

Gianni, Please check my later patch submitted last night. I basically did the
same thing you did, but with few differences:

- if descriptor size is < 2, goto fail
- if the descriptor is USB_DT_CONFIG, we can skip through all the sub
descriptors using wTotalLength field.
- otherwise, simply skip it

One thing to also watch out for is the string descriptors. I might be wrong, but
it appears (from reading the doc) that string descriptors (at least for the
device descriptor) can be interspersed with the config descriptors, in which
case (config_descr_len < USB_DT_CONFIG_SIZE) without checking descriptor type
might unwittingly lead to failure.

-TJ

> diff --git a/hw/usb.h b/hw/usb.h
> index 00d2802..efd4a65 100644
> --- a/hw/usb.h
> +++ b/hw/usb.h
> @@ -117,6 +117,14 @@
>  #define USB_DT_INTERFACE		0x04
>  #define USB_DT_ENDPOINT			0x05
>  
> +/*
> + * Descriptor sizes per descriptor type
> + */
> +#define USB_DT_DEVICE_SIZE		18
> +#define USB_DT_CONFIG_SIZE		9
> +#define USB_DT_INTERFACE_SIZE		9
> +#define USB_DT_ENDPOINT_SIZE		7
> +
>  #define USB_ENDPOINT_XFER_CONTROL	0
>  #define USB_ENDPOINT_XFER_ISOC		1
>  #define USB_ENDPOINT_XFER_BULK		2
> diff --git a/usb-linux.c b/usb-linux.c
> index 88273ff..d259290 100644
> --- a/usb-linux.c
> +++ b/usb-linux.c
> @@ -299,7 +299,7 @@ static int usb_host_claim_interfaces(USBHostDevice *dev, int configuration)
>  
>      i = 0;
>      dev_descr_len = dev->descr[0];
> -    if (dev_descr_len > dev->descr_len) {
> +    if ( dev_descr_len < USB_DT_DEVICE_SIZE || dev_descr_len > dev->descr_len) {
>          goto fail;
>      }
>  
> @@ -314,6 +314,8 @@ static int usb_host_claim_interfaces(USBHostDevice *dev, int configuration)
>              continue;
>          }
>          config_descr_len = dev->descr[i];
> +        if ( config_descr_len < USB_DT_CONFIG_SIZE )
> +            goto fail;
>  
>          printf("husb: config #%d need %d\n", dev->descr[i + 5], configuration);
>  

Re: [Qemu-devel] Guest OS hangs on usb_add

[Gianni Tedesco]

On Fri, 2010-06-25 at 18:23 +0100, TJ wrote:
> On 06/25/10 12:32, Gianni Tedesco wrote:
> > A device MAY provide extended descriptors in 2 ways mentioned in the
> > spec, but ISTR finding at least one device in the wild with standard
> > descriptors extended which were not so much used by the "host" but by
> > application software. So not sure about your patch, a quirks blacklist
> > based on idDevice/idProduct might be the better fix here.
> 
> Makes sense. I should add vend/prod id check.
> 
> > However the more serious problem is spinning on zero length descriptor
> > when truncated descriptors are not valid and zero length (in fact < 2)
> > is totally unacceptable. Following patch checks for truncation.
> 
> Gianni, Please check my later patch submitted last night. I basically did the
> same thing you did, but with few differences:
> 
> - if descriptor size is < 2, goto fail
> - if the descriptor is USB_DT_CONFIG, we can skip through all the sub
> descriptors using wTotalLength field.
> - otherwise, simply skip it

Good point, just seen you patch and it looks good.

> One thing to also watch out for is the string descriptors. I might be wrong, but
> it appears (from reading the doc) that string descriptors (at least for the
> device descriptor) can be interspersed with the config descriptors, in which
> case (config_descr_len < USB_DT_CONFIG_SIZE) without checking descriptor type
> might unwittingly lead to failure.

Yeah definitely, descriptors can be in pretty much any old order so the
code should not rely on any of that.

FWIW, I am signing off on your approach :)

Gianni Tedesco

Re: [Qemu-devel] Guest OS hangs on usb_add

[TJ]

On 06/28/10 08:32, Gianni Tedesco wrote:
> 
> FWIW, I am signing off on your approach :)
> 
> Gianni Tedesco
> 

Thank you Gianni :) I am gonna add simple vend/prod id check to my 0x18 hack and
resubmit final version.

-TJ

Re: [Qemu-devel] Guest OS hangs on usb_add

[Markus Armbruster]

Timothy Jones <one.timothy.jones@gmail.com> writes:

> With some digging around I found out that the qemu hangs in
> usb_host_claim_interfaces, which is caused by screwed up usb
> descriptor. The device reports the following:
>
> (gdb) p dev->descr_len
> $21 = 50
> (gdb) p /x dev->descr[0]@50
> $23 = {0x18, 0x1, 0x0, 0x1, 0xff, 0xff, 0xff, 0x8, 0x47, 0x46, 0x0,
> 0x30, 0x0, 0x0, 0x0, 0x0, 0x0, 0x1, 0x9, 0x2, 0x20,
>   0x0, 0x1, 0x1, 0x0, 0x80, 0x19, 0x9, 0x4, 0x0, 0x0, 0x2, 0xff, 0xff,
> 0xff, 0x0, 0x7, 0x5, 0x81, 0x2, 0x40, 0x0, 0x0,
>   0x7, 0x5, 0x3, 0x2, 0x10, 0x0, 0x0}
>
> The first 0x18 (Device Descriptor bLength) is supposed to be decimal
> 18, not hex! According to USB spec, if the device reports size greater
> than expected, the host is supposed ignore the extra bytes. So qemu
> behaves correctly here. However, with this length, the following
> Configuration Descriptor length falls on a 0x0 and so the qemu spins
> in an endless loop. (This is prolly something that should be detected
> and reported as error by qemu.)

Yup.

> My question is: This 0x18 -- is this something that comes from the
> device itself (ie, firmware bug)? Or does it come from the USB
> subsystem?

It's been a while since I hacked USB, but IIRC it's from the device.

> I don't mind writing a small patch to make descriptor parsing a bit
> more intelligent, but I am very unfamiliar with the code, so I might
> botch things up. Or is the above data sufficient for one of the devs
> to take a look at the code and improve it?

A botched up patch is often a pretty effective way to get somebody to
fix the thing correctly.

Re: [Qemu-devel] Guest OS hangs on usb_add

[TJ]

On 06/24/10 02:42, Markus Armbruster wrote:

> A botched up patch is often a pretty effective way to get somebody to
> fix the thing correctly.

OK, I gave it a shot and sent it to the list. Shoulda prolly added a disclaimer
in case it blows something up ;)

-TJ