TCBZ3398 and TCBZ3430: Make MessageLength the same size in EFI_MM_COM…

…MUNICATE_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.
microsoft · May 4, 2023 · de0bbd2 · de0bbd2
1 parent 41d0f95
commit de0bbd2
Show file tree

Hide file tree

Showing 4 changed files with 37 additions and 9 deletions.
diff --git 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
@@ -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
@@ -35,6 +35,7 @@
 #include <Library/PcdLib.h>
 #include <Library/ReportStatusCodeLib.h>
 
+#include <Library/SafeIntLib.h>           // MU_CHANGE: BZ3398
 #include <Library/SecurityLockAuditLib.h> // MSCHANGE
 #include <Library/SafeIntLib.h>
 
@@ -517,6 +518,7 @@ SmmCommunicationCommunicate (
   EFI_STATUS                  Status;
   EFI_SMM_COMMUNICATE_HEADER  *CommunicateHeader;
   BOOLEAN                     OldInSmm;
+  UINT64                      LongCommSize; // MU_CHANGE: BZ3398
   UINTN                       TempCommSize;
 
   //
@@ -529,7 +531,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
@@ -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.
   ///