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>

2 files changed, 38 insertions, 74 deletions

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.<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 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.<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