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