[U-Boot] [PATCH] usb: Use well-known descriptor sizes when parsing configuration
Julius Werner
jwerner at chromium.org
Sat Jul 13 02:30:16 CEST 2013
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 at chromium.org>
---
common/usb.c | 62 ++++++++++++++++++++++++++++++++++++++++++++------------
common/usb_hub.c | 14 ++++---------
2 files changed, 53 insertions(+), 23 deletions(-)
diff --git a/common/usb.c b/common/usb.c
index 55fff5b..4cf1d7a 100644
--- a/common/usb.c
+++ b/common/usb.c
@@ -341,6 +341,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 when it was read.)
*/
static int usb_parse_config(struct usb_device *dev,
unsigned char *buffer, int cfgno)
@@ -361,24 +362,34 @@ 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));
+ memcpy(&dev->config, buffer, 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 (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) {
/* 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, &buffer[index],
+ USB_DT_INTERFACE_SIZE);
if_desc->no_of_ep = 0;
if_desc->num_altsetting = 1;
curr_if_num =
@@ -392,12 +403,26 @@ static int usb_parse_config(struct usb_device *dev,
}
break;
case USB_DT_ENDPOINT:
+ 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]);
+ &buffer[index], USB_DT_ENDPOINT_SIZE);
ep_wMaxPacketSize = get_unaligned(&dev->config.\
if_desc[ifno].\
ep_desc[epno].\
@@ -410,9 +435,18 @@ 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 (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("EP SS 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]);
+ &buffer[index], USB_DT_SS_EP_COMP_SIZE);
break;
default:
if (head->bLength == 0)
@@ -491,7 +525,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];
@@ -505,16 +539,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;
}
diff --git a/common/usb_hub.c b/common/usb_hub.c
index 774ba63..d30962e 100644
--- a/common/usb_hub.c
+++ b/common/usb_hub.c
@@ -320,7 +320,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;
@@ -341,20 +341,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)),
--
1.7.12.4
More information about the U-Boot
mailing list