From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=104.47.42.107; helo=nam03-by2-obe.outbound.protection.outlook.com; envelope-from=bret.barkelew@microsoft.com; receiver=edk2-devel@lists.01.org Received: from NAM03-BY2-obe.outbound.protection.outlook.com (mail-by2nam03on0107.outbound.protection.outlook.com [104.47.42.107]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-SHA384 (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id DCB4B202E53EA for ; Mon, 25 Jun 2018 10:25:58 -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=4QCmbXZu9ePssJW00okAAAtzMtFOPh01vD3cUNL6kcY=; b=FHcFih75xa48jg6/UKUcR9v0it3vuqQ4AfUq2ckztHX5ayvODFpgvyStg44C+mdZYJElGC6PhUIes48i6FskraD6Lm47MuTCA7yEqxHAOdq0U5+6uy5GNkqB9tispPomw+Ygxrq5yQrqqm/roWtZnUZH1bM4dEsS6KYDpx3Ij40= Received: from MWHPR21MB0784.namprd21.prod.outlook.com (10.173.51.150) by MWHPR21MB0127.namprd21.prod.outlook.com (10.173.52.9) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.930.4; Mon, 25 Jun 2018 17:25:57 +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:25:57 +0000 From: Bret Barkelew To: Star Zeng , "edk2-devel@lists.01.org" CC: Star Zeng , Jiewen Yao , Ruiyu Ni Thread-Topic: [PATCH 1/2] MdeModulePkg UsbBusDxe: Fix wrong buffer length used to read hub desc Thread-Index: AQHUDHCkrFBGPrciKEeKPF52PJJ2HaRxOiKJ Date: Mon, 25 Jun 2018 17:25:57 +0000 Message-ID: References: <1529923093-156972-1-git-send-email-star.zeng@intel.com>, <1529923093-156972-2-git-send-email-star.zeng@intel.com> In-Reply-To: <1529923093-156972-2-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; MWHPR21MB0127; 7:P8d45W/ETbTu+v2zEOo8T7SHs4hxpVXEFa00NAXs9IUDRCROZmLGYb7ZNDrSMTuJiejDuaTs18LUJA3QqhkT6ejq/+Ps2quc4C8ph9BmtQ+eRkileijwOhh2j6tAVRCf4QKNbbJe4lYPmkEhEZp4DvxDv7bRL6S/H23vMQ+SA7lyl59oD1zgnuhnUgmWXRHGj9VBx6gEM3O/8jRuvnMpjWiKCth8Tt50MkHuzHBVKOXmrWz123O51Ju2qDdNF4dl x-ms-exchange-antispam-srfa-diagnostics: SOS; x-ms-office365-filtering-correlation-id: 84583aab-60f6-4534-a52d-08d5dac0b673 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:MWHPR21MB0127; x-ms-traffictypediagnostic: MWHPR21MB0127: 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:MWHPR21MB0127; BCL:0; PCL:0; RULEID:; SRVR:MWHPR21MB0127; x-forefront-prvs: 0714841678 x-forefront-antispam-report: SFV:NSPM; SFS:(10019020)(136003)(39860400002)(396003)(346002)(376002)(366004)(189003)(199004)(6436002)(106356001)(9686003)(33656002)(74316002)(7736002)(6306002)(6346003)(26005)(86612001)(53546011)(6506007)(575784001)(102836004)(86362001)(186003)(2906002)(236005)(54896002)(55016002)(105586002)(25786009)(478600001)(7696005)(76176011)(14444005)(8990500004)(97736004)(81156014)(966005)(8936002)(81166006)(72206003)(14454004)(6116002)(3846002)(8676002)(10090500001)(256004)(99286004)(476003)(229853002)(606006)(10290500003)(68736007)(5660300001)(2900100001)(4326008)(66066001)(2501003)(5250100002)(53936002)(316002)(446003)(54906003)(11346002)(110136005)(22452003)(486006)(6246003)(19627235001); DIR:OUT; SFP:1102; SCL:1; SRVR:MWHPR21MB0127; 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: oNTHX9HlLVB5ISgxHnK5UWZVQWBLhuuBzJ76BGErfyaxk8rvULQmOLaxcfgW0hY0UyY8xw+T3GFfYlA97jpEkIudh2XDF/0OxT6fZWd9AQwt1eTF+dkKodNj292gU2dW559gtiAMG/wCn7KbMlW6YQ4D43RkP8dBtGZr4fj8h05FSIrsTsiCQNpz1jk8eRA7kEuNGxJnYcwvy1dk3jLeipojYSNNfKVTErpAoEufAduIqUHIm0oK/U4uETLGieW5zBYTdO1vQpeOi/LBIkWsDipJ5AX5yQUDEEas07uhgBRXiym/YIBxYJyRLFXbB5FcIkRUmFomb0IKxVKr5g3LiUpyIKm89+NKxypuEVRSosg= spamdiagnosticoutput: 1:99 spamdiagnosticmetadata: NSPM MIME-Version: 1.0 X-OriginatorOrg: microsoft.com X-MS-Exchange-CrossTenant-Network-Message-Id: 84583aab-60f6-4534-a52d-08d5dac0b673 X-MS-Exchange-CrossTenant-originalarrivaltime: 25 Jun 2018 17:25:57.1210 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: 72f988bf-86f1-41af-91ab-2d7cd011db47 X-MS-Exchange-Transport-CrossTenantHeadersStamped: MWHPR21MB0127 X-Content-Filtered-By: Mailman/MimeDel 2.1.26 Subject: Re: [PATCH 1/2] MdeModulePkg UsbBusDxe: 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:25:59 -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:12 AM To: edk2-devel@lists.01.org Cc: Star Zeng; Jiewen Yao; Ruiyu Ni; Bret Barkelew Subject: [PATCH 1/2] MdeModulePkg UsbBusDxe: 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%7Ca26f6d8c41174e8e643208d5da87c49e%7C72f988bf86f141af= 91ab2d7cd011db47%7C1%7C0%7C636655199033311517&sdata=3Df7QwxronZfoAs9C7B= LHTk9mAAiNP8ioYzKeoAbdU%2FUM%3D&reserved=3D0 HUB descriptor has variable length. But the code uses stack (HubDesc in UsbHubInit) with fixed length sizeof(EFI_USB_HUB_DESCRIPTOR) to hold HUB descriptor data. It uses hard code length value (32 that is greater than sizeof(EFI_USB_HUB_DESCRIPTOR)) for SuperSpeed path, then there will be stack overflow when IOMMU is enabled because the Unmap operation will copy the data from device buffer to host buffer. 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. The definition EFI_USB_SUPER_SPEED_HUB_DESCRIPTOR is wrong (HubDelay field should be UINT16 type) and no code is using it, the patch removes it. Cc: Jiewen Yao Cc: Ruiyu Ni Cc: Bret Barkelew Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Star Zeng --- MdeModulePkg/Bus/Usb/UsbBusDxe/UsbHub.c | 96 +++++++++++------------------= ---- MdeModulePkg/Bus/Usb/UsbBusDxe/UsbHub.h | 14 +---- 2 files changed, 32 insertions(+), 78 deletions(-) diff --git a/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbHub.c b/MdeModulePkg/Bus/Usb= /UsbBusDxe/UsbHub.c index fabb44157037..a962f76638e8 100644 --- a/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbHub.c +++ b/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbHub.c @@ -2,7 +2,7 @@ Unified interface for RootHub and Hub. -Copyright (c) 2007 - 2016, Intel Corporation. All rights reserved.
+Copyright (c) 2007 - 2018, Intel Corporation. All rights reserved.
This program and the accompanying materials are licensed and made available under the terms and conditions of the BSD = License which accompanies this distribution. The full text of the license may be = found at @@ -201,42 +201,7 @@ UsbHubCtrlClearTTBuffer ( } /** - Usb hub control transfer to get the super speed hub descriptor. - - @param HubDev The hub device. - @param Buf The buffer to hold the descriptor. - - @retval EFI_SUCCESS The hub descriptor is retrieved. - @retval Others Failed to retrieve the hub descriptor. - -**/ -EFI_STATUS -UsbHubCtrlGetSuperSpeedHubDesc ( - IN USB_DEVICE *HubDev, - OUT VOID *Buf - ) -{ - EFI_STATUS Status; - - Status =3D EFI_INVALID_PARAMETER; - - Status =3D UsbCtrlRequest ( - HubDev, - EfiUsbDataIn, - USB_REQ_TYPE_CLASS, - USB_HUB_TARGET_HUB, - USB_HUB_REQ_GET_DESC, - (UINT16) (USB_DESC_TYPE_HUB_SUPER_SPEED << 8), - 0, - Buf, - 32 - ); - - return Status; -} - -/** - Usb hub control transfer to get the hub descriptor. + Usb hub control transfer to get the (super speed) hub descriptor. @param HubDev The hub device. @param Buf The buffer to hold the descriptor. @@ -254,6 +219,11 @@ UsbHubCtrlGetHubDesc ( ) { EFI_STATUS Status; + UINT8 DescType; + + DescType =3D (HubDev->Speed =3D=3D EFI_USB_SPEED_SUPER) ? + USB_DESC_TYPE_HUB_SUPER_SPEED : + USB_DESC_TYPE_HUB; Status =3D UsbCtrlRequest ( HubDev, @@ -261,7 +231,7 @@ UsbHubCtrlGetHubDesc ( USB_REQ_TYPE_CLASS, USB_HUB_TARGET_HUB, USB_HUB_REQ_GET_DESC, - (UINT16) (USB_DESC_TYPE_HUB << 8), + (UINT16) (DescType << 8), 0, Buf, Len @@ -475,29 +445,19 @@ UsbHubReadDesc ( { EFI_STATUS Status; - if (HubDev->Speed =3D=3D EFI_USB_SPEED_SUPER) { - // - // Get the super speed hub descriptor - // - Status =3D UsbHubCtrlGetSuperSpeedHubDesc (HubDev, HubDesc); - } else { - - // - // First get the hub descriptor length - // - Status =3D UsbHubCtrlGetHubDesc (HubDev, HubDesc, 2); - - if (EFI_ERROR (Status)) { - return Status; - } + // + // First get the hub descriptor length + // + Status =3D UsbHubCtrlGetHubDesc (HubDev, HubDesc, 2); - // - // Get the whole hub descriptor - // - Status =3D UsbHubCtrlGetHubDesc (HubDev, HubDesc, HubDesc->Length); + if (EFI_ERROR (Status)) { + return Status; } - return Status; + // + // Get the whole hub descriptor + // + return UsbHubCtrlGetHubDesc (HubDev, HubDesc, HubDesc->Length); } @@ -690,7 +650,8 @@ UsbHubInit ( IN USB_INTERFACE *HubIf ) { - EFI_USB_HUB_DESCRIPTOR HubDesc; + UINT8 HubDescBuffer[256]; + EFI_USB_HUB_DESCRIPTOR *HubDesc; USB_ENDPOINT_DESC *EpDesc; USB_INTERFACE_SETTING *Setting; EFI_USB_IO_PROTOCOL *UsbIo; @@ -725,14 +686,19 @@ UsbHubInit ( return EFI_DEVICE_ERROR; } - Status =3D UsbHubReadDesc (HubDev, &HubDesc); + // + // The length field of descriptor is UINT8 type, so the buffer + // with 256 bytes is enough to hold the descriptor data. + // + HubDesc =3D (EFI_USB_HUB_DESCRIPTOR *) HubDescBuffer; + Status =3D UsbHubReadDesc (HubDev, HubDesc); if (EFI_ERROR (Status)) { DEBUG (( EFI_D_ERROR, "UsbHubInit: failed to read HUB descriptor %r\n"= , Status)); return Status; } - HubIf->NumOfPort =3D HubDesc.NumPorts; + HubIf->NumOfPort =3D HubDesc->NumPorts; DEBUG (( EFI_D_INFO, "UsbHubInit: hub %d has %d ports\n", HubDev->Addres= s,HubIf->NumOfPort)); @@ -751,7 +717,7 @@ UsbHubInit ( DEBUG ((EFI_D_INFO, "UsbHubInit: Set Hub Depth as 0x%x\n", Depth)); UsbHubCtrlSetHubDepth (HubIf->Device, Depth); - for (Index =3D 0; Index < HubDesc.NumPorts; Index++) { + for (Index =3D 0; Index < HubDesc->NumPorts; Index++) { UsbHubCtrlSetPortFeature (HubIf->Device, Index, USB_HUB_PORT_REMOTE_= WAKE_MASK); } } else { @@ -759,15 +725,15 @@ UsbHubInit ( // Feed power to all the hub ports. It should be ok // for both gang/individual powered hubs. // - for (Index =3D 0; Index < HubDesc.NumPorts; Index++) { + for (Index =3D 0; Index < HubDesc->NumPorts; Index++) { UsbHubCtrlSetPortFeature (HubIf->Device, Index, (EFI_USB_PORT_FEATUR= E) USB_HUB_PORT_POWER); } // // Update for the usb hub has no power on delay requirement // - if (HubDesc.PwrOn2PwrGood > 0) { - gBS->Stall (HubDesc.PwrOn2PwrGood * USB_SET_PORT_POWER_STALL); + if (HubDesc->PwrOn2PwrGood > 0) { + gBS->Stall (HubDesc->PwrOn2PwrGood * USB_SET_PORT_POWER_STALL); } UsbHubAckHubStatus (HubIf->Device); } diff --git a/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbHub.h b/MdeModulePkg/Bus/Usb= /UsbBusDxe/UsbHub.h index 4e5fcd85e0af..fe9f1f74c751 100644 --- a/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbHub.h +++ b/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbHub.h @@ -2,7 +2,7 @@ The definition for USB hub. -Copyright (c) 2007 - 2010, Intel Corporation. All rights reserved.
+Copyright (c) 2007 - 2018, Intel Corporation. All rights reserved.
This program and the accompanying materials are licensed and made available under the terms and conditions of the BSD = License which accompanies this distribution. The full text of the license may be = found at @@ -115,18 +115,6 @@ typedef struct { UINT8 Filler[16]; } EFI_USB_HUB_DESCRIPTOR; -typedef struct { - UINT8 Length; - UINT8 DescType; - UINT8 NumPorts; - UINT16 HubCharacter; - UINT8 PwrOn2PwrGood; - UINT8 HubContrCurrent; - UINT8 HubHdrDecLat; - UINT8 HubDelay; - UINT8 DeviceRemovable; -} EFI_USB_SUPER_SPEED_HUB_DESCRIPTOR; - #pragma pack() -- 2.7.0.windows.1