From patchwork Sat Jul 13 00:30:16 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Julius Werner X-Patchwork-Id: 258817 X-Patchwork-Delegate: marek.vasut@gmail.com Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from theia.denx.de (theia.denx.de [85.214.87.163]) by ozlabs.org (Postfix) with ESMTP id 69DBF2C036C for ; Sat, 13 Jul 2013 10:56:53 +1000 (EST) Received: from localhost (localhost [127.0.0.1]) by theia.denx.de (Postfix) with ESMTP id CEC234A02F; Sat, 13 Jul 2013 02:56:50 +0200 (CEST) X-Virus-Scanned: Debian amavisd-new at theia.denx.de Received: from theia.denx.de ([127.0.0.1]) by localhost (theia.denx.de [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id x7b1bcik5e1d; Sat, 13 Jul 2013 02:56:50 +0200 (CEST) Received: from theia.denx.de (localhost [127.0.0.1]) by theia.denx.de (Postfix) with ESMTP id CF65B4A028; Sat, 13 Jul 2013 02:56:47 +0200 (CEST) Received: from localhost (localhost [127.0.0.1]) by theia.denx.de (Postfix) with ESMTP id 525B64A028 for ; Sat, 13 Jul 2013 02:56:44 +0200 (CEST) X-Virus-Scanned: Debian amavisd-new at theia.denx.de Received: from theia.denx.de ([127.0.0.1]) by localhost (theia.denx.de [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id RNl90kAJ7SZi for ; Sat, 13 Jul 2013 02:56:39 +0200 (CEST) X-Greylist: delayed 1554 seconds by postgrey-1.27 at theia; Sat, 13 Jul 2013 02:56:33 CEST X-policyd-weight: NOT_IN_SBL_XBL_SPAMHAUS=-1.5 NOT_IN_SPAMCOP=-1.5 NOT_IN_BL_NJABL=-1.5 (only DNSBL check requested) Received: from mail-pb0-f46.google.com (mail-pb0-f46.google.com [209.85.160.46]) by theia.denx.de (Postfix) with ESMTPS id 359474A025 for ; Sat, 13 Jul 2013 02:56:33 +0200 (CEST) Received: by mail-pb0-f46.google.com with SMTP id rq2so9592153pbb.33 for ; Fri, 12 Jul 2013 17:56:31 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org; s=google; h=from:to:cc:subject:date:message-id:x-mailer; bh=+iYsR0lA8HMv0/fDMV0voIIQ4XVkQc2H1HffGzxxz6s=; b=Y1rV/0WzplM8eadpJFjRQp3QThdTksnRgaggSh2Ru0bBhuRpVec7k1qpScQiIfKNo1 upxFK4v0/d4mqAnByi21+6SwXj2wZvR0j1V/oaJhj1Ozr5ZhPg2/1UoyWDklemRQnJem ayvHb68CZSnQJiCEpUWMLGYR3eQgMvW0Vgs+M= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20120113; h=from:to:cc:subject:date:message-id:x-mailer:x-gm-message-state; bh=+iYsR0lA8HMv0/fDMV0voIIQ4XVkQc2H1HffGzxxz6s=; b=AJ8zP/u8qDG81w+RBZ7P4ha/+PJaTc+KFJo9Ulp0ymzYqKodWGWLIhhY5986BnAWaU m0egLNgODp1/8ylD9e4a78rZx3bvgiOrnU0Lqo1ywPJOmgxEa0/txfIT2PQ4xc/QSCaI /6PWIfr25ePMGhvJxS3m+me72VNfPeseq7i7omwo2FqxHZkrsHRjI+axgDYmMv1tq0S2 neUiGCJuwUN1xAmGmpuQzU+3+N184NBQ5pJgh8ztpOLLV9Mst6/DKQeETGu/W9PzCd+Z jLERgnjJB/mCnOzlo0+uUocHny+3v+IkecOkuU3zgHwavPwUDo/LQ7lKOVuKiFBDjhu/ hnIQ== X-Received: by 10.68.58.97 with SMTP id p1mr44567340pbq.144.1373675437755; Fri, 12 Jul 2013 17:30:37 -0700 (PDT) Received: from jwerner-linux.mtv.corp.google.com (jwerner-linux.mtv.corp.google.com [172.22.72.75]) by mx.google.com with ESMTPSA id jf4sm47687205pbb.19.2013.07.12.17.30.36 for (version=TLSv1.1 cipher=ECDHE-RSA-RC4-SHA bits=128/128); Fri, 12 Jul 2013 17:30:36 -0700 (PDT) From: Julius Werner To: Marek Vasut Date: Fri, 12 Jul 2013 17:30:16 -0700 Message-Id: <1373675416-21559-1-git-send-email-jwerner@chromium.org> X-Mailer: git-send-email 1.8.3 X-Gm-Message-State: ALoCoQkmZWUiXqkQs2e4OKFh/aBkeYxuO9ZIcou5sQhZdNMDQ6sofpcfctAE2Di1/HdhrG5N+tck Cc: U-Boot Mailing List , u-boot-review@google.com, Julius Werner Subject: [U-Boot] [PATCH] usb: Use well-known descriptor sizes when parsing configuration X-BeenThere: u-boot@lists.denx.de X-Mailman-Version: 2.1.11 Precedence: list List-Id: U-Boot discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Sender: u-boot-bounces@lists.denx.de Errors-To: u-boot-bounces@lists.denx.de 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 --- 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)),