diff mbox

Guest OS hangs on usb_add

Message ID 1277483535.5325.5.camel@qabil.uk.xensource.com
State New
Headers show

Commit Message

Gianni Tedesco June 25, 2010, 4:32 p.m. UTC 
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 mbox

Patch

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