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
 /** @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
   \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
 }\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
 \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
   @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
 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
   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
 \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
   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
   //\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
 \r
   return  UsbIoPpi->UsbControlTransfer (\r
                       PeiServices,\r


@@ -357,7 +323,7 @@ PeiGetSuperSpeedHubDesc (
                       EfiUsbDataIn,\r
                       PcdGet32 (PcdUsbTransferTimeoutValue),\r
                       HubDescriptor,\r
                       EfiUsbDataIn,\r
                       PcdGet32 (PcdUsbTransferTimeoutValue),\r
                       HubDescriptor,\r

-                      12\r
+                      (UINT16)DescriptorSize\r

                       );\r
 }\r
 \r
                       );\r
 }\r
 \r


@@ -387,29 +353,19 @@ PeiUsbHubReadDesc (
 {\r
   EFI_STATUS Status;\r
 \r
 {\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

-    //\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
   }\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
 }\r
 \r
 /**\r


@@ -468,15 +424,21 @@ PeiDoHubConfig (
   IN PEI_USB_DEVICE      *PeiUsbDevice\r
   )\r
 {\r
   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
   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
   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
   //\r
   // Get the hub descriptor \r
   //\r


@@ -484,13 +446,13 @@ PeiDoHubConfig (
             PeiServices,\r
             PeiUsbDevice,\r
             UsbIoPpi,\r
             PeiServices,\r
             PeiUsbDevice,\r
             UsbIoPpi,\r

-            &HubDescriptor\r
+            HubDescriptor\r

             );\r
   if (EFI_ERROR (Status)) {\r
     return EFI_DEVICE_ERROR;\r
   }\r
 \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
 \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
       }\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
     }\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
 /** @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
   \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
   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
   @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
 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
   IN PEI_USB_IO_PPI           *UsbIoPpi,\r
   IN UINTN                    DescriptorSize,\r
   OUT EFI_USB_HUB_DESCRIPTOR  *HubDescriptor\r