]> git.sur5r.net Git - u-boot/commitdiff
usb: Use well-known descriptor sizes when parsing configuration
authorJulius Werner <jwerner@chromium.org>
Fri, 19 Jul 2013 20:12:08 +0000 (13:12 -0700)
committerMarek Vasut <marex@denx.de>
Mon, 26 Aug 2013 19:56:34 +0000 (21:56 +0200)
The existing USB configuration parsing code relies on the descriptors'
own length values when reading through the configuration blob. Since the
size of those descriptors is always well-defined, we should rather use
the known sizes instead of trusting device-provided values to be
correct. Also adds some safety to potential out-of-order descriptors.

Change-Id: I16f69dfdd6793aa0fe930b5148d4521f3e5c3090
Signed-off-by: Julius Werner <jwerner@chromium.org>
common/usb.c
common/usb_hub.c

index f740e5ec219cbe42a8d7375ed582b277a2ebc50b..c97f522beddd1745056c6f97fe3bdf01ed8fea94 100644 (file)
@@ -323,6 +323,7 @@ static int usb_set_maxpacket(struct usb_device *dev)
 /*******************************************************************************
  * Parse the config, located in buffer, and fills the dev->config structure.
  * Note that all little/big endian swapping are done automatically.
+ * (wTotalLength has already been swapped and sanitized when it was read.)
  */
 static int usb_parse_config(struct usb_device *dev,
                        unsigned char *buffer, int cfgno)
@@ -343,24 +344,43 @@ static int usb_parse_config(struct usb_device *dev,
                        head->bDescriptorType);
                return -1;
        }
-       memcpy(&dev->config, buffer, buffer[0]);
-       le16_to_cpus(&(dev->config.desc.wTotalLength));
+       if (head->bLength != USB_DT_CONFIG_SIZE) {
+               printf("ERROR: Invalid USB CFG length (%d)\n", head->bLength);
+               return -1;
+       }
+       memcpy(&dev->config, head, USB_DT_CONFIG_SIZE);
        dev->config.no_of_if = 0;
 
        index = dev->config.desc.bLength;
        /* Ok the first entry must be a configuration entry,
         * now process the others */
        head = (struct usb_descriptor_header *) &buffer[index];
-       while (index + 1 < dev->config.desc.wTotalLength) {
+       while (index + 1 < dev->config.desc.wTotalLength && head->bLength) {
                switch (head->bDescriptorType) {
                case USB_DT_INTERFACE:
+                       if (head->bLength != USB_DT_INTERFACE_SIZE) {
+                               printf("ERROR: Invalid USB IF length (%d)\n",
+                                       head->bLength);
+                               break;
+                       }
+                       if (index + USB_DT_INTERFACE_SIZE >
+                           dev->config.desc.wTotalLength) {
+                               puts("USB IF descriptor overflowed buffer!\n");
+                               break;
+                       }
                        if (((struct usb_interface_descriptor *) \
-                            &buffer[index])->bInterfaceNumber != curr_if_num) {
+                            head)->bInterfaceNumber != curr_if_num) {
                                /* this is a new interface, copy new desc */
                                ifno = dev->config.no_of_if;
+                               if (ifno >= USB_MAXINTERFACES) {
+                                       puts("Too many USB interfaces!\n");
+                                       /* try to go on with what we have */
+                                       return 1;
+                               }
                                if_desc = &dev->config.if_desc[ifno];
                                dev->config.no_of_if++;
-                               memcpy(if_desc, &buffer[index], buffer[index]);
+                               memcpy(if_desc, head,
+                                       USB_DT_INTERFACE_SIZE);
                                if_desc->no_of_ep = 0;
                                if_desc->num_altsetting = 1;
                                curr_if_num =
@@ -374,12 +394,31 @@ static int usb_parse_config(struct usb_device *dev,
                        }
                        break;
                case USB_DT_ENDPOINT:
+                       if (head->bLength != USB_DT_ENDPOINT_SIZE) {
+                               printf("ERROR: Invalid USB EP length (%d)\n",
+                                       head->bLength);
+                               break;
+                       }
+                       if (index + USB_DT_ENDPOINT_SIZE >
+                           dev->config.desc.wTotalLength) {
+                               puts("USB EP descriptor overflowed buffer!\n");
+                               break;
+                       }
+                       if (ifno < 0) {
+                               puts("Endpoint descriptor out of order!\n");
+                               break;
+                       }
                        epno = dev->config.if_desc[ifno].no_of_ep;
                        if_desc = &dev->config.if_desc[ifno];
+                       if (epno > USB_MAXENDPOINTS) {
+                               printf("Interface %d has too many endpoints!\n",
+                                       if_desc->desc.bInterfaceNumber);
+                               return 1;
+                       }
                        /* found an endpoint */
                        if_desc->no_of_ep++;
-                       memcpy(&if_desc->ep_desc[epno],
-                               &buffer[index], buffer[index]);
+                       memcpy(&if_desc->ep_desc[epno], head,
+                               USB_DT_ENDPOINT_SIZE);
                        ep_wMaxPacketSize = get_unaligned(&dev->config.\
                                                        if_desc[ifno].\
                                                        ep_desc[epno].\
@@ -392,9 +431,23 @@ static int usb_parse_config(struct usb_device *dev,
                        debug("if %d, ep %d\n", ifno, epno);
                        break;
                case USB_DT_SS_ENDPOINT_COMP:
+                       if (head->bLength != USB_DT_SS_EP_COMP_SIZE) {
+                               printf("ERROR: Invalid USB EPC length (%d)\n",
+                                       head->bLength);
+                               break;
+                       }
+                       if (index + USB_DT_SS_EP_COMP_SIZE >
+                           dev->config.desc.wTotalLength) {
+                               puts("USB EPC descriptor overflowed buffer!\n");
+                               break;
+                       }
+                       if (ifno < 0 || epno < 0) {
+                               puts("EPC descriptor out of order!\n");
+                               break;
+                       }
                        if_desc = &dev->config.if_desc[ifno];
-                       memcpy(&if_desc->ss_ep_comp_desc[epno],
-                               &buffer[index], buffer[index]);
+                       memcpy(&if_desc->ss_ep_comp_desc[epno], head,
+                               USB_DT_SS_EP_COMP_SIZE);
                        break;
                default:
                        if (head->bLength == 0)
@@ -473,7 +526,7 @@ int usb_get_configuration_no(struct usb_device *dev,
                             unsigned char *buffer, int cfgno)
 {
        int result;
-       unsigned int tmp;
+       unsigned int length;
        struct usb_config_descriptor *config;
 
        config = (struct usb_config_descriptor *)&buffer[0];
@@ -487,16 +540,18 @@ int usb_get_configuration_no(struct usb_device *dev,
                                "(expected %i, got %i)\n", 9, result);
                return -1;
        }
-       tmp = le16_to_cpu(config->wTotalLength);
+       length = le16_to_cpu(config->wTotalLength);
 
-       if (tmp > USB_BUFSIZ) {
-               printf("usb_get_configuration_no: failed to get " \
-                      "descriptor - too long: %d\n", tmp);
+       if (length > USB_BUFSIZ) {
+               printf("%s: failed to get descriptor - too long: %d\n",
+                       __func__, length);
                return -1;
        }
 
-       result = usb_get_descriptor(dev, USB_DT_CONFIG, cfgno, buffer, tmp);
-       debug("get_conf_no %d Result %d, wLength %d\n", cfgno, result, tmp);
+       result = usb_get_descriptor(dev, USB_DT_CONFIG, cfgno, buffer, length);
+       debug("get_conf_no %d Result %d, wLength %d\n", cfgno, result, length);
+       config->wTotalLength = length; /* validated, with CPU byte order */
+
        return result;
 }
 
index fd2b4ed4f4a8b71a68877d3877ea58a8893327a0..ffac0e743cef38f2c98cb7a8c58d40acd5097516 100644 (file)
@@ -306,7 +306,7 @@ void usb_hub_port_connect_change(struct usb_device *dev, int port)
 
 static int usb_hub_configure(struct usb_device *dev)
 {
-       int i;
+       int i, length;
        ALLOC_CACHE_ALIGN_BUFFER(unsigned char, buffer, USB_BUFSIZ);
        unsigned char *bitmap;
        short hubCharacteristics;
@@ -327,20 +327,14 @@ static int usb_hub_configure(struct usb_device *dev)
        }
        descriptor = (struct usb_hub_descriptor *)buffer;
 
-       /* silence compiler warning if USB_BUFSIZ is > 256 [= sizeof(char)] */
-       i = descriptor->bLength;
-       if (i > USB_BUFSIZ) {
-               debug("usb_hub_configure: failed to get hub " \
-                     "descriptor - too long: %d\n", descriptor->bLength);
-               return -1;
-       }
+       length = min(descriptor->bLength, sizeof(struct usb_hub_descriptor));
 
-       if (usb_get_hub_descriptor(dev, buffer, descriptor->bLength) < 0) {
+       if (usb_get_hub_descriptor(dev, buffer, length) < 0) {
                debug("usb_hub_configure: failed to get hub " \
                      "descriptor 2nd giving up %lX\n", dev->status);
                return -1;
        }
-       memcpy((unsigned char *)&hub->desc, buffer, descriptor->bLength);
+       memcpy((unsigned char *)&hub->desc, buffer, length);
        /* adjust 16bit values */
        put_unaligned(le16_to_cpu(get_unaligned(
                        &descriptor->wHubCharacteristics)),