Fix SMM Variable driver stack GetVariable return INVALID_PARAMETER when DataSize is bigger than SMM communication buffer.

Signed-off-by: Chao Zhang <chao.b.zhang@intel.com>
Reviewed-by  : Dong Guo   <guo.dong@intel.com>
Reviewed-by  : Fu Siyuan  <siyuan.fu@intel.com>

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

1 files changed, 28 insertions, 10 deletions

diff --git a/SecurityPkg/VariableAuthenticated/RuntimeDxe/VariableSmmRuntimeDxe.c b/SecurityPkg/VariableAuthenticated/RuntimeDxe/VariableSmmRuntimeDxe.c
index f1111b3b6b..9f750d6780 100644
--- a/SecurityPkg/VariableAuthenticated/RuntimeDxe/VariableSmmRuntimeDxe.c
+++ b/SecurityPkg/VariableAuthenticated/RuntimeDxe/VariableSmmRuntimeDxe.c
@@ -205,6 +205,8 @@ RuntimeServiceGetVariable (
   EFI_STATUS                                Status;

   UINTN                                     PayloadSize;

   SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE  *SmmVariableHeader;

+  UINTN                                     SmmCommBufPayloadSize;

+  UINTN                                     TempDataSize;

 

   if (VariableName == NULL || VendorGuid == NULL || DataSize == NULL) {

     return EFI_INVALID_PARAMETER;

@@ -214,13 +216,16 @@ RuntimeServiceGetVariable (
     return EFI_INVALID_PARAMETER;

   }

 

-  if (*DataSize >= mVariableBufferSize) {

-    //

-    // DataSize may be near MAX_ADDRESS incorrectly, this can cause the computed PayLoadSize to

-    // overflow to a small value and pass the check in InitCommunicateBuffer().

-    // To protect against this vulnerability, return EFI_INVALID_PARAMETER if DataSize is >= mVariableBufferSize.

-    // And there will be further check to ensure the total size is also not > mVariableBufferSize.

-    //

+  //

+  // SMM Communication Buffer max payload size

+  //

+  SmmCommBufPayloadSize = mVariableBufferSize - (SMM_COMMUNICATE_HEADER_SIZE + SMM_VARIABLE_COMMUNICATE_HEADER_SIZE);

+  TempDataSize          = *DataSize;

+

+  //

+  // If VariableName exceeds SMM payload limit. Return failure

+  //

+  if (StrSize (VariableName) > SmmCommBufPayloadSize - OFFSET_OF (SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE, Name)) {

     return EFI_INVALID_PARAMETER;

   }

 

@@ -230,7 +235,14 @@ RuntimeServiceGetVariable (
   // Init the communicate buffer. The buffer data size is:

   // SMM_COMMUNICATE_HEADER_SIZE + SMM_VARIABLE_COMMUNICATE_HEADER_SIZE + PayloadSize.

   //

-  PayloadSize = OFFSET_OF (SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE, Name) + StrSize (VariableName) + *DataSize;

+  if (TempDataSize > SmmCommBufPayloadSize - OFFSET_OF (SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE, Name) - StrSize (VariableName)) {

+    //

+    // If output data buffer exceed SMM payload limit. Trim output buffer to SMM payload size

+    //

+    TempDataSize = SmmCommBufPayloadSize - OFFSET_OF (SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE, Name) - StrSize (VariableName);

+  }

+  PayloadSize = OFFSET_OF (SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE, Name) + StrSize (VariableName) + TempDataSize;

+

   Status = InitCommunicateBuffer ((VOID **)&SmmVariableHeader, PayloadSize, SMM_VARIABLE_FUNCTION_GET_VARIABLE);

   if (EFI_ERROR (Status)) {

     goto Done;

@@ -238,7 +250,7 @@ RuntimeServiceGetVariable (
   ASSERT (SmmVariableHeader != NULL);

 

   CopyGuid (&SmmVariableHeader->Guid, VendorGuid);

-  SmmVariableHeader->DataSize   = *DataSize;

+  SmmVariableHeader->DataSize   = TempDataSize;

   SmmVariableHeader->NameSize   = StrSize (VariableName);

   if (Attributes == NULL) {

     SmmVariableHeader->Attributes = 0;

@@ -255,7 +267,13 @@ RuntimeServiceGetVariable (
   //

   // Get data from SMM.

   //

-  *DataSize = SmmVariableHeader->DataSize;

+  if (Status == EFI_SUCCESS || Status == EFI_BUFFER_TOO_SMALL) {

+    //

+    // SMM CommBuffer DataSize can be a trimed value

+    // Only update DataSize when needed

+    //

+    *DataSize = SmmVariableHeader->DataSize;

+  }

   if (Attributes != NULL) {

     *Attributes = SmmVariableHeader->Attributes;

   }