From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=104.47.37.112; helo=nam02-cy1-obe.outbound.protection.outlook.com; envelope-from=bret.barkelew@microsoft.com; receiver=edk2-devel@lists.01.org Received: from NAM02-CY1-obe.outbound.protection.outlook.com (mail-cys01nam02on0112.outbound.protection.outlook.com [104.47.37.112]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-SHA384 (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 0A492202E53EA for ; Mon, 25 Jun 2018 10:27:09 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=selector1; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=X6MM6ZzDHjYQZrL9Xbav0wh1hU/t+hCx+AtpFhfA3Uk=; b=W7Wkaky3qWbgVlT9I1rWZtHOBlhXsUFAsu9Ll6UFZJeqt+Cm04w6r4hCqbbUHU9h+iJzNwhm6X1ztW19hcq164Un0Ilxl5RuarvIOuQRb45/Q6pZSp6S3xP9un7gx4GEZYDXfUsXMyYekxu0KsEFDuOgm3gxIkVJud87sQud9tk= Received: from MWHPR21MB0784.namprd21.prod.outlook.com (10.173.51.150) by MWHPR21MB0640.namprd21.prod.outlook.com (10.175.141.141) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.930.2; Mon, 25 Jun 2018 17:27:08 +0000 Received: from MWHPR21MB0784.namprd21.prod.outlook.com ([fe80::7885:2855:3f6e:e4c7]) by MWHPR21MB0784.namprd21.prod.outlook.com ([fe80::7885:2855:3f6e:e4c7%3]) with mapi id 15.20.0930.005; Mon, 25 Jun 2018 17:27:08 +0000 From: Bret Barkelew To: Star Zeng , "edk2-devel@lists.01.org" CC: Star Zeng , Jiewen Yao , Ruiyu Ni Thread-Topic: [PATCH 2/2] MdeModulePkg UsbBusPei: Fix wrong buffer length used to read hub desc Thread-Index: AQHUDHCja3yajtemikKT/mUZ/Re0YKRxOokw Date: Mon, 25 Jun 2018 17:27:07 +0000 Message-ID: References: <1529923093-156972-1-git-send-email-star.zeng@intel.com>, <1529923093-156972-3-git-send-email-star.zeng@intel.com> In-Reply-To: <1529923093-156972-3-git-send-email-star.zeng@intel.com> Accept-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [131.107.32.41] x-ms-publictraffictype: Email x-microsoft-exchange-diagnostics: 1; MWHPR21MB0640; 7:Tvj3ZY1MZmQ9nrc2Gk9xg8cF7+SlHYWiAC1Iwsvo6kVq6Jo1fo7PjvV9qIfeIlwHaFyE+9Aus7FkHOdXv9j0oKyPu/huFDQTHnPQMeE2f++bLmQz7Eizen0tbKC4U0X9vl8wMpDstveu7EXmna6jgZVlUBnNTM9tI3sOt2s6sNghjL7BCUdRgWSKjNAR3XrCH4WOMtqADJ54iK33B5HD/jZvp1IAuSgAa/1ZjNUlcTwPKNoBLcaXfmCXv/NLdcC6 x-ms-exchange-antispam-srfa-diagnostics: SOS; x-ms-office365-filtering-correlation-id: 731b97ca-7a88-45b0-e5c2-08d5dac0e0a6 x-ms-office365-filtering-ht: Tenant x-microsoft-antispam: UriScan:; BCL:0; PCL:0; RULEID:(7020095)(4652020)(8989117)(4534165)(4627221)(201703031133081)(201702281549075)(8990107)(5600026)(711020)(48565401081)(2017052603328)(7193020); SRVR:MWHPR21MB0640; x-ms-traffictypediagnostic: MWHPR21MB0640: x-o365ent-eop-header: Message processed by - O365_ENT: Allow from ranges (Engineering ONLY) x-microsoft-antispam-prvs: x-exchange-antispam-report-test: UriScan:(28532068793085)(158342451672863)(89211679590171)(189930954265078)(162533806227266)(219752817060721)(228905959029699); x-ms-exchange-senderadcheck: 1 x-exchange-antispam-report-cfa-test: BCL:0; PCL:0; RULEID:(8211001083)(6040522)(2401047)(5005006)(8121501046)(3002001)(93006095)(93001095)(10201501046)(3231254)(2018427008)(944501410)(52105095)(6055026)(149027)(150027)(6041310)(201703131423095)(201702281528075)(20161123555045)(201703061421075)(201703061406153)(20161123560045)(20161123562045)(20161123558120)(20161123564045)(6072148)(201708071742011)(7699016); SRVR:MWHPR21MB0640; BCL:0; PCL:0; RULEID:; SRVR:MWHPR21MB0640; x-forefront-prvs: 0714841678 x-forefront-antispam-report: SFV:NSPM; SFS:(10019020)(346002)(376002)(136003)(39860400002)(396003)(366004)(189003)(199004)(8990500004)(229853002)(33656002)(110136005)(74316002)(25786009)(6246003)(6306002)(9686003)(54896002)(236005)(6436002)(68736007)(8936002)(55016002)(606006)(10090500001)(53936002)(4326008)(316002)(22452003)(81166006)(105586002)(81156014)(106356001)(5250100002)(2501003)(7736002)(54906003)(8676002)(256004)(14454004)(102836004)(476003)(478600001)(53546011)(99286004)(966005)(446003)(76176011)(66066001)(486006)(2900100001)(10290500003)(6506007)(72206003)(86612001)(2906002)(5660300001)(86362001)(3846002)(6116002)(186003)(6346003)(7696005)(26005)(11346002)(14444005)(97736004)(19627235001); DIR:OUT; SFP:1102; SCL:1; SRVR:MWHPR21MB0640; H:MWHPR21MB0784.namprd21.prod.outlook.com; FPR:; SPF:None; LANG:en; PTR:InfoNoRecords; MX:1; A:1; received-spf: None (protection.outlook.com: microsoft.com does not designate permitted sender hosts) authentication-results: spf=none (sender IP is ) smtp.mailfrom=Bret.Barkelew@microsoft.com; x-microsoft-antispam-message-info: sHZrfzJvtN2ULubngGK28oAsXvJ5zKuf2le3efl8Li1kntJCS9gkfzM6CoPhEV/JC82lwweuv8obcD7uR/2HO7LvyPVFCDul8jXL16BhUljBfEy9/+6TTLi86qrfzuZ7HmdbDE+AM0hRvxtS8idv7b3ornk0Hf5UCKfDM++fkuWH4dDsBYegAj6pyIpU5BkweitQDN3xlFYqFB+Z5og+mpXVxBqNTqpNh4kFsBHEsAKB3KzRhdd6n6dPaVlEL3GEjDZCSI5ZqNKvXchJqBYymZRJzlVqbPpF2snXR2xdZxu9uT0kgwzR+JBsGh3nxm5ZDOtyJvTIuR9Iz/e4JH0WlWuAPg1zVweZCZxFpKJaOVY= spamdiagnosticoutput: 1:99 spamdiagnosticmetadata: NSPM MIME-Version: 1.0 X-OriginatorOrg: microsoft.com X-MS-Exchange-CrossTenant-Network-Message-Id: 731b97ca-7a88-45b0-e5c2-08d5dac0e0a6 X-MS-Exchange-CrossTenant-originalarrivaltime: 25 Jun 2018 17:27:07.9252 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: 72f988bf-86f1-41af-91ab-2d7cd011db47 X-MS-Exchange-Transport-CrossTenantHeadersStamped: MWHPR21MB0640 X-Content-Filtered-By: Mailman/MimeDel 2.1.26 Subject: Re: [PATCH 2/2] MdeModulePkg UsbBusPei: Fix wrong buffer length used to read hub desc X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.26 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 25 Jun 2018 17:27:10 -0000 Content-Language: en-US Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Reviewed-by: Bret Barkelew - Bret ________________________________ From: Star Zeng Sent: Monday, June 25, 2018 3:38:13 AM To: edk2-devel@lists.01.org Cc: Star Zeng; Jiewen Yao; Ruiyu Ni; Bret Barkelew Subject: [PATCH 2/2] MdeModulePkg UsbBusPei: Fix wrong buffer length used t= o read hub desc REF: https://na01.safelinks.protection.outlook.com/?url=3Dhttps%3A%2F%2Fbug= zilla.tianocore.org%2Fshow_bug.cgi%3Fid%3D973&data=3D02%7C01%7Cbret.bar= kelew%40microsoft.com%7C80ae823f4723435b90f408d5da87c509%7C72f988bf86f141af= 91ab2d7cd011db47%7C1%7C0%7C636655199021176324&sdata=3DzZ1NN9uNtJLLmGt6j= XOKq7WeWIWJXDY8IgO09HDpsTU%3D&reserved=3D0 Bug 973 just mentions UsbBusDxe, but UsbBusPei has similar issue. HUB descriptor has variable length. But the code uses stack (HubDescriptor in PeiDoHubConfig) with fixed length sizeof(EFI_USB_HUB_DESCRIPTOR) to hold HUB descriptor data. It uses hard code length value (12) for SuperSpeed path. And it uses HubDesc->Length for none SuperSpeed path, then there will be stack overflow when HubDesc->Length is greater than sizeof(EFI_USB_HUB_DESCRIPTOR). The patch updates the code to use a big enough buffer to hold the descriptor data. Cc: Jiewen Yao Cc: Ruiyu Ni Cc: Bret Barkelew Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Star Zeng --- MdeModulePkg/Bus/Usb/UsbBusPei/HubPeim.c | 108 ++++++++++-----------------= ---- MdeModulePkg/Bus/Usb/UsbBusPei/HubPeim.h | 4 +- 2 files changed, 38 insertions(+), 74 deletions(-) diff --git a/MdeModulePkg/Bus/Usb/UsbBusPei/HubPeim.c b/MdeModulePkg/Bus/Us= b/UsbBusPei/HubPeim.c index 16a7b589c1c2..ac930ee8ea00 100644 --- a/MdeModulePkg/Bus/Usb/UsbBusPei/HubPeim.c +++ b/MdeModulePkg/Bus/Usb/UsbBusPei/HubPeim.c @@ -1,7 +1,7 @@ /** @file Usb Hub Request Support In PEI Phase -Copyright (c) 2006 - 2014, Intel Corporation. All rights reserved.
+Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.
This program and the accompanying materials are licensed and made available under the terms and conditions @@ -276,9 +276,10 @@ PeiHubClearHubFeature ( } /** - Get a given hub descriptor. + Get a given (SuperSpeed) hub descriptor. @param PeiServices General-purpose services that are available to ev= ery PEIM. + @param PeiUsbDevice Indicates the hub controller device. @param UsbIoPpi Indicates the PEI_USB_IO_PPI instance. @param DescriptorSize The length of Hub Descriptor buffer. @param HubDescriptor Caller allocated buffer to store the hub descript= or if @@ -292,63 +293,28 @@ PeiHubClearHubFeature ( EFI_STATUS PeiGetHubDescriptor ( IN EFI_PEI_SERVICES **PeiServices, + IN PEI_USB_DEVICE *PeiUsbDevice, IN PEI_USB_IO_PPI *UsbIoPpi, IN UINTN DescriptorSize, OUT EFI_USB_HUB_DESCRIPTOR *HubDescriptor ) { EFI_USB_DEVICE_REQUEST DevReq; - ZeroMem (&DevReq, sizeof (EFI_USB_DEVICE_REQUEST)); - - // - // Fill Device request packet - // - DevReq.RequestType =3D USB_RT_HUB | 0x80; - DevReq.Request =3D USB_HUB_GET_DESCRIPTOR; - DevReq.Value =3D USB_DT_HUB << 8; - DevReq.Length =3D (UINT16)DescriptorSize; - - return UsbIoPpi->UsbControlTransfer ( - PeiServices, - UsbIoPpi, - &DevReq, - EfiUsbDataIn, - PcdGet32 (PcdUsbTransferTimeoutValue), - HubDescriptor, - (UINT16)DescriptorSize - ); -} - -/** - Get a given SuperSpeed hub descriptor. + UINT8 DescType; - @param PeiServices General-purpose services that are available to= every PEIM. - @param UsbIoPpi Indicates the PEI_USB_IO_PPI instance. - @param HubDescriptor Caller allocated buffer to store the hub descr= iptor if - successfully returned. - - @retval EFI_SUCCESS Hub descriptor is obtained successfully. - @retval EFI_DEVICE_ERROR Cannot get the hub descriptor due to a hardwar= e error. - @retval Others Other failure occurs. - -**/ -EFI_STATUS -PeiGetSuperSpeedHubDesc ( - IN EFI_PEI_SERVICES **PeiServices, - IN PEI_USB_IO_PPI *UsbIoPpi, - OUT EFI_USB_HUB_DESCRIPTOR *HubDescriptor - ) -{ - EFI_USB_DEVICE_REQUEST DevReq; ZeroMem (&DevReq, sizeof (EFI_USB_DEVICE_REQUEST)); + DescType =3D (PeiUsbDevice->DeviceSpeed =3D=3D EFI_USB_SPEED_SUPER) ? + USB_DT_SUPERSPEED_HUB : + USB_DT_HUB; + // // Fill Device request packet // DevReq.RequestType =3D USB_RT_HUB | 0x80; DevReq.Request =3D USB_HUB_GET_DESCRIPTOR; - DevReq.Value =3D USB_DT_SUPERSPEED_HUB << 8; - DevReq.Length =3D 12; + DevReq.Value =3D (UINT16) (DescType << 8); + DevReq.Length =3D (UINT16) DescriptorSize; return UsbIoPpi->UsbControlTransfer ( PeiServices, @@ -357,7 +323,7 @@ PeiGetSuperSpeedHubDesc ( EfiUsbDataIn, PcdGet32 (PcdUsbTransferTimeoutValue), HubDescriptor, - 12 + (UINT16)DescriptorSize ); } @@ -387,29 +353,19 @@ PeiUsbHubReadDesc ( { EFI_STATUS Status; - if (PeiUsbDevice->DeviceSpeed =3D=3D EFI_USB_SPEED_SUPER) { - // - // Get the super speed hub descriptor - // - Status =3D PeiGetSuperSpeedHubDesc (PeiServices, UsbIoPpi, HubDescript= or); - } else { - - // - // First get the hub descriptor length - // - Status =3D PeiGetHubDescriptor (PeiServices, UsbIoPpi, 2, HubDescripto= r); - - if (EFI_ERROR (Status)) { - return Status; - } + // + // First get the hub descriptor length + // + Status =3D PeiGetHubDescriptor (PeiServices, PeiUsbDevice, UsbIoPpi, 2, = HubDescriptor); - // - // Get the whole hub descriptor - // - Status =3D PeiGetHubDescriptor (PeiServices, UsbIoPpi, HubDescriptor->= Length, HubDescriptor); + if (EFI_ERROR (Status)) { + return Status; } - return Status; + // + // Get the whole hub descriptor + // + return PeiGetHubDescriptor (PeiServices, PeiUsbDevice, UsbIoPpi, HubDesc= riptor->Length, HubDescriptor); } /** @@ -468,29 +424,35 @@ PeiDoHubConfig ( IN PEI_USB_DEVICE *PeiUsbDevice ) { - EFI_USB_HUB_DESCRIPTOR HubDescriptor; + UINT8 HubDescBuffer[256]; + EFI_USB_HUB_DESCRIPTOR *HubDescriptor; EFI_STATUS Status; EFI_USB_HUB_STATUS HubStatus; UINTN Index; PEI_USB_IO_PPI *UsbIoPpi; - ZeroMem (&HubDescriptor, sizeof (HubDescriptor)); UsbIoPpi =3D &PeiUsbDevice->UsbIoPpi; // + // The length field of descriptor is UINT8 type, so the buffer + // with 256 bytes is enough to hold the descriptor data. + // + HubDescriptor =3D (EFI_USB_HUB_DESCRIPTOR *) HubDescBuffer; + + // // Get the hub descriptor // Status =3D PeiUsbHubReadDesc ( PeiServices, PeiUsbDevice, UsbIoPpi, - &HubDescriptor + HubDescriptor ); if (EFI_ERROR (Status)) { return EFI_DEVICE_ERROR; } - PeiUsbDevice->DownStreamPortNo =3D HubDescriptor.NbrPorts; + PeiUsbDevice->DownStreamPortNo =3D HubDescriptor->NbrPorts; if (PeiUsbDevice->DeviceSpeed =3D=3D EFI_USB_SPEED_SUPER) { DEBUG ((EFI_D_INFO, "PeiDoHubConfig: Set Hub Depth as 0x%x\n", PeiUsbD= evice->Tier)); @@ -516,9 +478,9 @@ PeiDoHubConfig ( } } - DEBUG (( EFI_D_INFO, "PeiDoHubConfig: HubDescriptor.PwrOn2PwrGood: 0x%= x\n", HubDescriptor.PwrOn2PwrGood)); - if (HubDescriptor.PwrOn2PwrGood > 0) { - MicroSecondDelay (HubDescriptor.PwrOn2PwrGood * USB_SET_PORT_POWER_S= TALL); + DEBUG (( EFI_D_INFO, "PeiDoHubConfig: HubDescriptor.PwrOn2PwrGood: 0x%= x\n", HubDescriptor->PwrOn2PwrGood)); + if (HubDescriptor->PwrOn2PwrGood > 0) { + MicroSecondDelay (HubDescriptor->PwrOn2PwrGood * USB_SET_PORT_POWER_= STALL); } // diff --git a/MdeModulePkg/Bus/Usb/UsbBusPei/HubPeim.h b/MdeModulePkg/Bus/Us= b/UsbBusPei/HubPeim.h index f50bc6350156..341f6f32e3d0 100644 --- a/MdeModulePkg/Bus/Usb/UsbBusPei/HubPeim.h +++ b/MdeModulePkg/Bus/Usb/UsbBusPei/HubPeim.h @@ -1,7 +1,7 @@ /** @file Constants definitions for Usb Hub Peim -Copyright (c) 2006 - 2014, Intel Corporation. All rights reserved.
+Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.
This program and the accompanying materials are licensed and made available under the terms and conditions @@ -227,6 +227,7 @@ PeiHubClearHubFeature ( Get a given hub descriptor. @param PeiServices General-purpose services that are available to ev= ery PEIM. + @param PeiUsbDevice Indicates the hub controller device. @param UsbIoPpi Indicates the PEI_USB_IO_PPI instance. @param DescriptorSize The length of Hub Descriptor buffer. @param HubDescriptor Caller allocated buffer to store the hub descript= or if @@ -240,6 +241,7 @@ PeiHubClearHubFeature ( EFI_STATUS PeiGetHubDescriptor ( IN EFI_PEI_SERVICES **PeiServices, + IN PEI_USB_DEVICE *PeiUsbDevice, IN PEI_USB_IO_PPI *UsbIoPpi, IN UINTN DescriptorSize, OUT EFI_USB_HUB_DESCRIPTOR *HubDescriptor -- 2.7.0.windows.1