Reviewed-by: Bret Barkelew <bret.barke...@microsoft.com>
- Bret ________________________________ From: Star Zeng <star.z...@intel.com> 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 to read hub desc REF: https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugzilla.tianocore.org%2Fshow_bug.cgi%3Fid%3D973&data=02%7C01%7Cbret.barkelew%40microsoft.com%7C80ae823f4723435b90f408d5da87c509%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C636655199021176324&sdata=zZ1NN9uNtJLLmGt6jXOKq7WeWIWJXDY8IgO09HDpsTU%3D&reserved=0 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 <jiewen....@intel.com> Cc: Ruiyu Ni <ruiyu...@intel.com> Cc: Bret Barkelew <bret.barke...@microsoft.com> Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Star Zeng <star.z...@intel.com> --- 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.<BR> +Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.<BR> 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.<BR> +Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.<BR> 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 _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
