StandaloneMmPkg: Move sanity check for comm buffer to entrypoint

Arm has three types of communication buffer
    - Non secure communication buffer: shared buffer with NS partition
    - Secure communication buffer: shared buffer with Secure partition
    - Internal Misc service buffer: For misc service
        i.e. for services that do not use MmCommunication protocol,
        a buffer is created and populated internally.

Since, internal Misc service buffer is allocated dynamically in
StandaloneMm there is no additional check required for the Misc
service buffer.

This patch move sanity check of communication buffer from
StandaloneMmCpu Driver to StandaloneMmEntryPoint.

Signed-off-by: Levi Yun <yeoreum.yun@arm.com>

5 files changed, 125 insertions, 107 deletions

diff --git a/StandaloneMmPkg/Drivers/StandaloneMmCpu/EventHandle.c b/StandaloneMmPkg/Drivers/StandaloneMmCpu/EventHandle.c
index 1d61c45a35..f660d1210a 100644
--- a/StandaloneMmPkg/Drivers/StandaloneMmCpu/EventHandle.c
+++ b/StandaloneMmPkg/Drivers/StandaloneMmCpu/EventHandle.c
@@ -43,10 +43,6 @@ MmFoundationEntryRegister (
 //

 EFI_MM_COMMUNICATE_HEADER  **PerCpuGuidedEventContext = NULL;

 

-// Descriptor with whereabouts of memory used for communication with the normal world

-EFI_MMRAM_DESCRIPTOR  mNsCommBuffer;

-EFI_MMRAM_DESCRIPTOR  mSCommBuffer;

-

 MP_INFORMATION_HOB_DATA  *mMpInformationHobData;

 

 EFI_MM_CONFIGURATION_PROTOCOL  mMmConfig = {

@@ -61,53 +57,6 @@ EDKII_PI_MM_CPU_DRIVER_EP_PROTOCOL  mPiMmCpuDriverEpProtocol = {
 STATIC EFI_MM_ENTRY_POINT  mMmEntryPoint = NULL;

 

 /**

-  Perform bounds check on the common buffer.

-

-  @param  [in] BufferAddr   Address of the common buffer.

-

-  @retval   EFI_SUCCESS             Success.

-  @retval   EFI_ACCESS_DENIED       Access not permitted.

-**/

-STATIC

-EFI_STATUS

-CheckBufferAddr (

-  IN UINTN  BufferAddr

-  )

-{

-  UINT64  NsCommBufferEnd;

-  UINT64  SCommBufferEnd;

-  UINT64  CommBufferEnd;

-

-  NsCommBufferEnd = mNsCommBuffer.PhysicalStart + mNsCommBuffer.PhysicalSize;

-  SCommBufferEnd  = mSCommBuffer.PhysicalStart + mSCommBuffer.PhysicalSize;

-

-  if ((BufferAddr >= mNsCommBuffer.PhysicalStart) &&

-      (BufferAddr < NsCommBufferEnd))

-  {

-    CommBufferEnd = NsCommBufferEnd;

-  } else if ((BufferAddr >= mSCommBuffer.PhysicalStart) &&

-             (BufferAddr < SCommBufferEnd))

-  {

-    CommBufferEnd = SCommBufferEnd;

-  } else {

-    return EFI_ACCESS_DENIED;

-  }

-

-  if ((CommBufferEnd - BufferAddr) < sizeof (EFI_MM_COMMUNICATE_HEADER)) {

-    return EFI_ACCESS_DENIED;

-  }

-

-  // perform bounds check.

-  if ((CommBufferEnd - BufferAddr - sizeof (EFI_MM_COMMUNICATE_HEADER)) <

-      ((EFI_MM_COMMUNICATE_HEADER *)BufferAddr)->MessageLength)

-  {

-    return EFI_ACCESS_DENIED;

-  }

-

-  return EFI_SUCCESS;

-}

-

-/**

   The PI Standalone MM entry point for the CPU driver.

 

   @param  [in] EventId            The event Id.

@@ -141,12 +90,6 @@ PiMmStandaloneMmCpuDriverEntry (
     return EFI_INVALID_PARAMETER;

   }

 

-  Status = CheckBufferAddr (NsCommBufferAddr);

-  if (EFI_ERROR (Status)) {

-    DEBUG ((DEBUG_ERROR, "Check Buffer failed: %r\n", Status));

-    return Status;

-  }

-

   // Find out the size of the buffer passed

   NsCommBufferSize = ((EFI_MM_COMMUNICATE_HEADER *)NsCommBufferAddr)->MessageLength +

                      sizeof (EFI_MM_COMMUNICATE_HEADER);

diff --git a/StandaloneMmPkg/Drivers/StandaloneMmCpu/StandaloneMmCpu.c b/StandaloneMmPkg/Drivers/StandaloneMmCpu/StandaloneMmCpu.c
index db9c2a899a..b6d8d37b97 100644
--- a/StandaloneMmPkg/Drivers/StandaloneMmCpu/StandaloneMmCpu.c
+++ b/StandaloneMmPkg/Drivers/StandaloneMmCpu/StandaloneMmCpu.c
@@ -95,16 +95,14 @@ StandaloneMmCpuInitialize (
   IN EFI_MM_SYSTEM_TABLE  *SystemTable   // not actual systemtable

   )

 {

-  EFI_CONFIGURATION_TABLE         *ConfigurationTable;

-  MP_INFORMATION_HOB_DATA         *MpInformationHobData;

-  EFI_MMRAM_DESCRIPTOR            *NsCommBufMmramRange;

-  EFI_STATUS                      Status;

-  EFI_HANDLE                      DispatchHandle;

-  UINT32                          MpInfoSize;

-  UINTN                           Index;

-  UINTN                           ArraySize;

-  VOID                            *HobStart;

-  EFI_MMRAM_HOB_DESCRIPTOR_BLOCK  *MmramRangesHob;

+  EFI_CONFIGURATION_TABLE  *ConfigurationTable;

+  MP_INFORMATION_HOB_DATA  *MpInformationHobData;

+  EFI_STATUS               Status;

+  EFI_HANDLE               DispatchHandle;

+  UINT32                   MpInfoSize;

+  UINTN                    Index;

+  UINTN                    ArraySize;

+  VOID                     *HobStart;

 

   ASSERT (SystemTable != NULL);

   mMmst = SystemTable;

@@ -160,44 +158,6 @@ StandaloneMmCpuInitialize (
 

   HobStart = ConfigurationTable[Index].VendorTable;

 

-  // Find the descriptor that contains the whereabouts of the buffer for

-  // communication with the Normal world.

-  Status = GetGuidedHobData (

-             HobStart,

-             &gEfiStandaloneMmNonSecureBufferGuid,

-             (VOID **)&NsCommBufMmramRange

-             );

-  if (EFI_ERROR (Status)) {

-    DEBUG ((DEBUG_ERROR, "NsCommBufMmramRange HOB data extraction failed - 0x%x\n", Status));

-    return Status;

-  }

-

-  DEBUG ((DEBUG_INFO, "mNsCommBuffer.PhysicalStart - 0x%lx\n", (UINTN)NsCommBufMmramRange->PhysicalStart));

-  DEBUG ((DEBUG_INFO, "mNsCommBuffer.PhysicalSize - 0x%lx\n", (UINTN)NsCommBufMmramRange->PhysicalSize));

-

-  CopyMem (&mNsCommBuffer, NsCommBufMmramRange, sizeof (EFI_MMRAM_DESCRIPTOR));

-  DEBUG ((DEBUG_INFO, "mNsCommBuffer: 0x%016lx - 0x%lx\n", mNsCommBuffer.CpuStart, mNsCommBuffer.PhysicalSize));

-

-  Status = GetGuidedHobData (

-             HobStart,

-             &gEfiMmPeiMmramMemoryReserveGuid,

-             (VOID **)&MmramRangesHob

-             );

-  if (EFI_ERROR (Status)) {

-    DEBUG ((DEBUG_ERROR, "MmramRangesHob data extraction failed - 0x%x\n", Status));

-    return Status;

-  }

-

-  //

-  // As CreateHobListFromBootInfo(), the base and size of buffer shared with

-  // privileged Secure world software is in second one.

-  //

-  CopyMem (

-    &mSCommBuffer,

-    &MmramRangesHob->Descriptor[0] + 1,

-    sizeof (EFI_MMRAM_DESCRIPTOR)

-    );

-

   //

   // Extract the MP information from the Hoblist

   //

diff --git a/StandaloneMmPkg/Include/Library/Arm/StandaloneMmCoreEntryPoint.h b/StandaloneMmPkg/Include/Library/Arm/StandaloneMmCoreEntryPoint.h
index 0b5b8a2ea5..1549211613 100644
--- a/StandaloneMmPkg/Include/Library/Arm/StandaloneMmCoreEntryPoint.h
+++ b/StandaloneMmPkg/Include/Library/Arm/StandaloneMmCoreEntryPoint.h
@@ -31,6 +31,16 @@
 #define CPU_INFO_FLAG_PRIMARY_CPU  0x00000001

 

 /*

+ * Index information for mm range descriptors in

+ * gEfiMmPeiMmramMemoryReserveGuid Guid Hob.

+ */

+#define MMRAM_DESC_IDX_IMAGE                 0

+#define MMRAM_DESC_IDX_SECURE_SHARED_BUFFER  1

+#define MMRAM_DESC_IDX_NORMAL_SHARED_BUFFER  2

+#define MMRAM_DESC_IDX_HEAP                  3

+#define MMRAM_DESC_MIN_COUNT                 4

+

+/*

  * Communication ABI protocol to communicate between normal/secure partition.

  */

 typedef enum {

diff --git a/StandaloneMmPkg/Include/StandaloneMmCpu.h b/StandaloneMmPkg/Include/StandaloneMmCpu.h
index 0e34e8c7d8..dbe6189b2b 100644
--- a/StandaloneMmPkg/Include/StandaloneMmCpu.h
+++ b/StandaloneMmPkg/Include/StandaloneMmCpu.h
@@ -31,8 +31,6 @@ extern EFI_MM_CPU_PROTOCOL  mMmCpuState;
 // MM event handling specific declarations

 //

 extern EFI_MM_COMMUNICATE_HEADER      **PerCpuGuidedEventContext;

-extern EFI_MMRAM_DESCRIPTOR           mNsCommBuffer;

-extern EFI_MMRAM_DESCRIPTOR           mSCommBuffer;

 extern MP_INFORMATION_HOB_DATA        *mMpInformationHobData;

 extern EFI_MM_CONFIGURATION_PROTOCOL  mMmConfig;

 

diff --git a/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/Arm/StandaloneMmCoreEntryPoint.c b/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/Arm/StandaloneMmCoreEntryPoint.c
index 353a5b2d23..cf2f1ef4d4 100644
--- a/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/Arm/StandaloneMmCoreEntryPoint.c
+++ b/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/Arm/StandaloneMmCoreEntryPoint.c
@@ -46,6 +46,7 @@
 #include <IndustryStandard/ArmFfaBootInfo.h>

 

 #include <Protocol/PiMmCpuDriverEp.h>

+#include <Protocol/MmCommunication.h>

 

 extern EFI_MM_SYSTEM_TABLE  gMmCoreMmst;

 

@@ -53,6 +54,8 @@ VOID  *gHobList = NULL;
 

 STATIC MP_INFORMATION_HOB_DATA     *mMpInfo                  = NULL;

 STATIC MISC_MM_COMMUNICATE_BUFFER  *mMiscMmCommunicateBuffer = NULL;

+STATIC EFI_MMRAM_DESCRIPTOR        *mNsCommBuffer            = NULL;

+STATIC EFI_MMRAM_DESCRIPTOR        *mSCommBuffer             = NULL;

 

 /**

   Get communication ABI protocol.

@@ -485,6 +488,59 @@ GetCpuNumber (
 }

 

 /**

+  Perform bounds check for the Ns and Secure Communication buffer.

+

+  NOTE: We do not need to validate the Misc Communication buffer as

+  we are initialising that in StandaloneMm.

+

+  @param  [in] CommBufferAddr   Address of the common buffer.

+

+  @retval   EFI_SUCCESS             Success.

+  @retval   EFI_ACCESS_DENIED       Access not permitted.

+**/

+STATIC

+EFI_STATUS

+ValidateMmCommBufferAddr (

+  IN UINTN  CommBufferAddr

+  )

+{

+  UINT64  NsCommBufferEnd;

+  UINT64  SCommBufferEnd;

+  UINT64  CommBufferEnd;

+  UINT64  CommBufferRange;

+

+  NsCommBufferEnd = mNsCommBuffer->PhysicalStart + mNsCommBuffer->PhysicalSize;

+  SCommBufferEnd  = mSCommBuffer->PhysicalStart + mSCommBuffer->PhysicalSize;

+

+  if ((CommBufferAddr >= mNsCommBuffer->PhysicalStart) &&

+      (CommBufferAddr < NsCommBufferEnd))

+  {

+    CommBufferEnd = NsCommBufferEnd;

+  } else if ((CommBufferAddr >= mSCommBuffer->PhysicalStart) &&

+             (CommBufferAddr < SCommBufferEnd))

+  {

+    CommBufferEnd = SCommBufferEnd;

+  } else {

+    return EFI_ACCESS_DENIED;

+  }

+

+  CommBufferRange = CommBufferEnd - CommBufferAddr;

+

+  if (CommBufferRange < sizeof (EFI_MM_COMMUNICATE_HEADER)) {

+    return EFI_ACCESS_DENIED;

+  }

+

+  // perform bounds check.

+  if ((CommBufferRange - sizeof (EFI_MM_COMMUNICATE_HEADER)) <

+      ((EFI_MM_COMMUNICATE_HEADER *)CommBufferAddr)->MessageLength)

+  {

+    return EFI_ACCESS_DENIED;

+  }

+

+  return EFI_SUCCESS;

+}

+

+/**

   Dump mp information descriptor.

 

   @param[in]      ProcessorInfo       Mp information

@@ -993,6 +1049,18 @@ DelegatedEventLoop (
       ServiceType    = ServiceTypeMmCommunication;

     }

 

+    if (ServiceType == ServiceTypeMmCommunication) {

+      Status = ValidateMmCommBufferAddr (CommBufferAddr);

+      if (EFI_ERROR (Status)) {

+        DEBUG ((

+          DEBUG_ERROR,

+          "Error: Failed to validate Communication Buffer address(0x%x)...\n",

+          CommBufferAddr

+          ));

+        goto ExitHandler;

+      }

+    }

+

     Status = CpuDriverEntryPoint ((UINTN)ServiceType, CpuNumber, CommBufferAddr);

     if (EFI_ERROR (Status)) {

       DEBUG ((

@@ -1054,6 +1122,7 @@ CEntryPoint (
   EFI_HOB_GUID_TYPE                   *GuidHob;

   EFI_CONFIGURATION_TABLE             *ConfigurationTable;

   UINTN                               Idx;

+  EFI_MMRAM_HOB_DESCRIPTOR_BLOCK      *MmramRangesHob;

 

   CpuDriverEntryPoint = NULL;

 

@@ -1187,6 +1256,44 @@ CEntryPoint (
 

   mMpInfo = GET_GUID_HOB_DATA (GuidHob);

 

+  // Find the descriptor that contains the whereabouts of the buffer for

+  // communication with the Normal world.

+  GuidHob = GetNextGuidHob (&gEfiStandaloneMmNonSecureBufferGuid, gHobList);

+  if (GuidHob == NULL) {

+    Status = EFI_NOT_FOUND;

+    DEBUG ((DEBUG_ERROR, "Error: No NsCommBuffer hob ...\n"));

+    goto finish;

+  }

+

+  mNsCommBuffer = GET_GUID_HOB_DATA (GuidHob);

+  if (mNsCommBuffer == NULL) {

+    Status = EFI_NOT_FOUND;

+    DEBUG ((DEBUG_ERROR, "Error: No NsCommBuffer hob data...\n"));

+    goto finish;

+  }

+

+  //

+  // The base and size of buffer shared with

+  // privileged Secure world software is in PeiMmramMemoryReservedGuid Hob.

+  //

+  GuidHob = GetNextGuidHob (&gEfiMmPeiMmramMemoryReserveGuid, gHobList);

+  if (GuidHob == NULL) {

+    Status = EFI_NOT_FOUND;

+    DEBUG ((DEBUG_ERROR, "Error: No PeiMmramMemoryReserved hob ...\n"));

+    goto finish;

+  }

+

+  MmramRangesHob = GET_GUID_HOB_DATA (GuidHob);

+  if ((MmramRangesHob == NULL) ||

+      (MmramRangesHob->NumberOfMmReservedRegions < MMRAM_DESC_MIN_COUNT))

+  {

+    Status = EFI_NOT_FOUND;

+    DEBUG ((DEBUG_ERROR, "Error: Failed to get shared comm buffer ...\n"));

+    goto finish;

+  }

+

+  mSCommBuffer = &MmramRangesHob->Descriptor[MMRAM_DESC_IDX_SECURE_SHARED_BUFFER];

+

   //

   // Find out cpu driver entry point used in DelegatedEventLoop

   // to handle MMI request.