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>
This commit is contained in:
Star Zeng 2018-06-25 16:50:01 +08:00
parent acebdf14c9
commit 72750e3bf9
2 changed files with 38 additions and 74 deletions

View File

@ -1,7 +1,7 @@
/** @file /** @file
Usb Hub Request Support In PEI Phase 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 This program and the accompanying materials
are licensed and made available under the terms and conditions 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 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 UsbIoPpi Indicates the PEI_USB_IO_PPI instance.
@param DescriptorSize The length of Hub Descriptor buffer. @param DescriptorSize The length of Hub Descriptor buffer.
@param HubDescriptor Caller allocated buffer to store the hub descriptor if @param HubDescriptor Caller allocated buffer to store the hub descriptor if
@ -292,21 +293,28 @@ PeiHubClearHubFeature (
EFI_STATUS EFI_STATUS
PeiGetHubDescriptor ( PeiGetHubDescriptor (
IN EFI_PEI_SERVICES **PeiServices, IN EFI_PEI_SERVICES **PeiServices,
IN PEI_USB_DEVICE *PeiUsbDevice,
IN PEI_USB_IO_PPI *UsbIoPpi, IN PEI_USB_IO_PPI *UsbIoPpi,
IN UINTN DescriptorSize, IN UINTN DescriptorSize,
OUT EFI_USB_HUB_DESCRIPTOR *HubDescriptor OUT EFI_USB_HUB_DESCRIPTOR *HubDescriptor
) )
{ {
EFI_USB_DEVICE_REQUEST DevReq; EFI_USB_DEVICE_REQUEST DevReq;
UINT8 DescType;
ZeroMem (&DevReq, sizeof (EFI_USB_DEVICE_REQUEST)); 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 // Fill Device request packet
// //
DevReq.RequestType = USB_RT_HUB | 0x80; DevReq.RequestType = USB_RT_HUB | 0x80;
DevReq.Request = USB_HUB_GET_DESCRIPTOR; DevReq.Request = USB_HUB_GET_DESCRIPTOR;
DevReq.Value = USB_DT_HUB << 8; DevReq.Value = (UINT16) (DescType << 8);
DevReq.Length = (UINT16)DescriptorSize; DevReq.Length = (UINT16) DescriptorSize;
return UsbIoPpi->UsbControlTransfer ( return UsbIoPpi->UsbControlTransfer (
PeiServices, PeiServices,
@ -319,48 +327,6 @@ PeiGetHubDescriptor (
); );
} }
/**
Get a given SuperSpeed hub descriptor.
@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));
//
// 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;
return UsbIoPpi->UsbControlTransfer (
PeiServices,
UsbIoPpi,
&DevReq,
EfiUsbDataIn,
PcdGet32 (PcdUsbTransferTimeoutValue),
HubDescriptor,
12
);
}
/** /**
Read the whole usb hub descriptor. It is necessary Read the whole usb hub descriptor. It is necessary
to do it in two steps because hub descriptor is of to do it in two steps because hub descriptor is of
@ -387,29 +353,19 @@ PeiUsbHubReadDesc (
{ {
EFI_STATUS Status; EFI_STATUS Status;
if (PeiUsbDevice->DeviceSpeed == EFI_USB_SPEED_SUPER) { //
// // First get the hub descriptor length
// Get the super speed hub descriptor //
// Status = PeiGetHubDescriptor (PeiServices, PeiUsbDevice, UsbIoPpi, 2, HubDescriptor);
Status = PeiGetSuperSpeedHubDesc (PeiServices, UsbIoPpi, HubDescriptor);
} else {
// if (EFI_ERROR (Status)) {
// First get the hub descriptor length return Status;
//
Status = PeiGetHubDescriptor (PeiServices, UsbIoPpi, 2, HubDescriptor);
if (EFI_ERROR (Status)) {
return Status;
}
//
// Get the whole hub descriptor
//
Status = PeiGetHubDescriptor (PeiServices, UsbIoPpi, HubDescriptor->Length, HubDescriptor);
} }
return Status; //
// Get the whole hub descriptor
//
return PeiGetHubDescriptor (PeiServices, PeiUsbDevice, UsbIoPpi, HubDescriptor->Length, HubDescriptor);
} }
/** /**
@ -468,15 +424,21 @@ PeiDoHubConfig (
IN PEI_USB_DEVICE *PeiUsbDevice IN PEI_USB_DEVICE *PeiUsbDevice
) )
{ {
EFI_USB_HUB_DESCRIPTOR HubDescriptor; UINT8 HubDescBuffer[256];
EFI_USB_HUB_DESCRIPTOR *HubDescriptor;
EFI_STATUS Status; EFI_STATUS Status;
EFI_USB_HUB_STATUS HubStatus; EFI_USB_HUB_STATUS HubStatus;
UINTN Index; UINTN Index;
PEI_USB_IO_PPI *UsbIoPpi; PEI_USB_IO_PPI *UsbIoPpi;
ZeroMem (&HubDescriptor, sizeof (HubDescriptor));
UsbIoPpi = &PeiUsbDevice->UsbIoPpi; 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 // Get the hub descriptor
// //
@ -484,13 +446,13 @@ PeiDoHubConfig (
PeiServices, PeiServices,
PeiUsbDevice, PeiUsbDevice,
UsbIoPpi, UsbIoPpi,
&HubDescriptor HubDescriptor
); );
if (EFI_ERROR (Status)) { if (EFI_ERROR (Status)) {
return EFI_DEVICE_ERROR; return EFI_DEVICE_ERROR;
} }
PeiUsbDevice->DownStreamPortNo = HubDescriptor.NbrPorts; PeiUsbDevice->DownStreamPortNo = HubDescriptor->NbrPorts;
if (PeiUsbDevice->DeviceSpeed == EFI_USB_SPEED_SUPER) { if (PeiUsbDevice->DeviceSpeed == EFI_USB_SPEED_SUPER) {
DEBUG ((EFI_D_INFO, "PeiDoHubConfig: Set Hub Depth as 0x%x\n", PeiUsbDevice->Tier)); 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)); DEBUG (( EFI_D_INFO, "PeiDoHubConfig: HubDescriptor.PwrOn2PwrGood: 0x%x\n", HubDescriptor->PwrOn2PwrGood));
if (HubDescriptor.PwrOn2PwrGood > 0) { if (HubDescriptor->PwrOn2PwrGood > 0) {
MicroSecondDelay (HubDescriptor.PwrOn2PwrGood * USB_SET_PORT_POWER_STALL); MicroSecondDelay (HubDescriptor->PwrOn2PwrGood * USB_SET_PORT_POWER_STALL);
} }
// //

View File

@ -1,7 +1,7 @@
/** @file /** @file
Constants definitions for Usb Hub Peim 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 This program and the accompanying materials
are licensed and made available under the terms and conditions are licensed and made available under the terms and conditions
@ -227,6 +227,7 @@ PeiHubClearHubFeature (
Get a given hub descriptor. Get a given hub descriptor.
@param PeiServices General-purpose services that are available to every PEIM. @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 UsbIoPpi Indicates the PEI_USB_IO_PPI instance.
@param DescriptorSize The length of Hub Descriptor buffer. @param DescriptorSize The length of Hub Descriptor buffer.
@param HubDescriptor Caller allocated buffer to store the hub descriptor if @param HubDescriptor Caller allocated buffer to store the hub descriptor if
@ -240,6 +241,7 @@ PeiHubClearHubFeature (
EFI_STATUS EFI_STATUS
PeiGetHubDescriptor ( PeiGetHubDescriptor (
IN EFI_PEI_SERVICES **PeiServices, IN EFI_PEI_SERVICES **PeiServices,
IN PEI_USB_DEVICE *PeiUsbDevice,
IN PEI_USB_IO_PPI *UsbIoPpi, IN PEI_USB_IO_PPI *UsbIoPpi,
IN UINTN DescriptorSize, IN UINTN DescriptorSize,
OUT EFI_USB_HUB_DESCRIPTOR *HubDescriptor OUT EFI_USB_HUB_DESCRIPTOR *HubDescriptor