From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=134.134.136.126; helo=mga18.intel.com; envelope-from=star.zeng@intel.com; receiver=edk2-devel@lists.01.org Received: from mga18.intel.com (mga18.intel.com [134.134.136.126]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 182422096FACE for ; Mon, 25 Jun 2018 03:38:20 -0700 (PDT) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga003.jf.intel.com ([10.7.209.27]) by orsmga106.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 25 Jun 2018 03:38:19 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.51,270,1526367600"; d="scan'208";a="61964527" Received: from shwdeopenpsi068.ccr.corp.intel.com ([10.239.158.46]) by orsmga003.jf.intel.com with ESMTP; 25 Jun 2018 03:38:17 -0700 From: Star Zeng To: edk2-devel@lists.01.org Cc: Star Zeng , Jiewen Yao , Ruiyu Ni , Bret Barkelew Date: Mon, 25 Jun 2018 18:38:13 +0800 Message-Id: <1529923093-156972-3-git-send-email-star.zeng@intel.com> X-Mailer: git-send-email 2.7.0.windows.1 In-Reply-To: <1529923093-156972-1-git-send-email-star.zeng@intel.com> References: <1529923093-156972-1-git-send-email-star.zeng@intel.com> Subject: [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 10:38:20 -0000 REF: https://bugzilla.tianocore.org/show_bug.cgi?id=973 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/Usb/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 every 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 descriptor 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 = USB_RT_HUB | 0x80; - DevReq.Request = USB_HUB_GET_DESCRIPTOR; - DevReq.Value = USB_DT_HUB << 8; - DevReq.Length = (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 descriptor if - successfully returned. - - @retval EFI_SUCCESS Hub descriptor is obtained successfully. - @retval EFI_DEVICE_ERROR Cannot get the hub descriptor due to a hardware 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 = (PeiUsbDevice->DeviceSpeed == EFI_USB_SPEED_SUPER) ? + USB_DT_SUPERSPEED_HUB : + USB_DT_HUB; + // // Fill Device request packet // DevReq.RequestType = USB_RT_HUB | 0x80; DevReq.Request = USB_HUB_GET_DESCRIPTOR; - DevReq.Value = USB_DT_SUPERSPEED_HUB << 8; - DevReq.Length = 12; + DevReq.Value = (UINT16) (DescType << 8); + DevReq.Length = (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 == EFI_USB_SPEED_SUPER) { - // - // Get the super speed hub descriptor - // - Status = PeiGetSuperSpeedHubDesc (PeiServices, UsbIoPpi, HubDescriptor); - } else { - - // - // First get the hub descriptor length - // - Status = PeiGetHubDescriptor (PeiServices, UsbIoPpi, 2, HubDescriptor); - - if (EFI_ERROR (Status)) { - return Status; - } + // + // First get the hub descriptor length + // + Status = PeiGetHubDescriptor (PeiServices, PeiUsbDevice, UsbIoPpi, 2, HubDescriptor); - // - // Get the whole hub descriptor - // - Status = PeiGetHubDescriptor (PeiServices, UsbIoPpi, HubDescriptor->Length, HubDescriptor); + if (EFI_ERROR (Status)) { + return Status; } - return Status; + // + // Get the whole hub descriptor + // + return PeiGetHubDescriptor (PeiServices, PeiUsbDevice, UsbIoPpi, HubDescriptor->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 = &PeiUsbDevice->UsbIoPpi; // + // The length field of descriptor is UINT8 type, so the buffer + // with 256 bytes is enough to hold the descriptor data. + // + HubDescriptor = (EFI_USB_HUB_DESCRIPTOR *) HubDescBuffer; + + // // Get the hub descriptor // Status = PeiUsbHubReadDesc ( PeiServices, PeiUsbDevice, UsbIoPpi, - &HubDescriptor + HubDescriptor ); if (EFI_ERROR (Status)) { return EFI_DEVICE_ERROR; } - PeiUsbDevice->DownStreamPortNo = HubDescriptor.NbrPorts; + PeiUsbDevice->DownStreamPortNo = HubDescriptor->NbrPorts; if (PeiUsbDevice->DeviceSpeed == EFI_USB_SPEED_SUPER) { DEBUG ((EFI_D_INFO, "PeiDoHubConfig: Set Hub Depth as 0x%x\n", PeiUsbDevice->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_STALL); + 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/Usb/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 every 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 descriptor 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