From: Ming Huang <huangming@linux.alibaba.com>
Date: Fri, 31 Dec 2021 11:06:23 +0000 (+0800)
Subject: StandaloneMmPkg: Fix check buffer address failed issue from TF-A
X-Git-Tag: edk2-stable202208~122
X-Git-Url: https://git.proxmox.com/?p=mirror_edk2.git;a=commitdiff_plain;h=5496c763aaddc4a47639d4652abe23aa3419263a

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

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;
 
@@ -59,6 +60,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.
 
@@ -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;