StandaloneMmPkg: Fix check buffer address failed issue from TF-A

There are two scene communicate with StandaloneMm(MM):
1 edk2 -> TF-A -> MM, communicate MM use non-secure buffer which
  specify by EFI_SECURE_PARTITION_BOOT_INFO.SpNsCommBufBase;
2 RAS scene: fiq -> TF-A -> MM, use secure buffer which
  specify by EFI_SECURE_PARTITION_BOOT_INFO.SpShareBufBase;

For now, the second scene will failed because check buffer address.
This patch add CheckBufferAddr() to support check address for secure
buffer.

Signed-off-by: Ming Huang <huangming@linux.alibaba.com>
Reviewed-by: Sami Mujawar <sami.mujawar@arm.com>

3 files changed, 74 insertions, 15 deletions

diff --git a/StandaloneMmPkg/Drivers/StandaloneMmCpu/EventHandle.c b/StandaloneMmPkg/Drivers/StandaloneMmCpu/EventHandle.c
index 556fd21451..818e147f87 100644
--- a/StandaloneMmPkg/Drivers/StandaloneMmCpu/EventHandle.c
+++ b/StandaloneMmPkg/Drivers/StandaloneMmCpu/EventHandle.c
@@ -49,6 +49,7 @@ 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;

 

@@ -60,6 +61,53 @@ EFI_MM_CONFIGURATION_PROTOCOL  mMmConfig = {
 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 TF-A CPU driver.

 

   @param  [in] EventId            The event Id.

@@ -104,27 +152,16 @@ PiMmStandaloneArmTfCpuDriverEntry (
     return EFI_INVALID_PARAMETER;

   }

 

-  if (NsCommBufferAddr < mNsCommBuffer.PhysicalStart) {

-    return EFI_ACCESS_DENIED;

-  }

-

-  if ((NsCommBufferAddr + sizeof (EFI_MM_COMMUNICATE_HEADER)) >=

-      (mNsCommBuffer.PhysicalStart + mNsCommBuffer.PhysicalSize))

-  {

-    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);

 

-  // perform bounds check.

-  if (NsCommBufferAddr + NsCommBufferSize >=

-      mNsCommBuffer.PhysicalStart + mNsCommBuffer.PhysicalSize)

-  {

-    return EFI_ACCESS_DENIED;

-  }

-

   GuidedEventContext = NULL;

   // Now that the secure world can see the normal world buffer, allocate

   // memory to copy the communication buffer to the secure world.

diff --git a/StandaloneMmPkg/Drivers/StandaloneMmCpu/StandaloneMmCpu.c b/StandaloneMmPkg/Drivers/StandaloneMmCpu/StandaloneMmCpu.c
index d8bfba263a..3d1dc6181b 100644
--- a/StandaloneMmPkg/Drivers/StandaloneMmCpu/StandaloneMmCpu.c
+++ b/StandaloneMmPkg/Drivers/StandaloneMmCpu/StandaloneMmCpu.c
@@ -106,6 +106,7 @@ StandaloneMmCpuInitialize (
   UINTN                            Index;

   UINTN                            ArraySize;

   VOID                             *HobStart;

+  EFI_MMRAM_HOB_DESCRIPTOR_BLOCK   *MmramRangesHob;

 

   ASSERT (SystemTable != NULL);

   mMmst = SystemTable;

@@ -188,6 +189,26 @@ StandaloneMmCpuInitialize (
   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/Drivers/StandaloneMmCpu/StandaloneMmCpu.h b/StandaloneMmPkg/Drivers/StandaloneMmCpu/StandaloneMmCpu.h
index 251765eddf..3d4698ec19 100644
--- a/StandaloneMmPkg/Drivers/StandaloneMmCpu/StandaloneMmCpu.h
+++ b/StandaloneMmPkg/Drivers/StandaloneMmCpu/StandaloneMmCpu.h
@@ -30,6 +30,7 @@ extern EFI_MM_CPU_PROTOCOL  mMmCpuState;
 //

 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;