From: Star Zeng Date: Mon, 25 Jun 2018 08:50:01 +0000 (+0800) Subject: MdeModulePkg UsbBusPei: Fix wrong buffer length used to read hub desc X-Git-Tag: edk2-stable201903~1527 X-Git-Url: https://git.proxmox.com/?p=mirror_edk2.git;a=commitdiff_plain;h=72750e3bf9174f15c17e78f0f117b5e7311bb49f MdeModulePkg UsbBusPei: Fix wrong buffer length used to read hub desc 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 Reviewed-by: Bret Barkelew --- diff --git a/MdeModulePkg/Bus/Usb/UsbBusPei/HubPeim.c b/MdeModulePkg/Bus/Usb/UsbBusPei/HubPeim.c index 16a7b589c1..ac930ee8ea 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,15 +424,21 @@ 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 // @@ -484,13 +446,13 @@ PeiDoHubConfig ( 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 f50bc63501..341f6f32e3 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