From 6f8d1889f375a76644274265fdd7285bef314559 Mon Sep 17 00:00:00 2001 From: kuqin Date: Thu, 3 Jun 2021 19:58:35 -0700 Subject: [PATCH] TCBZ3398 and TCBZ3430: Make MessageLength the same size in EFI_MM_COMMUNICATE_HEADER for both IA32 and X64 REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3398 REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3430 These changes update references to the MessageLength field of EFI_MM_COMMUNICATE_HEADER. This structure can be used for both PEI and DXE MM communication. Thus, for a system that supports PEI MM launch but operates PEI in 32bit mode and MM foundation in 64bit, the current EFI_MM_COMMUNICATE_HEADER definition will cause structure parse error due to UINTN used. cherry-pick from 271a1a02a6 --- .../MemoryProfileInfo/MemoryProfileInfo.c | 20 ++++++++++++++----- .../SmiHandlerProfileInfo.c | 8 ++++++-- MdeModulePkg/Core/PiSmmCore/PiSmmIpl.c | 14 ++++++++++++- MdePkg/Include/Protocol/MmCommunication.h | 3 ++- 4 files changed, 36 insertions(+), 9 deletions(-) diff --git a/MdeModulePkg/Application/MemoryProfileInfo/MemoryProfileInfo.c b/MdeModulePkg/Application/MemoryProfileInfo/MemoryProfileInfo.c index 326603503f..8157c10f09 100644 --- a/MdeModulePkg/Application/MemoryProfileInfo/MemoryProfileInfo.c +++ b/MdeModulePkg/Application/MemoryProfileInfo/MemoryProfileInfo.c @@ -1240,7 +1240,9 @@ GetSmramProfileData ( CommRecordingState->Header.ReturnStatus = (UINT64)-1; CommRecordingState->RecordingState = MEMORY_PROFILE_RECORDING_DISABLE; - CommSize = sizeof (EFI_GUID) + sizeof (UINTN) + CommHeader->MessageLength; + // MU_CHANGE: BZ3398 Make MessageLength the same size in EFI_MM_COMMUNICATE_HEADER for both IA32 and X64. + // The CommHeader->MessageLength contains a definitive value, thus UINTN cast is safe here. + CommSize = OFFSET_OF (EFI_SMM_COMMUNICATE_HEADER, Data) + (UINTN)CommHeader->MessageLength; Status = SmmCommunication->Communicate (SmmCommunication, CommBuffer, &CommSize); if (EFI_ERROR (Status)) { DEBUG ((DEBUG_ERROR, "SmramProfile: SmmCommunication - %r\n", Status)); @@ -1264,7 +1266,9 @@ GetSmramProfileData ( CommRecordingState->Header.ReturnStatus = (UINT64)-1; CommRecordingState->RecordingState = MEMORY_PROFILE_RECORDING_DISABLE; - CommSize = sizeof (EFI_GUID) + sizeof (UINTN) + CommHeader->MessageLength; + // MU_CHANGE: BZ3398 Make MessageLength the same size in EFI_MM_COMMUNICATE_HEADER for both IA32 and X64. + // The CommHeader->MessageLength contains a definitive value, thus UINTN cast is safe here. + CommSize = OFFSET_OF (EFI_SMM_COMMUNICATE_HEADER, Data) + (UINTN)CommHeader->MessageLength; SmmCommunication->Communicate (SmmCommunication, CommBuffer, &CommSize); } @@ -1281,7 +1285,9 @@ GetSmramProfileData ( CommGetProfileInfo->Header.ReturnStatus = (UINT64)-1; CommGetProfileInfo->ProfileSize = 0; - CommSize = sizeof (EFI_GUID) + sizeof (UINTN) + CommHeader->MessageLength; + // MU_CHANGE: BZ3398 Make MessageLength the same size in EFI_MM_COMMUNICATE_HEADER for both IA32 and X64. + // The CommHeader->MessageLength contains a definitive value, thus UINTN cast is safe here. + CommSize = OFFSET_OF (EFI_SMM_COMMUNICATE_HEADER, Data) + (UINTN)CommHeader->MessageLength; Status = SmmCommunication->Communicate (SmmCommunication, CommBuffer, &CommSize); ASSERT_EFI_ERROR (Status); @@ -1312,7 +1318,9 @@ GetSmramProfileData ( CommGetProfileData->Header.DataLength = sizeof (*CommGetProfileData); CommGetProfileData->Header.ReturnStatus = (UINT64)-1; - CommSize = sizeof (EFI_GUID) + sizeof (UINTN) + CommHeader->MessageLength; + // MU_CHANGE: BZ3398 Make MessageLength the same size in EFI_MM_COMMUNICATE_HEADER for both IA32 and X64. + // The CommHeader->MessageLength contains a definitive value, thus UINTN cast is safe here. + CommSize = OFFSET_OF (EFI_SMM_COMMUNICATE_HEADER, Data) + (UINTN)CommHeader->MessageLength; Buffer = (UINT8 *)CommHeader + CommSize; Size -= CommSize; @@ -1372,7 +1380,9 @@ GetSmramProfileData ( CommRecordingState->Header.ReturnStatus = (UINT64)-1; CommRecordingState->RecordingState = MEMORY_PROFILE_RECORDING_ENABLE; - CommSize = sizeof (EFI_GUID) + sizeof (UINTN) + CommHeader->MessageLength; + // MU_CHANGE: BZ3398 Make MessageLength the same size in EFI_MM_COMMUNICATE_HEADER for both IA32 and X64. + // The CommHeader->MessageLength contains a definitive value, thus UINTN cast is safe here. + CommSize = OFFSET_OF (EFI_SMM_COMMUNICATE_HEADER, Data) + (UINTN)CommHeader->MessageLength; SmmCommunication->Communicate (SmmCommunication, CommBuffer, &CommSize); } diff --git a/MdeModulePkg/Application/SmiHandlerProfileInfo/SmiHandlerProfileInfo.c b/MdeModulePkg/Application/SmiHandlerProfileInfo/SmiHandlerProfileInfo.c index 9e39b2cc8c..a180cb7c67 100644 --- a/MdeModulePkg/Application/SmiHandlerProfileInfo/SmiHandlerProfileInfo.c +++ b/MdeModulePkg/Application/SmiHandlerProfileInfo/SmiHandlerProfileInfo.c @@ -120,7 +120,9 @@ GetSmiHandlerProfileDatabase ( CommGetInfo->Header.ReturnStatus = (UINT64)-1; CommGetInfo->DataSize = 0; - CommSize = sizeof (EFI_GUID) + sizeof (UINTN) + CommHeader->MessageLength; + // MU_CHANGE: BZ3398 Make MessageLength the same size in EFI_MM_COMMUNICATE_HEADER for both IA32 and X64. + // The CommHeader->MessageLength contains a definitive value, thus UINTN cast is safe here. + CommSize = OFFSET_OF (EFI_SMM_COMMUNICATE_HEADER, Data) + (UINTN)CommHeader->MessageLength; Status = SmmCommunication->Communicate (SmmCommunication, CommBuffer, &CommSize); if (EFI_ERROR (Status)) { Print (L"SmiHandlerProfile: SmmCommunication - %r\n", Status); @@ -153,7 +155,9 @@ GetSmiHandlerProfileDatabase ( CommGetData->Header.DataLength = sizeof (*CommGetData); CommGetData->Header.ReturnStatus = (UINT64)-1; - CommSize = sizeof (EFI_GUID) + sizeof (UINTN) + CommHeader->MessageLength; + // MU_CHANGE: BZ3398 Make MessageLength the same size in EFI_MM_COMMUNICATE_HEADER for both IA32 and X64. + // The CommHeader->MessageLength contains a definitive value, thus UINTN cast is safe here. + CommSize = OFFSET_OF (EFI_SMM_COMMUNICATE_HEADER, Data) + (UINTN)CommHeader->MessageLength; Buffer = (UINT8 *)CommHeader + CommSize; Size -= CommSize; diff --git a/MdeModulePkg/Core/PiSmmCore/PiSmmIpl.c b/MdeModulePkg/Core/PiSmmCore/PiSmmIpl.c index fbba868fd0..39caba5d93 100644 --- a/MdeModulePkg/Core/PiSmmCore/PiSmmIpl.c +++ b/MdeModulePkg/Core/PiSmmCore/PiSmmIpl.c @@ -514,6 +514,7 @@ SmmCommunicationCommunicate ( EFI_STATUS Status; EFI_SMM_COMMUNICATE_HEADER *CommunicateHeader; BOOLEAN OldInSmm; + UINT64 LongCommSize; // MU_CHANGE: BZ3398 UINTN TempCommSize; // @@ -526,7 +527,18 @@ SmmCommunicationCommunicate ( CommunicateHeader = (EFI_SMM_COMMUNICATE_HEADER *)CommBuffer; if (CommSize == NULL) { - TempCommSize = OFFSET_OF (EFI_SMM_COMMUNICATE_HEADER, Data) + CommunicateHeader->MessageLength; + // MU_CHANGE Starts: BZ3398 Make MessageLength the same size in EFI_MM_COMMUNICATE_HEADER for both IA32 and X64. + Status = SafeUint64Add (OFFSET_OF (EFI_SMM_COMMUNICATE_HEADER, Data), CommunicateHeader->MessageLength, &LongCommSize); + if (EFI_ERROR (Status)) { + return EFI_INVALID_PARAMETER; + } + + Status = SafeUint64ToUintn (LongCommSize, &TempCommSize); + if (EFI_ERROR (Status)) { + return EFI_INVALID_PARAMETER; + } + + // MU_CHANGE Ends: BZ3398 } else { TempCommSize = *CommSize; // diff --git a/MdePkg/Include/Protocol/MmCommunication.h b/MdePkg/Include/Protocol/MmCommunication.h index 0ccb989020..205ec6bca1 100644 --- a/MdePkg/Include/Protocol/MmCommunication.h +++ b/MdePkg/Include/Protocol/MmCommunication.h @@ -26,7 +26,8 @@ typedef struct { /// /// Describes the size of Data (in bytes) and does not include the size of the header. /// - UINTN MessageLength; + // MU_CHANGE: BZ3398 Make MessageLength the same size in EFI_MM_COMMUNICATE_HEADER for both IA32 and X64. + UINT64 MessageLength; /// /// Designates an array of bytes that is MessageLength in size. ///