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);