From 665567cda914855b29632120174ab28be8c6df58 Mon Sep 17 00:00:00 2001

From: Laszlo Ersek <lersek@redhat.com>

Date: Tue, 9 Apr 2019 16:11:36 +0200

Subject: [PATCH 8/8] MdeModulePkg UsbBusDxe: Fix wrong buffer length used to

 read hub desc

MIME-Version: 1.0

Content-Type: text/plain; charset=UTF-8

Content-Transfer-Encoding: 8bit

Message-id: <20190409141136.27390-2-lersek@redhat.com>

Patchwork-id: 85539

O-Subject:  [RHEL-7.7 ovmf PATCH 1/1] MdeModulePkg UsbBusDxe: Fix wrong buffer

	length used to read hub desc

Bugzilla: 1697534

Acked-by: Philippe Mathieu-Daudé <philmd@redhat.com>

Acked-by: Vitaly Kuznetsov <vkuznets@redhat.com>

From: Star Zeng <star.zeng@intel.com>

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=973

HUB descriptor has variable length.

But the code uses stack (HubDesc in UsbHubInit) with fixed length

sizeof(EFI_USB_HUB_DESCRIPTOR) to hold HUB descriptor data.

It uses hard code length value (32 that is greater than

sizeof(EFI_USB_HUB_DESCRIPTOR)) for SuperSpeed path, then there will

be stack overflow when IOMMU is enabled because the Unmap operation

will copy the data from device buffer to host buffer.

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.

The definition EFI_USB_SUPER_SPEED_HUB_DESCRIPTOR is wrong (HubDelay

field should be UINT16 type) and no code is using it, the patch

removes it.

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>

(cherry picked from commit acebdf14c985c5c9f50b37ece0b15ada87767359)

Signed-off-by: Laszlo Ersek <lersek@redhat.com>

---

 MdeModulePkg/Bus/Usb/UsbBusDxe/UsbHub.c | 96 +++++++++++----------------------

 MdeModulePkg/Bus/Usb/UsbBusDxe/UsbHub.h | 14 +----

 2 files changed, 32 insertions(+), 78 deletions(-)

diff --git a/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbHub.c b/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbHub.c

index fabb441..a962f76 100644

--- a/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbHub.c

+++ b/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbHub.c

@@ -2,7 +2,7 @@

     Unified interface for RootHub and Hub.

-Copyright (c) 2007 - 2016, Intel Corporation. All rights reserved.
 

+Copyright (c) 2007 - 2018, Intel Corporation. All rights reserved.
 

 This program and the accompanying materials

 are licensed and made available under the terms and conditions of the BSD License

 which accompanies this distribution.  The full text of the license may be found at

@@ -201,42 +201,7 @@ UsbHubCtrlClearTTBuffer (

 }

 /**

-  Usb hub control transfer to get the super speed hub descriptor.

-

-  @param  HubDev                The hub device.

-  @param  Buf                   The buffer to hold the descriptor.

-

-  @retval EFI_SUCCESS           The hub descriptor is retrieved.

-  @retval Others                Failed to retrieve the hub descriptor.

-

-**/

-EFI_STATUS

-UsbHubCtrlGetSuperSpeedHubDesc (

-  IN  USB_DEVICE          *HubDev,

-  OUT VOID                *Buf

-  )

-{

-  EFI_STATUS              Status;

-  

-  Status = EFI_INVALID_PARAMETER;

-  

-  Status = UsbCtrlRequest (

-             HubDev,

-             EfiUsbDataIn,

-             USB_REQ_TYPE_CLASS,

-             USB_HUB_TARGET_HUB,

-             USB_HUB_REQ_GET_DESC,

-             (UINT16) (USB_DESC_TYPE_HUB_SUPER_SPEED << 8),

-             0,

-             Buf,

-             32

-             );

-

-  return Status;

-}

-

-/**

-  Usb hub control transfer to get the hub descriptor.

+  Usb hub control transfer to get the (super speed) hub descriptor.

   @param  HubDev                The hub device.

   @param  Buf                   The buffer to hold the descriptor.

@@ -254,6 +219,11 @@ UsbHubCtrlGetHubDesc (

   )

 {

   EFI_STATUS              Status;

+  UINT8                   DescType;

+

+  DescType = (HubDev->Speed == EFI_USB_SPEED_SUPER) ?

+             USB_DESC_TYPE_HUB_SUPER_SPEED :

+             USB_DESC_TYPE_HUB;

   Status = UsbCtrlRequest (

              HubDev,

@@ -261,7 +231,7 @@ UsbHubCtrlGetHubDesc (

              USB_REQ_TYPE_CLASS,

              USB_HUB_TARGET_HUB,

              USB_HUB_REQ_GET_DESC,

-             (UINT16) (USB_DESC_TYPE_HUB << 8),

+             (UINT16) (DescType << 8),

              0,

              Buf,

              Len

@@ -475,29 +445,19 @@ UsbHubReadDesc (

 {

   EFI_STATUS              Status;

-  if (HubDev->Speed == EFI_USB_SPEED_SUPER) {

-    //

-    // Get the super speed hub descriptor

-    //

-    Status = UsbHubCtrlGetSuperSpeedHubDesc (HubDev, HubDesc);

-  } else {

-

-    //

-    // First get the hub descriptor length

-    //

-    Status = UsbHubCtrlGetHubDesc (HubDev, HubDesc, 2);

-

-    if (EFI_ERROR (Status)) {

-      return Status;

-    }

+  //

+  // First get the hub descriptor length

+  //

+  Status = UsbHubCtrlGetHubDesc (HubDev, HubDesc, 2);

-    //

-    // Get the whole hub descriptor

-    //

-    Status = UsbHubCtrlGetHubDesc (HubDev, HubDesc, HubDesc->Length);

+  if (EFI_ERROR (Status)) {

+    return Status;

   }

-  return Status;

+  //

+  // Get the whole hub descriptor

+  //

+  return UsbHubCtrlGetHubDesc (HubDev, HubDesc, HubDesc->Length);

 }

@@ -690,7 +650,8 @@ UsbHubInit (

   IN USB_INTERFACE        *HubIf

   )

 {

-  EFI_USB_HUB_DESCRIPTOR  HubDesc;

+  UINT8                   HubDescBuffer[256];

+  EFI_USB_HUB_DESCRIPTOR  *HubDesc;

   USB_ENDPOINT_DESC       *EpDesc;

   USB_INTERFACE_SETTING   *Setting;

   EFI_USB_IO_PROTOCOL     *UsbIo;

@@ -725,14 +686,19 @@ UsbHubInit (

     return EFI_DEVICE_ERROR;

   }

-  Status = UsbHubReadDesc (HubDev, &HubDesc);

+  //

+  // The length field of descriptor is UINT8 type, so the buffer

+  // with 256 bytes is enough to hold the descriptor data.

+  //

+  HubDesc = (EFI_USB_HUB_DESCRIPTOR *) HubDescBuffer;

+  Status = UsbHubReadDesc (HubDev, HubDesc);

   if (EFI_ERROR (Status)) {

     DEBUG (( EFI_D_ERROR, "UsbHubInit: failed to read HUB descriptor %r\n", Status));

     return Status;

   }

-  HubIf->NumOfPort = HubDesc.NumPorts;

+  HubIf->NumOfPort = HubDesc->NumPorts;

   DEBUG (( EFI_D_INFO, "UsbHubInit: hub %d has %d ports\n", HubDev->Address,HubIf->NumOfPort));

@@ -751,7 +717,7 @@ UsbHubInit (

     DEBUG ((EFI_D_INFO, "UsbHubInit: Set Hub Depth as 0x%x\n", Depth));

     UsbHubCtrlSetHubDepth (HubIf->Device, Depth);

-    for (Index = 0; Index < HubDesc.NumPorts; Index++) {

+    for (Index = 0; Index < HubDesc->NumPorts; Index++) {

       UsbHubCtrlSetPortFeature (HubIf->Device, Index, USB_HUB_PORT_REMOTE_WAKE_MASK);

     }    

   } else {

@@ -759,15 +725,15 @@ UsbHubInit (

     // Feed power to all the hub ports. It should be ok

     // for both gang/individual powered hubs.

     //

-    for (Index = 0; Index < HubDesc.NumPorts; Index++) {

+    for (Index = 0; Index < HubDesc->NumPorts; Index++) {

       UsbHubCtrlSetPortFeature (HubIf->Device, Index, (EFI_USB_PORT_FEATURE) USB_HUB_PORT_POWER);

     }

     //

     // Update for the usb hub has no power on delay requirement

     //

-    if (HubDesc.PwrOn2PwrGood > 0) {

-      gBS->Stall (HubDesc.PwrOn2PwrGood * USB_SET_PORT_POWER_STALL);

+    if (HubDesc->PwrOn2PwrGood > 0) {

+      gBS->Stall (HubDesc->PwrOn2PwrGood * USB_SET_PORT_POWER_STALL);

     }

     UsbHubAckHubStatus (HubIf->Device);

   }

diff --git a/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbHub.h b/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbHub.h

index 4e5fcd8..fe9f1f7 100644

--- a/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbHub.h

+++ b/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbHub.h

@@ -2,7 +2,7 @@

     The definition for USB hub.

-Copyright (c) 2007 - 2010, Intel Corporation. All rights reserved.

+Copyright (c) 2007 - 2018, Intel Corporation. All rights reserved.

 This program and the accompanying materials

 are licensed and made available under the terms and conditions of the BSD License

 which accompanies this distribution.  The full text of the license may be found at

@@ -115,18 +115,6 @@ typedef struct {

   UINT8           Filler[16];

 } EFI_USB_HUB_DESCRIPTOR;

-typedef struct {

-  UINT8           Length;

-  UINT8           DescType;

-  UINT8           NumPorts;

-  UINT16          HubCharacter;

-  UINT8           PwrOn2PwrGood;

-  UINT8           HubContrCurrent;

-  UINT8           HubHdrDecLat;

-  UINT8           HubDelay;

-  UINT8           DeviceRemovable;

-} EFI_USB_SUPER_SPEED_HUB_DESCRIPTOR;

-

 #pragma pack()

-- 

1.8.3.1