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

[TJ <one.timothy.jones@gmail.com>]

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