Add SMRAM range check to variable SMM SMI handler.

git-svn-id: https://edk2.svn.sourceforge.net/svnroot/edk2/trunk/edk2@13514 6f19259b-4bc3-4df7-8a09-765794883524

2 files changed, 123 insertions, 7 deletions

diff --git a/SecurityPkg/VariableAuthenticated/RuntimeDxe/VariableSmm.c b/SecurityPkg/VariableAuthenticated/RuntimeDxe/VariableSmm.c
index 37b6f11839..8247836a63 100644
--- a/SecurityPkg/VariableAuthenticated/RuntimeDxe/VariableSmm.c
+++ b/SecurityPkg/VariableAuthenticated/RuntimeDxe/VariableSmm.c
@@ -28,12 +28,17 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
 #include <Protocol/SmmVariable.h>

 #include <Protocol/SmmFirmwareVolumeBlock.h>

 #include <Protocol/SmmFaultTolerantWrite.h>

+#include <Protocol/SmmAccess2.h>

+

 #include <Library/SmmServicesTableLib.h>

 

 #include <Guid/AuthenticatedVariableFormat.h>

 #include <Guid/SmmVariableCommon.h>

 #include "Variable.h"

 

+EFI_SMRAM_DESCRIPTOR                                 *mSmramRanges;

+UINTN                                                mSmramRangeCount;

+

 extern VARIABLE_INFO_ENTRY                           *gVariableInfo;

 EFI_HANDLE                                           mSmmVariableHandle      = NULL;

 EFI_HANDLE                                           mVariableHandle         = NULL;

@@ -62,6 +67,34 @@ AtRuntime (
 }

 

 /**

+  This function check if the address is in SMRAM.

+

+  @param Buffer  the buffer address to be checked.

+  @param Length  the buffer length to be checked.

+

+  @retval TRUE  this address is in SMRAM.

+  @retval FALSE this address is NOT in SMRAM.

+**/

+BOOLEAN

+InternalIsAddressInSmram (

+  IN EFI_PHYSICAL_ADDRESS  Buffer,

+  IN UINT64                Length

+  )

+{

+  UINTN  Index;

+

+  for (Index = 0; Index < mSmramRangeCount; Index ++) {

+    if (((Buffer >= mSmramRanges[Index].CpuStart) && (Buffer < mSmramRanges[Index].CpuStart + mSmramRanges[Index].PhysicalSize)) ||

+        ((mSmramRanges[Index].CpuStart >= Buffer) && (mSmramRanges[Index].CpuStart < Buffer + Length))) {

+      return TRUE;

+    }

+  }

+

+  return FALSE;

+}

+

+

+/**

   Initializes a basic mutual exclusion lock.

 

   This function initializes a basic mutual exclusion lock to the released state 

@@ -372,7 +405,6 @@ SmmVariableGetStatistics (
   @retval EFI_WARN_INTERRUPT_SOURCE_PENDING   The interrupt is still pending and other handlers should still 

                                               be called.

   @retval EFI_INTERRUPT_PENDING               The interrupt could not be quiesced.

-  @retval EFI_INVALID_PARAMETER               Input parameter is invalid.

 

 **/

 EFI_STATUS

@@ -392,14 +424,39 @@ SmmVariableHandler (
   VARIABLE_INFO_ENTRY                              *VariableInfo;

   UINTN                                            InfoSize;

 

-  if (CommBuffer == NULL) {

-    return EFI_INVALID_PARAMETER;

+  //

+  // If input is invalid, stop processing this SMI

+  //

+  if (CommBuffer == NULL || CommBufferSize == NULL) {

+    return EFI_SUCCESS;

   }

 

+  if (*CommBufferSize < sizeof(SMM_VARIABLE_COMMUNICATE_HEADER) - 1) {

+    return EFI_SUCCESS;

+  }

+

+  if (InternalIsAddressInSmram ((EFI_PHYSICAL_ADDRESS)(UINTN)CommBuffer, *CommBufferSize)) {

+    DEBUG ((EFI_D_ERROR, "SMM communication buffer size is in SMRAM!\n"));

+    return EFI_SUCCESS;

+  }

+  

   SmmVariableFunctionHeader = (SMM_VARIABLE_COMMUNICATE_HEADER *)CommBuffer;

+    

   switch (SmmVariableFunctionHeader->Function) {

     case SMM_VARIABLE_FUNCTION_GET_VARIABLE:

-      SmmVariableHeader = (SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE *) SmmVariableFunctionHeader->Data;     

+      SmmVariableHeader = (SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE *) SmmVariableFunctionHeader->Data;

+      InfoSize = OFFSET_OF(SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE, Name) 

+                 + SmmVariableHeader->DataSize + SmmVariableHeader->NameSize;

+

+      //

+      // SMRAM range check already covered before

+      //

+      if (InfoSize > *CommBufferSize - OFFSET_OF (SMM_VARIABLE_COMMUNICATE_HEADER, Data)) {

+        DEBUG ((EFI_D_ERROR, "Data size exceed communication buffer size limit!\n"));

+        Status = EFI_ACCESS_DENIED;

+        goto EXIT;

+      }

+

       Status = VariableServiceGetVariable (

                  SmmVariableHeader->Name,

                  &SmmVariableHeader->Guid,

@@ -411,6 +468,17 @@ SmmVariableHandler (
       

     case SMM_VARIABLE_FUNCTION_GET_NEXT_VARIABLE_NAME:

       GetNextVariableName = (SMM_VARIABLE_COMMUNICATE_GET_NEXT_VARIABLE_NAME *) SmmVariableFunctionHeader->Data;

+      InfoSize = OFFSET_OF(SMM_VARIABLE_COMMUNICATE_GET_NEXT_VARIABLE_NAME, Name) + GetNextVariableName->NameSize;

+

+      //

+      // SMRAM range check already covered before

+      //

+      if (InfoSize > *CommBufferSize - OFFSET_OF (SMM_VARIABLE_COMMUNICATE_HEADER, Data)) {

+        DEBUG ((EFI_D_ERROR, "Data size exceed communication buffer size limit!\n"));

+        Status = EFI_ACCESS_DENIED;

+        goto EXIT;

+      }

+

       Status = VariableServiceGetNextVariableName (

                  &GetNextVariableName->NameSize,

                  GetNextVariableName->Name,

@@ -431,6 +499,17 @@ SmmVariableHandler (
       

     case SMM_VARIABLE_FUNCTION_QUERY_VARIABLE_INFO:

       QueryVariableInfo = (SMM_VARIABLE_COMMUNICATE_QUERY_VARIABLE_INFO *) SmmVariableFunctionHeader->Data;

+      InfoSize = sizeof(SMM_VARIABLE_COMMUNICATE_QUERY_VARIABLE_INFO);

+

+      //

+      // SMRAM range check already covered before

+      //

+      if (InfoSize > *CommBufferSize - OFFSET_OF (SMM_VARIABLE_COMMUNICATE_HEADER, Data)) {

+        DEBUG ((EFI_D_ERROR, "Data size exceed communication buffer size limit!\n"));

+        Status = EFI_ACCESS_DENIED;

+        goto EXIT;

+      }

+  

       Status = VariableServiceQueryVariableInfo (

                  QueryVariableInfo->Attributes,

                  &QueryVariableInfo->MaximumVariableStorageSize,

@@ -452,17 +531,29 @@ SmmVariableHandler (
     case SMM_VARIABLE_FUNCTION_GET_STATISTICS:

       VariableInfo = (VARIABLE_INFO_ENTRY *) SmmVariableFunctionHeader->Data;

       InfoSize = *CommBufferSize - OFFSET_OF (SMM_VARIABLE_COMMUNICATE_HEADER, Data);

+

+      //

+      // Do not need to check SmmVariableFunctionHeader->Data in SMRAM here. 

+      // It is covered by previous CommBuffer check 

+      //

+     

+      if (InternalIsAddressInSmram ((EFI_PHYSICAL_ADDRESS)(UINTN)CommBufferSize, sizeof(UINTN))) {

+        DEBUG ((EFI_D_ERROR, "SMM communication buffer size is in SMRAM!\n"));

+        Status = EFI_ACCESS_DENIED;

+        goto EXIT;

+      }  

+

       Status = SmmVariableGetStatistics (VariableInfo, &InfoSize);

       *CommBufferSize = InfoSize + OFFSET_OF (SMM_VARIABLE_COMMUNICATE_HEADER, Data);

       break;

 

     default:

-      ASSERT (FALSE);

       Status = EFI_UNSUPPORTED;

   }

 

-  SmmVariableFunctionHeader->ReturnStatus = Status;

+EXIT:

 

+  SmmVariableFunctionHeader->ReturnStatus = Status;

   return EFI_SUCCESS;

 }

 

@@ -560,7 +651,9 @@ VariableServiceInitialize (
   EFI_STATUS                              Status;

   EFI_HANDLE                              VariableHandle;

   VOID                                    *SmmFtwRegistration;

-  

+  EFI_SMM_ACCESS2_PROTOCOL                *SmmAccess;

+  UINTN                                   Size;

+

   //

   // Variable initialize.

   //

@@ -579,6 +672,28 @@ VariableServiceInitialize (
                     );

   ASSERT_EFI_ERROR (Status);

 

+  //

+  // Get SMRAM information

+  //

+  Status = gBS->LocateProtocol (&gEfiSmmAccess2ProtocolGuid, NULL, (VOID **)&SmmAccess);

+  ASSERT_EFI_ERROR (Status);

+

+  Size = 0;

+  Status = SmmAccess->GetCapabilities (SmmAccess, &Size, NULL);

+  ASSERT (Status == EFI_BUFFER_TOO_SMALL);

+

+  Status = gSmst->SmmAllocatePool (

+                    EfiRuntimeServicesData,

+                    Size,

+                    (VOID **)&mSmramRanges

+                    );

+  ASSERT_EFI_ERROR (Status);

+

+  Status = SmmAccess->GetCapabilities (SmmAccess, &Size, mSmramRanges);

+  ASSERT_EFI_ERROR (Status);

+

+  mSmramRangeCount = Size / sizeof (EFI_SMRAM_DESCRIPTOR);

+

   ///

   /// Register SMM variable SMI handler

   ///

diff --git a/SecurityPkg/VariableAuthenticated/RuntimeDxe/VariableSmm.inf b/SecurityPkg/VariableAuthenticated/RuntimeDxe/VariableSmm.inf
index a6343dbbbd..e0aa40ac6d 100644
--- a/SecurityPkg/VariableAuthenticated/RuntimeDxe/VariableSmm.inf
+++ b/SecurityPkg/VariableAuthenticated/RuntimeDxe/VariableSmm.inf
@@ -72,6 +72,7 @@
   gEfiSmmFirmwareVolumeBlockProtocolGuid        ## SOMETIMES_CONSUMES

   gEfiSmmVariableProtocolGuid                   ## ALWAYS_PRODUCES

   gEfiSmmFaultTolerantWriteProtocolGuid         ## SOMETIMES_CONSUMES

+  gEfiSmmAccess2ProtocolGuid                    ## ALWAYS_CONSUMES

 

 [Guids]

   gEfiAuthenticatedVariableGuid                 ## PRODUCES ## Configuration Table Guid