commit eaae7b33b1cf6b9f21db1636f219c2b6a8d88afd
author Jeff Fan <jeff.fan@intel.com> Fri Nov 18 10:46:43 2016 +0800
committer Jeff Fan <jeff.fan@intel.com> Tue Nov 22 15:22:17 2016 +0800
tree b7e736f6b63d62f7b6c4823ea241c0aa6f0f32ed
parent 01dd077315c6759c94af9af4232f8318db13cf8d

MdeModulePkg/PiSmmCore: Cache CommunicationBuffer info before using it

gSmmCorePrivate->CommunicationBuffer and gSmmCorePrivate->BufferSize locate at
runtime memory region. That means they could be modified by non-SMM code during
runtime.

We should cache them into SMM local variables before we verify them. After
verification, we should use the cached ones directly instead of the ones in
gSmmCorePrivate.

Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Feng Tian <feng.tian@intel.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Jeff Fan <jeff.fan@intel.com>

MdeModulePkg/Core/PiSmmCore/PiSmmCore.c

diff --git a/MdeModulePkg/Core/PiSmmCore/PiSmmCore.c b/MdeModulePkg/Core/PiSmmCore/PiSmmCore.c
index b877a33..de8db65 100644
--- a/MdeModulePkg/Core/PiSmmCore/PiSmmCore.c
+++ b/MdeModulePkg/Core/PiSmmCore/PiSmmCore.c
@@ -432,6 +432,8 @@
   EFI_SMM_COMMUNICATE_HEADER  *CommunicateHeader;

   BOOLEAN                     InLegacyBoot;

   BOOLEAN                     IsOverlapped;

+  VOID                        *CommunicationBuffer;

+  UINTN                       BufferSize;

 

   PERF_START (NULL, "SMM", NULL, 0) ;

 

@@ -463,17 +465,19 @@
     // Check to see if this is a Synchronous SMI sent through the SMM Communication 

     // Protocol or an Asynchronous SMI

     //

-    if (gSmmCorePrivate->CommunicationBuffer != NULL) {

+    CommunicationBuffer = gSmmCorePrivate->CommunicationBuffer;

+    BufferSize          = gSmmCorePrivate->BufferSize;

+    if (CommunicationBuffer != NULL) {

       //

       // Synchronous SMI for SMM Core or request from Communicate protocol

       //

       IsOverlapped = InternalIsBufferOverlapped (

-                       (UINT8 *) gSmmCorePrivate->CommunicationBuffer,

-                       gSmmCorePrivate->BufferSize,

+                       (UINT8 *) CommunicationBuffer,

+                       BufferSize,

                        (UINT8 *) gSmmCorePrivate,

                        sizeof (*gSmmCorePrivate)

                        );

-      if (!SmmIsBufferOutsideSmmValid ((UINTN)gSmmCorePrivate->CommunicationBuffer, gSmmCorePrivate->BufferSize) || IsOverlapped) {

+      if (!SmmIsBufferOutsideSmmValid ((UINTN)CommunicationBuffer, BufferSize) || IsOverlapped) {

         //

         // If CommunicationBuffer is not in valid address scope,

         // or there is overlap between gSmmCorePrivate and CommunicationBuffer,

@@ -482,19 +486,19 @@
         gSmmCorePrivate->CommunicationBuffer = NULL;

         gSmmCorePrivate->ReturnStatus = EFI_INVALID_PARAMETER;

       } else {

-        CommunicateHeader = (EFI_SMM_COMMUNICATE_HEADER *)gSmmCorePrivate->CommunicationBuffer;

-        gSmmCorePrivate->BufferSize -= OFFSET_OF (EFI_SMM_COMMUNICATE_HEADER, Data);

+        CommunicateHeader = (EFI_SMM_COMMUNICATE_HEADER *)CommunicationBuffer;

+        BufferSize -= OFFSET_OF (EFI_SMM_COMMUNICATE_HEADER, Data);

         Status = SmiManage (

                    &CommunicateHeader->HeaderGuid, 

                    NULL, 

                    CommunicateHeader->Data, 

-                   &gSmmCorePrivate->BufferSize

+                   &BufferSize

                    );

         //

         // Update CommunicationBuffer, BufferSize and ReturnStatus

         // Communicate service finished, reset the pointer to CommBuffer to NULL

         //

-        gSmmCorePrivate->BufferSize += OFFSET_OF (EFI_SMM_COMMUNICATE_HEADER, Data);

+        gSmmCorePrivate->BufferSize = BufferSize + OFFSET_OF (EFI_SMM_COMMUNICATE_HEADER, Data);

         gSmmCorePrivate->CommunicationBuffer = NULL;

         gSmmCorePrivate->ReturnStatus = (Status == EFI_SUCCESS) ? EFI_SUCCESS : EFI_NOT_FOUND;

       }