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 <jiewen.yao@intel.com>
Cc: Ruiyu Ni <ruiyu.ni@intel.com>
Cc: Bret Barkelew <bret.barkelew@microsoft.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Star Zeng <star.zeng@intel.com>
Reviewed-by: Bret Barkelew <bret.barkelew@microsoft.com>

diff --git a/MdeModulePkg/Bus/Usb/UsbBusPei/HubPeim.c b/MdeModulePkg/Bus/Usb/UsbBusPei/HubPeim.c
index 16a7b589c1c2ca66ba58b16cc87a8fb6ee269242..ac930ee8ea0046615108aad77762141748306404 100644 (file)
--- a/MdeModulePkg/Bus/Usb/UsbBusPei/HubPeim.c
+++ b/MdeModulePkg/Bus/Usb/UsbBusPei/HubPeim.c
@@ -1,7 +1,7 @@
 /** @file\r
 Usb Hub Request Support In PEI Phase\r
 \r
-Copyright (c) 2006 - 2014, Intel Corporation. All rights reserved.<BR>\r
+Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.<BR>\r
   \r
 This program and the accompanying materials\r
 are licensed and made available under the terms and conditions\r
@@ -276,9 +276,10 @@ PeiHubClearHubFeature (
 }\r
 \r
 /**\r
-  Get a given hub descriptor.\r
+  Get a given (SuperSpeed) hub descriptor.\r
 \r
   @param  PeiServices    General-purpose services that are available to every PEIM.\r
+  @param  PeiUsbDevice   Indicates the hub controller device.\r
   @param  UsbIoPpi       Indicates the PEI_USB_IO_PPI instance.\r
   @param  DescriptorSize The length of Hub Descriptor buffer.\r
   @param  HubDescriptor  Caller allocated buffer to store the hub descriptor if\r
@@ -292,63 +293,28 @@ PeiHubClearHubFeature (
 EFI_STATUS\r
 PeiGetHubDescriptor (\r
   IN  EFI_PEI_SERVICES          **PeiServices,\r
+  IN  PEI_USB_DEVICE            *PeiUsbDevice,\r
   IN  PEI_USB_IO_PPI            *UsbIoPpi,\r
   IN  UINTN                     DescriptorSize,\r
   OUT EFI_USB_HUB_DESCRIPTOR    *HubDescriptor\r
   )\r
 {\r
   EFI_USB_DEVICE_REQUEST      DevReq;\r
-  ZeroMem (&DevReq, sizeof (EFI_USB_DEVICE_REQUEST));\r
-\r
-  //\r
-  // Fill Device request packet\r
-  //\r
-  DevReq.RequestType = USB_RT_HUB | 0x80;\r
-  DevReq.Request     = USB_HUB_GET_DESCRIPTOR;\r
-  DevReq.Value       = USB_DT_HUB << 8;\r
-  DevReq.Length      = (UINT16)DescriptorSize;\r
-\r
-  return  UsbIoPpi->UsbControlTransfer (\r
-                      PeiServices,\r
-                      UsbIoPpi,\r
-                      &DevReq,\r
-                      EfiUsbDataIn,\r
-                      PcdGet32 (PcdUsbTransferTimeoutValue),\r
-                      HubDescriptor,\r
-                      (UINT16)DescriptorSize\r
-                      );\r
-}\r
-\r
-/**\r
-  Get a given SuperSpeed hub descriptor.\r
+  UINT8                       DescType;\r
 \r
-  @param  PeiServices       General-purpose services that are available to every PEIM.\r
-  @param  UsbIoPpi          Indicates the PEI_USB_IO_PPI instance.\r
-  @param  HubDescriptor     Caller allocated buffer to store the hub descriptor if\r
-                            successfully returned.\r
-\r
-  @retval EFI_SUCCESS       Hub descriptor is obtained successfully.\r
-  @retval EFI_DEVICE_ERROR  Cannot get the hub descriptor due to a hardware error.\r
-  @retval Others            Other failure occurs.\r
-\r
-**/\r
-EFI_STATUS\r
-PeiGetSuperSpeedHubDesc (\r
-  IN  EFI_PEI_SERVICES          **PeiServices,\r
-  IN  PEI_USB_IO_PPI            *UsbIoPpi,\r
-  OUT EFI_USB_HUB_DESCRIPTOR    *HubDescriptor\r
-  )\r
-{\r
-  EFI_USB_DEVICE_REQUEST        DevReq;\r
   ZeroMem (&DevReq, sizeof (EFI_USB_DEVICE_REQUEST));\r
 \r
+  DescType = (PeiUsbDevice->DeviceSpeed == EFI_USB_SPEED_SUPER) ?\r
+             USB_DT_SUPERSPEED_HUB :\r
+             USB_DT_HUB;\r
+\r
   //\r
   // Fill Device request packet\r
   //\r
   DevReq.RequestType = USB_RT_HUB | 0x80;\r
   DevReq.Request     = USB_HUB_GET_DESCRIPTOR;\r
-  DevReq.Value       = USB_DT_SUPERSPEED_HUB << 8;\r
-  DevReq.Length      = 12;\r
+  DevReq.Value       = (UINT16) (DescType << 8);\r
+  DevReq.Length      = (UINT16) DescriptorSize;\r
 \r
   return  UsbIoPpi->UsbControlTransfer (\r
                       PeiServices,\r
@@ -357,7 +323,7 @@ PeiGetSuperSpeedHubDesc (
                       EfiUsbDataIn,\r
                       PcdGet32 (PcdUsbTransferTimeoutValue),\r
                       HubDescriptor,\r
-                      12\r
+                      (UINT16)DescriptorSize\r
                       );\r
 }\r
 \r
@@ -387,29 +353,19 @@ PeiUsbHubReadDesc (
 {\r
   EFI_STATUS Status;\r
 \r
-  if (PeiUsbDevice->DeviceSpeed == EFI_USB_SPEED_SUPER) {\r
-    //\r
-    // Get the super speed hub descriptor\r
-    //\r
-    Status = PeiGetSuperSpeedHubDesc (PeiServices, UsbIoPpi, HubDescriptor);\r
-  } else {\r
-\r
-    //\r
-    // First get the hub descriptor length\r
-    //\r
-    Status = PeiGetHubDescriptor (PeiServices, UsbIoPpi, 2, HubDescriptor);\r
-\r
-    if (EFI_ERROR (Status)) {\r
-      return Status;\r
-    }\r
+  //\r
+  // First get the hub descriptor length\r
+  //\r
+  Status = PeiGetHubDescriptor (PeiServices, PeiUsbDevice, UsbIoPpi, 2, HubDescriptor);\r
 \r
-    //\r
-    // Get the whole hub descriptor\r
-    //\r
-    Status = PeiGetHubDescriptor (PeiServices, UsbIoPpi, HubDescriptor->Length, HubDescriptor);\r
+  if (EFI_ERROR (Status)) {\r
+    return Status;\r
   }\r
 \r
-  return Status;\r
+  //\r
+  // Get the whole hub descriptor\r
+  //\r
+  return PeiGetHubDescriptor (PeiServices, PeiUsbDevice, UsbIoPpi, HubDescriptor->Length, HubDescriptor);\r
 }\r
 \r
 /**\r
@@ -468,15 +424,21 @@ PeiDoHubConfig (
   IN PEI_USB_DEVICE      *PeiUsbDevice\r
   )\r
 {\r
-  EFI_USB_HUB_DESCRIPTOR  HubDescriptor;\r
+  UINT8                   HubDescBuffer[256];\r
+  EFI_USB_HUB_DESCRIPTOR  *HubDescriptor;\r
   EFI_STATUS              Status;\r
   EFI_USB_HUB_STATUS      HubStatus;\r
   UINTN                   Index;\r
   PEI_USB_IO_PPI          *UsbIoPpi;\r
 \r
-  ZeroMem (&HubDescriptor, sizeof (HubDescriptor));\r
   UsbIoPpi = &PeiUsbDevice->UsbIoPpi;\r
 \r
+  //\r
+  // The length field of descriptor is UINT8 type, so the buffer\r
+  // with 256 bytes is enough to hold the descriptor data.\r
+  //\r
+  HubDescriptor = (EFI_USB_HUB_DESCRIPTOR *) HubDescBuffer;\r
+\r
   //\r
   // Get the hub descriptor \r
   //\r
@@ -484,13 +446,13 @@ PeiDoHubConfig (
             PeiServices,\r
             PeiUsbDevice,\r
             UsbIoPpi,\r
-            &HubDescriptor\r
+            HubDescriptor\r
             );\r
   if (EFI_ERROR (Status)) {\r
     return EFI_DEVICE_ERROR;\r
   }\r
 \r
-  PeiUsbDevice->DownStreamPortNo = HubDescriptor.NbrPorts;\r
+  PeiUsbDevice->DownStreamPortNo = HubDescriptor->NbrPorts;\r
 \r
   if (PeiUsbDevice->DeviceSpeed == EFI_USB_SPEED_SUPER) {\r
     DEBUG ((EFI_D_INFO, "PeiDoHubConfig: Set Hub Depth as 0x%x\n", PeiUsbDevice->Tier));\r
@@ -516,9 +478,9 @@ PeiDoHubConfig (
       }\r
     }\r
 \r
-    DEBUG (( EFI_D_INFO, "PeiDoHubConfig: HubDescriptor.PwrOn2PwrGood: 0x%x\n", HubDescriptor.PwrOn2PwrGood));\r
-    if (HubDescriptor.PwrOn2PwrGood > 0) {\r
-      MicroSecondDelay (HubDescriptor.PwrOn2PwrGood * USB_SET_PORT_POWER_STALL);\r
+    DEBUG (( EFI_D_INFO, "PeiDoHubConfig: HubDescriptor.PwrOn2PwrGood: 0x%x\n", HubDescriptor->PwrOn2PwrGood));\r
+    if (HubDescriptor->PwrOn2PwrGood > 0) {\r
+      MicroSecondDelay (HubDescriptor->PwrOn2PwrGood * USB_SET_PORT_POWER_STALL);\r
     }\r
 \r
     //\r

diff --git a/MdeModulePkg/Bus/Usb/UsbBusPei/HubPeim.h b/MdeModulePkg/Bus/Usb/UsbBusPei/HubPeim.h
index f50bc63501569a9b83664195f87c8b183911671c..341f6f32e3d0530dfa7a6870cc226f132dd442c9 100644 (file)
--- a/MdeModulePkg/Bus/Usb/UsbBusPei/HubPeim.h
+++ b/MdeModulePkg/Bus/Usb/UsbBusPei/HubPeim.h
@@ -1,7 +1,7 @@
 /** @file\r
 Constants definitions for Usb Hub Peim\r
 \r
-Copyright (c) 2006 - 2014, Intel Corporation. All rights reserved.<BR>\r
+Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.<BR>\r
   \r
 This program and the accompanying materials\r
 are licensed and made available under the terms and conditions\r
@@ -227,6 +227,7 @@ PeiHubClearHubFeature (
   Get a given hub descriptor.\r
 \r
   @param  PeiServices    General-purpose services that are available to every PEIM.\r
+  @param  PeiUsbDevice   Indicates the hub controller device.\r
   @param  UsbIoPpi       Indicates the PEI_USB_IO_PPI instance.\r
   @param  DescriptorSize The length of Hub Descriptor buffer.\r
   @param  HubDescriptor  Caller allocated buffer to store the hub descriptor if\r
@@ -240,6 +241,7 @@ PeiHubClearHubFeature (
 EFI_STATUS\r
 PeiGetHubDescriptor (\r
   IN EFI_PEI_SERVICES         **PeiServices,\r
+  IN PEI_USB_DEVICE           *PeiUsbDevice,\r
   IN PEI_USB_IO_PPI           *UsbIoPpi,\r
   IN UINTN                    DescriptorSize,\r
   OUT EFI_USB_HUB_DESCRIPTOR  *HubDescriptor\r