1. Use the check IsAddressValid() to prevent SMM communication buffer overflow in...

2. Refine the debug message.
3. Add check to make sure the input VariableName is A Null-terminated string.
4. Use local variable to hold StrSize (VariableName) to avoid duplicated StrSize calculation.

Signed-off-by: Star Zeng <star.zeng@intel.com>
Reviewed-by: Jiewen Yao <jiewen.yao@intel.com>
Reviewed-by: Chao Zhang <chao.b.zhang@intel.com>
git-svn-id: https://edk2.svn.sourceforge.net/svnroot/edk2/trunk/edk2@14317 6f19259b-4bc3-4df7-8a09-765794883524

diff --git a/EdkCompatibilityPkg/Compatibility/SmmBaseHelper/SmmBaseHelper.c b/EdkCompatibilityPkg/Compatibility/SmmBaseHelper/SmmBaseHelper.c
index 193ab197561c64047f0bc263d7a4e3d86b19be22..a2d89eaa213e6333be068190ee72996775a5fa06 100644 (file)
--- a/EdkCompatibilityPkg/Compatibility/SmmBaseHelper/SmmBaseHelper.c
+++ b/EdkCompatibilityPkg/Compatibility/SmmBaseHelper/SmmBaseHelper.c
@@ -11,7 +11,7 @@
 \r
   SmmHandlerEntry() will receive untrusted input and do validation.\r
 \r
-  Copyright (c) 2009 - 2012, Intel Corporation. All rights reserved.<BR>\r
+  Copyright (c) 2009 - 2013, Intel Corporation. All rights reserved.<BR>\r
   This program and the accompanying materials\r
   are licensed and made available under the terms and conditions of the BSD License\r
   which accompanies this distribution.  The full text of the license may be found at\r
@@ -732,6 +732,33 @@ IsAddressInSmram (
   return FALSE;\r
 }\r
 \r
+/**\r
+  This function check if the address refered by Buffer and Length is valid.\r
+\r
+  @param Buffer  the buffer address to be checked.\r
+  @param Length  the buffer length to be checked.\r
+\r
+  @retval TRUE  this address is valid.\r
+  @retval FALSE this address is NOT valid.\r
+**/\r
+BOOLEAN\r
+IsAddressValid (\r
+  IN UINTN                 Buffer,\r
+  IN UINTN                 Length\r
+  )\r
+{\r
+  if (Buffer > (MAX_ADDRESS - Length)) {\r
+    //\r
+    // Overflow happen\r
+    //\r
+    return FALSE;\r
+  }\r
+  if (IsAddressInSmram ((EFI_PHYSICAL_ADDRESS)Buffer, (UINT64)Length)) {\r
+    return FALSE;\r
+  }\r
+  return TRUE;\r
+}\r
+\r
 /** \r
   Thunk service of EFI_SMM_BASE_PROTOCOL.Register().\r
 \r
@@ -1068,7 +1095,7 @@ SmmHandlerEntry (
   ASSERT (CommBufferSize != NULL);\r
 \r
   if (*CommBufferSize == sizeof (SMMBASE_FUNCTION_DATA) &&\r
-      !IsAddressInSmram ((EFI_PHYSICAL_ADDRESS)(UINTN)CommBuffer, *CommBufferSize)) {\r
+      IsAddressValid ((UINTN)CommBuffer, *CommBufferSize)) {\r
     FunctionData = (SMMBASE_FUNCTION_DATA *)CommBuffer;\r
 \r
     switch (FunctionData->Function) {\r

diff --git a/MdeModulePkg/Library/SmmCorePerformanceLib/SmmCorePerformanceLib.c b/MdeModulePkg/Library/SmmCorePerformanceLib/SmmCorePerformanceLib.c
index 314d46d03a046012c1776841cb31221c34e50b6e..5755f2affc6bd2e0431823bd9b2d9f1c8371d853 100644 (file)
--- a/MdeModulePkg/Library/SmmCorePerformanceLib/SmmCorePerformanceLib.c
+++ b/MdeModulePkg/Library/SmmCorePerformanceLib/SmmCorePerformanceLib.c
@@ -16,7 +16,7 @@
 \r
  SmmPerformanceHandlerEx(), SmmPerformanceHandler() will receive untrusted input and do basic validation.\r
 \r
-Copyright (c) 2011 - 2012, Intel Corporation. All rights reserved.<BR>\r
+Copyright (c) 2011 - 2013, Intel Corporation. All rights reserved.<BR>\r
 This program and the accompanying materials\r
 are licensed and made available under the terms and conditions of the BSD License\r
 which accompanies this distribution.  The full text of the license may be found at\r
@@ -477,6 +477,33 @@ IsAddressInSmram (
   return FALSE;\r
 }\r
 \r
+/**\r
+  This function check if the address refered by Buffer and Length is valid.\r
+\r
+  @param Buffer  the buffer address to be checked.\r
+  @param Length  the buffer length to be checked.\r
+\r
+  @retval TRUE  this address is valid.\r
+  @retval FALSE this address is NOT valid.\r
+**/\r
+BOOLEAN\r
+IsAddressValid (\r
+  IN UINTN                 Buffer,\r
+  IN UINTN                 Length\r
+  )\r
+{\r
+  if (Buffer > (MAX_ADDRESS - Length)) {\r
+    //\r
+    // Overflow happen\r
+    //\r
+    return FALSE;\r
+  }\r
+  if (IsAddressInSmram ((EFI_PHYSICAL_ADDRESS)Buffer, (UINT64)Length)) {\r
+    return FALSE;\r
+  }\r
+  return TRUE;\r
+}\r
+\r
 /**\r
   Communication service SMI Handler entry.\r
 \r
@@ -527,8 +554,8 @@ SmmPerformanceHandlerEx (
     return EFI_SUCCESS;\r
   }\r
 \r
-  if (IsAddressInSmram ((EFI_PHYSICAL_ADDRESS)(UINTN)CommBuffer, *CommBufferSize)) {\r
-    DEBUG ((EFI_D_ERROR, "SMM communcation data buffer is in SMRAM!\n"));\r
+  if (!IsAddressValid ((UINTN)CommBuffer, *CommBufferSize)) {\r
+    DEBUG ((EFI_D_ERROR, "SMM communcation data buffer in SMRAM or overflow!\n"));\r
     return EFI_SUCCESS;\r
   }\r
   \r
@@ -551,8 +578,8 @@ SmmPerformanceHandlerEx (
        // Sanity check\r
        //\r
        DataSize = SmmPerfCommData->NumberOfEntries * sizeof(GAUGE_DATA_ENTRY_EX);\r
-       if (IsAddressInSmram ((EFI_PHYSICAL_ADDRESS)(UINTN)SmmPerfCommData->GaugeDataEx, DataSize)) {\r
-         DEBUG ((EFI_D_ERROR, "SMM Performance Data buffer is in SMRAM!\n"));\r
+       if (!IsAddressValid ((UINTN)SmmPerfCommData->GaugeDataEx, DataSize)) {\r
+         DEBUG ((EFI_D_ERROR, "SMM Performance Data buffer in SMRAM or overflow!\n"));\r
          Status = EFI_ACCESS_DENIED;\r
          break;\r
        }\r
@@ -628,8 +655,8 @@ SmmPerformanceHandler (
     return EFI_SUCCESS;\r
   }\r
 \r
-  if (IsAddressInSmram ((EFI_PHYSICAL_ADDRESS)(UINTN)CommBuffer, *CommBufferSize)) {\r
-    DEBUG ((EFI_D_ERROR, "SMM communcation data buffer is in SMRAM!\n"));\r
+  if (!IsAddressValid ((UINTN)CommBuffer, *CommBufferSize)) {\r
+    DEBUG ((EFI_D_ERROR, "SMM communcation data buffer in SMRAM or overflow!\n"));\r
     return EFI_SUCCESS;\r
   }\r
 \r
@@ -652,8 +679,8 @@ SmmPerformanceHandler (
        // Sanity check\r
        //\r
        DataSize = SmmPerfCommData->NumberOfEntries * sizeof(GAUGE_DATA_ENTRY);\r
-       if (IsAddressInSmram ((EFI_PHYSICAL_ADDRESS)(UINTN)SmmPerfCommData->GaugeData, DataSize)) {\r
-         DEBUG ((EFI_D_ERROR, "SMM Performance Data buffer is in SMRAM!\n"));\r
+       if (!IsAddressValid ((UINTN)SmmPerfCommData->GaugeData, DataSize)) {\r
+         DEBUG ((EFI_D_ERROR, "SMM Performance Data buffer in SMRAM or overflow!\n"));\r
          Status = EFI_ACCESS_DENIED;\r
          break;\r
        }\r

diff --git a/MdeModulePkg/Universal/Acpi/FirmwarePerformanceDataTableSmm/FirmwarePerformanceSmm.c b/MdeModulePkg/Universal/Acpi/FirmwarePerformanceDataTableSmm/FirmwarePerformanceSmm.c
index 54a2275d4ed908bcd0e480242d4de82885d6bd15..ebf81ca6f72ce1106e1db10b5eebdd6a0efbf9ce 100644 (file)
--- a/MdeModulePkg/Universal/Acpi/FirmwarePerformanceDataTableSmm/FirmwarePerformanceSmm.c
+++ b/MdeModulePkg/Universal/Acpi/FirmwarePerformanceDataTableSmm/FirmwarePerformanceSmm.c
@@ -11,7 +11,7 @@
 \r
   FpdtSmiHandler() will receive untrusted input and do basic validation.\r
 \r
-  Copyright (c) 2011 - 2012, Intel Corporation. All rights reserved.<BR>\r
+  Copyright (c) 2011 - 2013, Intel Corporation. All rights reserved.<BR>\r
   This program and the accompanying materials\r
   are licensed and made available under the terms and conditions of the BSD License\r
   which accompanies this distribution.  The full text of the license may be found at\r
@@ -204,6 +204,33 @@ InternalIsAddressInSmram (
   return FALSE;\r
 }\r
 \r
+/**\r
+  This function check if the address refered by Buffer and Length is valid.\r
+\r
+  @param Buffer  the buffer address to be checked.\r
+  @param Length  the buffer length to be checked.\r
+\r
+  @retval TRUE  this address is valid.\r
+  @retval FALSE this address is NOT valid.\r
+**/\r
+BOOLEAN\r
+InternalIsAddressValid (\r
+  IN UINTN                 Buffer,\r
+  IN UINTN                 Length\r
+  )\r
+{\r
+  if (Buffer > (MAX_ADDRESS - Length)) {\r
+    //\r
+    // Overflow happen\r
+    //\r
+    return FALSE;\r
+  }\r
+  if (InternalIsAddressInSmram ((EFI_PHYSICAL_ADDRESS)Buffer, (UINT64)Length)) {\r
+    return FALSE;\r
+  }\r
+  return TRUE;\r
+}\r
+\r
 /**\r
   Communication service SMI Handler entry.\r
 \r
@@ -251,8 +278,8 @@ FpdtSmiHandler (
     return EFI_SUCCESS;\r
   }\r
   \r
-  if (InternalIsAddressInSmram ((EFI_PHYSICAL_ADDRESS)(UINTN)CommBuffer, *CommBufferSize)) {\r
-    DEBUG ((EFI_D_ERROR, "SMM communication data buffer is in SMRAM!\n"));\r
+  if (!InternalIsAddressValid ((UINTN)CommBuffer, *CommBufferSize)) {\r
+    DEBUG ((EFI_D_ERROR, "SMM communication data buffer in SMRAM or overflow!\n"));\r
     return EFI_SUCCESS;\r
   }\r
 \r
@@ -275,8 +302,8 @@ FpdtSmiHandler (
        // Sanity check\r
        //\r
        SmmCommData->BootRecordSize = mBootRecordSize;\r
-       if (InternalIsAddressInSmram ((EFI_PHYSICAL_ADDRESS)(UINTN)SmmCommData->BootRecordData, mBootRecordSize)) {\r
-         DEBUG ((EFI_D_ERROR, "SMM Data buffer is in SMRAM!\n"));\r
+       if (!InternalIsAddressValid ((UINTN)SmmCommData->BootRecordData, mBootRecordSize)) {\r
+         DEBUG ((EFI_D_ERROR, "SMM Data buffer in SMRAM or overflow!\n"));\r
          Status = EFI_ACCESS_DENIED;\r
          break;\r
        }\r

diff --git a/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteSmm.c b/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteSmm.c
index 13c709658f255f5bb5ec61218065d6372f67bc67..8b88ae41ee5df33d86b5c74f2cda680814594b1e 100644 (file)
--- a/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteSmm.c
+++ b/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteSmm.c
@@ -99,6 +99,32 @@ InternalIsAddressInSmram (
   return FALSE;\r
 }\r
 \r
+/**\r
+  This function check if the address refered by Buffer and Length is valid.\r
+\r
+  @param Buffer  the buffer address to be checked.\r
+  @param Length  the buffer length to be checked.\r
+\r
+  @retval TRUE  this address is valid.\r
+  @retval FALSE this address is NOT valid.\r
+**/\r
+BOOLEAN\r
+InternalIsAddressValid (\r
+  IN UINTN                 Buffer,\r
+  IN UINTN                 Length\r
+  )\r
+{\r
+  if (Buffer > (MAX_ADDRESS - Length)) {\r
+    //\r
+    // Overflow happen\r
+    //\r
+    return FALSE;\r
+  }\r
+  if (InternalIsAddressInSmram ((EFI_PHYSICAL_ADDRESS)Buffer, (UINT64)Length)) {\r
+    return FALSE;\r
+  }\r
+  return TRUE;\r
+}\r
 \r
 /**\r
   Retrive the SMM FVB protocol interface by HANDLE.\r
@@ -356,8 +382,8 @@ SmmFaultTolerantWriteHandler (
     return EFI_SUCCESS;\r
   }\r
 \r
-  if (InternalIsAddressInSmram ((EFI_PHYSICAL_ADDRESS)(UINTN)CommBuffer, *CommBufferSize)) {\r
-    DEBUG ((EFI_D_ERROR, "SMM communication buffer size is in SMRAM!\n"));\r
+  if (!InternalIsAddressValid ((UINTN)CommBuffer, *CommBufferSize)) {\r
+    DEBUG ((EFI_D_ERROR, "SMM communication buffer in SMRAM or overflow!\n"));\r
     return EFI_SUCCESS;\r
   }\r
 \r

diff --git a/MdeModulePkg/Universal/LockBox/SmmLockBox/SmmLockBox.c b/MdeModulePkg/Universal/LockBox/SmmLockBox/SmmLockBox.c
index e8b831e1fb5ba576a795412a5a243ab48f7c297f..7a0d6d29a5f7685e586307b13028bd5e056cd817 100644 (file)
--- a/MdeModulePkg/Universal/LockBox/SmmLockBox/SmmLockBox.c
+++ b/MdeModulePkg/Universal/LockBox/SmmLockBox/SmmLockBox.c
@@ -9,7 +9,7 @@
   SmmLockBoxHandler(), SmmLockBoxRestore(), SmmLockBoxUpdate(), SmmLockBoxSave()\r
   will receive untrusted input and do basic validation.\r
 \r
-Copyright (c) 2010 - 2012, Intel Corporation. All rights reserved.<BR>\r
+Copyright (c) 2010 - 2013, Intel Corporation. All rights reserved.<BR>\r
 \r
 This program and the accompanying materials\r
 are licensed and made available under the terms and conditions\r
@@ -125,7 +125,7 @@ SmmLockBoxSave (
   // Sanity check\r
   //\r
   if (!IsAddressValid ((UINTN)LockBoxParameterSave->Buffer, (UINTN)LockBoxParameterSave->Length)) {\r
-    DEBUG ((EFI_D_ERROR, "SmmLockBox Save address in SMRAM!\n"));\r
+    DEBUG ((EFI_D_ERROR, "SmmLockBox Save address in SMRAM or buffer overflow!\n"));\r
     LockBoxParameterSave->Header.ReturnStatus = (UINT64)EFI_ACCESS_DENIED;\r
     return ;\r
   }\r
@@ -203,7 +203,7 @@ SmmLockBoxUpdate (
   // Sanity check\r
   //\r
   if (!IsAddressValid ((UINTN)LockBoxParameterUpdate->Buffer, (UINTN)LockBoxParameterUpdate->Length)) {\r
-    DEBUG ((EFI_D_ERROR, "SmmLockBox Update address in SMRAM!\n"));\r
+    DEBUG ((EFI_D_ERROR, "SmmLockBox Update address in SMRAM or buffer overflow!\n"));\r
     LockBoxParameterUpdate->Header.ReturnStatus = (UINT64)EFI_ACCESS_DENIED;\r
     return ;\r
   }\r
@@ -241,7 +241,7 @@ SmmLockBoxRestore (
   // Sanity check\r
   //\r
   if (!IsAddressValid ((UINTN)LockBoxParameterRestore->Buffer, (UINTN)LockBoxParameterRestore->Length)) {\r
-    DEBUG ((EFI_D_ERROR, "SmmLockBox Restore address in SMRAM!\n"));\r
+    DEBUG ((EFI_D_ERROR, "SmmLockBox Restore address in SMRAM or buffer overflow!\n"));\r
     LockBoxParameterRestore->Header.ReturnStatus = (UINT64)EFI_ACCESS_DENIED;\r
     return ;\r
   }\r
@@ -320,7 +320,7 @@ SmmLockBoxHandler (
     return EFI_SUCCESS;\r
   }\r
   if (!IsAddressValid ((UINTN)CommBuffer, *CommBufferSize)) {\r
-    DEBUG ((EFI_D_ERROR, "SmmLockBox Command Buffer in SMRAM!\n"));\r
+    DEBUG ((EFI_D_ERROR, "SmmLockBox Command Buffer in SMRAM or overflow!\n"));\r
     return EFI_SUCCESS;\r
   }\r
 \r

diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c
index 7541f6ae1d2606144c1b4778f48469df162a4637..f8e6bd58828ae592e205d4fdeafe94961cee767b 100644 (file)
--- a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c
+++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c
@@ -93,6 +93,32 @@ InternalIsAddressInSmram (
   return FALSE;\r
 }\r
 \r
+/**\r
+  This function check if the address refered by Buffer and Length is valid.\r
+\r
+  @param Buffer  the buffer address to be checked.\r
+  @param Length  the buffer length to be checked.\r
+\r
+  @retval TRUE  this address is valid.\r
+  @retval FALSE this address is NOT valid.\r
+**/\r
+BOOLEAN\r
+InternalIsAddressValid (\r
+  IN UINTN                 Buffer,\r
+  IN UINTN                 Length\r
+  )\r
+{\r
+  if (Buffer > (MAX_ADDRESS - Length)) {\r
+    //\r
+    // Overflow happen\r
+    //\r
+    return FALSE;\r
+  }\r
+  if (InternalIsAddressInSmram ((EFI_PHYSICAL_ADDRESS)Buffer, (UINT64)Length)) {\r
+    return FALSE;\r
+  }\r
+  return TRUE;\r
+}\r
 \r
 /**\r
   Initializes a basic mutual exclusion lock.\r
@@ -418,6 +444,7 @@ SmmVariableHandler (
   SMM_VARIABLE_COMMUNICATE_QUERY_VARIABLE_INFO     *QueryVariableInfo;\r
   VARIABLE_INFO_ENTRY                              *VariableInfo;\r
   UINTN                                            InfoSize;\r
+  UINTN                                            NameBufferSize;\r
 \r
   //\r
   // If input is invalid, stop processing this SMI\r
@@ -430,8 +457,8 @@ SmmVariableHandler (
     return EFI_SUCCESS;\r
   }\r
 \r
-  if (InternalIsAddressInSmram ((EFI_PHYSICAL_ADDRESS)(UINTN)CommBuffer, *CommBufferSize)) {\r
-    DEBUG ((EFI_D_ERROR, "SMM communication buffer size is in SMRAM!\n"));\r
+  if (!InternalIsAddressValid ((UINTN)CommBuffer, *CommBufferSize)) {\r
+    DEBUG ((EFI_D_ERROR, "SMM communication buffer in SMRAM or overflow!\n"));\r
     return EFI_SUCCESS;\r
   }\r
 \r
@@ -439,6 +466,14 @@ SmmVariableHandler (
   switch (SmmVariableFunctionHeader->Function) {\r
     case SMM_VARIABLE_FUNCTION_GET_VARIABLE:\r
       SmmVariableHeader = (SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE *) SmmVariableFunctionHeader->Data;\r
+      if (((UINTN)(~0) - SmmVariableHeader->DataSize < OFFSET_OF(SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE, Name)) ||\r
+         ((UINTN)(~0) - SmmVariableHeader->NameSize < OFFSET_OF(SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE, Name) + SmmVariableHeader->DataSize)) {\r
+        //\r
+        // Prevent InfoSize overflow happen\r
+        //\r
+        Status = EFI_ACCESS_DENIED;\r
+        goto EXIT;\r
+      }\r
       InfoSize = OFFSET_OF(SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE, Name) \r
                  + SmmVariableHeader->DataSize + SmmVariableHeader->NameSize;\r
 \r
@@ -451,6 +486,14 @@ SmmVariableHandler (
         goto EXIT;\r
       }\r
 \r
+      if (SmmVariableHeader->NameSize < sizeof (CHAR16) || SmmVariableHeader->Name[SmmVariableHeader->NameSize/sizeof (CHAR16) - 1] != L'\0') {\r
+        //\r
+        // Make sure VariableName is A Null-terminated string.\r
+        //\r
+        Status = EFI_ACCESS_DENIED;\r
+        goto EXIT;\r
+      }\r
+\r
       Status = VariableServiceGetVariable (\r
                  SmmVariableHeader->Name,\r
                  &SmmVariableHeader->Guid,\r
@@ -462,6 +505,13 @@ SmmVariableHandler (
       \r
     case SMM_VARIABLE_FUNCTION_GET_NEXT_VARIABLE_NAME:\r
       GetNextVariableName = (SMM_VARIABLE_COMMUNICATE_GET_NEXT_VARIABLE_NAME *) SmmVariableFunctionHeader->Data;\r
+      if ((UINTN)(~0) - GetNextVariableName->NameSize < OFFSET_OF(SMM_VARIABLE_COMMUNICATE_GET_NEXT_VARIABLE_NAME, Name)) {\r
+        //\r
+        // Prevent InfoSize overflow happen\r
+        //\r
+        Status = EFI_ACCESS_DENIED;\r
+        goto EXIT;\r
+      }\r
       InfoSize = OFFSET_OF(SMM_VARIABLE_COMMUNICATE_GET_NEXT_VARIABLE_NAME, Name) + GetNextVariableName->NameSize;\r
 \r
       //\r
@@ -473,6 +523,15 @@ SmmVariableHandler (
         goto EXIT;\r
       }\r
 \r
+      NameBufferSize = *CommBufferSize - SMM_VARIABLE_COMMUNICATE_HEADER_SIZE -  OFFSET_OF(SMM_VARIABLE_COMMUNICATE_GET_NEXT_VARIABLE_NAME, Name);\r
+      if (NameBufferSize < sizeof (CHAR16) || GetNextVariableName->Name[NameBufferSize/sizeof (CHAR16) - 1] != L'\0') {\r
+        //\r
+        // Make sure input VariableName is A Null-terminated string.\r
+        //\r
+        Status = EFI_ACCESS_DENIED;\r
+        goto EXIT;\r
+      }\r
+\r
       Status = VariableServiceGetNextVariableName (\r
                  &GetNextVariableName->NameSize,\r
                  GetNextVariableName->Name,\r
@@ -482,6 +541,14 @@ SmmVariableHandler (
       \r
     case SMM_VARIABLE_FUNCTION_SET_VARIABLE:\r
       SmmVariableHeader = (SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE *) SmmVariableFunctionHeader->Data;\r
+      if (((UINTN)(~0) - SmmVariableHeader->DataSize < OFFSET_OF(SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE, Name)) ||\r
+         ((UINTN)(~0) - SmmVariableHeader->NameSize < OFFSET_OF(SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE, Name) + SmmVariableHeader->DataSize)) {\r
+        //\r
+        // Prevent InfoSize overflow happen\r
+        //\r
+        Status = EFI_ACCESS_DENIED;\r
+        goto EXIT;\r
+      }\r
       InfoSize = OFFSET_OF(SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE, Name)\r
                  + SmmVariableHeader->DataSize + SmmVariableHeader->NameSize;\r
 \r
@@ -495,6 +562,14 @@ SmmVariableHandler (
         goto EXIT;\r
       }\r
 \r
+      if (SmmVariableHeader->NameSize < sizeof (CHAR16) || SmmVariableHeader->Name[SmmVariableHeader->NameSize/sizeof (CHAR16) - 1] != L'\0') {\r
+        //\r
+        // Make sure VariableName is A Null-terminated string.\r
+        //\r
+        Status = EFI_ACCESS_DENIED;\r
+        goto EXIT;\r
+      }\r
+\r
       Status = VariableServiceSetVariable (\r
                  SmmVariableHeader->Name,\r
                  &SmmVariableHeader->Guid,\r
@@ -549,7 +624,7 @@ SmmVariableHandler (
       //\r
      \r
       if (InternalIsAddressInSmram ((EFI_PHYSICAL_ADDRESS)(UINTN)CommBufferSize, sizeof(UINTN))) {\r
-        DEBUG ((EFI_D_ERROR, "SMM communication buffer size is in SMRAM!\n"));\r
+        DEBUG ((EFI_D_ERROR, "SMM communication buffer in SMRAM!\n"));\r
         Status = EFI_ACCESS_DENIED;\r
         goto EXIT;\r
       }  \r

diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.c b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.c
index 4d60da120511ae5454963773b79fd20544f54c16..e76600ee6a01aaf5ea7a86e331b6c7225707dd39 100644 (file)
--- a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.c
+++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.c
@@ -191,6 +191,7 @@ RuntimeServiceGetVariable (
   SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE  *SmmVariableHeader;\r
   UINTN                                     SmmCommBufPayloadSize;\r
   UINTN                                     TempDataSize;\r
+  UINTN                                     VariableNameSize;\r
 \r
   if (VariableName == NULL || VendorGuid == NULL || DataSize == NULL) {\r
     return EFI_INVALID_PARAMETER;\r
@@ -205,11 +206,12 @@ RuntimeServiceGetVariable (
   //\r
   SmmCommBufPayloadSize = mVariableBufferSize - (SMM_COMMUNICATE_HEADER_SIZE + SMM_VARIABLE_COMMUNICATE_HEADER_SIZE);\r
   TempDataSize          = *DataSize;\r
+  VariableNameSize      = StrSize (VariableName);\r
 \r
   //\r
   // If VariableName exceeds SMM payload limit. Return failure\r
   //\r
-  if (StrSize (VariableName) > SmmCommBufPayloadSize - OFFSET_OF (SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE, Name)) {\r
+  if (VariableNameSize > SmmCommBufPayloadSize - OFFSET_OF (SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE, Name)) {\r
     return EFI_INVALID_PARAMETER;\r
   }\r
 \r
@@ -219,13 +221,13 @@ RuntimeServiceGetVariable (
   // Init the communicate buffer. The buffer data size is:\r
   // SMM_COMMUNICATE_HEADER_SIZE + SMM_VARIABLE_COMMUNICATE_HEADER_SIZE + PayloadSize.\r
   //\r
-  if (TempDataSize > SmmCommBufPayloadSize - OFFSET_OF (SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE, Name) - StrSize (VariableName)) {\r
+  if (TempDataSize > SmmCommBufPayloadSize - OFFSET_OF (SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE, Name) - VariableNameSize) {\r
     //\r
     // If output data buffer exceed SMM payload limit. Trim output buffer to SMM payload size\r
     //\r
-    TempDataSize = SmmCommBufPayloadSize - OFFSET_OF (SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE, Name) - StrSize (VariableName);\r
+    TempDataSize = SmmCommBufPayloadSize - OFFSET_OF (SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE, Name) - VariableNameSize;\r
   }\r
-  PayloadSize = OFFSET_OF (SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE, Name) + StrSize (VariableName) + TempDataSize;\r
+  PayloadSize = OFFSET_OF (SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE, Name) + VariableNameSize + TempDataSize;\r
 \r
   Status = InitCommunicateBuffer ((VOID **)&SmmVariableHeader, PayloadSize, SMM_VARIABLE_FUNCTION_GET_VARIABLE);\r
   if (EFI_ERROR (Status)) {\r
@@ -235,7 +237,7 @@ RuntimeServiceGetVariable (
 \r
   CopyGuid (&SmmVariableHeader->Guid, VendorGuid);\r
   SmmVariableHeader->DataSize   = TempDataSize;\r
-  SmmVariableHeader->NameSize   = StrSize (VariableName);\r
+  SmmVariableHeader->NameSize   = VariableNameSize;\r
   if (Attributes == NULL) {\r
     SmmVariableHeader->Attributes = 0;\r
   } else {\r
@@ -299,6 +301,8 @@ RuntimeServiceGetNextVariableName (
   UINTN                                           PayloadSize;\r
   SMM_VARIABLE_COMMUNICATE_GET_NEXT_VARIABLE_NAME *SmmGetNextVariableName;\r
   UINTN                                           SmmCommBufPayloadSize;\r
+  UINTN                                           OutVariableNameSize;\r
+  UINTN                                           InVariableNameSize;\r
 \r
   if (VariableNameSize == NULL || VariableName == NULL || VendorGuid == NULL) {\r
     return EFI_INVALID_PARAMETER;\r
@@ -308,11 +312,13 @@ RuntimeServiceGetNextVariableName (
   // SMM Communication Buffer max payload size\r
   //\r
   SmmCommBufPayloadSize = mVariableBufferSize - (SMM_COMMUNICATE_HEADER_SIZE + SMM_VARIABLE_COMMUNICATE_HEADER_SIZE);\r
+  OutVariableNameSize   = *VariableNameSize;\r
+  InVariableNameSize    = StrSize (VariableName);\r
 \r
   //\r
   // If input string exceeds SMM payload limit. Return failure\r
   //\r
-  if (StrSize (VariableName) > SmmCommBufPayloadSize - OFFSET_OF (SMM_VARIABLE_COMMUNICATE_GET_NEXT_VARIABLE_NAME, Name)) {\r
+  if (InVariableNameSize > SmmCommBufPayloadSize - OFFSET_OF (SMM_VARIABLE_COMMUNICATE_GET_NEXT_VARIABLE_NAME, Name)) {\r
     return EFI_INVALID_PARAMETER;\r
   }\r
 \r
@@ -322,16 +328,16 @@ RuntimeServiceGetNextVariableName (
   // Init the communicate buffer. The buffer data size is:\r
   // SMM_COMMUNICATE_HEADER_SIZE + SMM_VARIABLE_COMMUNICATE_HEADER_SIZE + PayloadSize.\r
   //\r
-  if (*VariableNameSize > SmmCommBufPayloadSize - OFFSET_OF (SMM_VARIABLE_COMMUNICATE_GET_NEXT_VARIABLE_NAME, Name)) {\r
+  if (OutVariableNameSize > SmmCommBufPayloadSize - OFFSET_OF (SMM_VARIABLE_COMMUNICATE_GET_NEXT_VARIABLE_NAME, Name)) {\r
     //\r
     // If output buffer exceed SMM payload limit. Trim output buffer to SMM payload size\r
     //\r
-    *VariableNameSize = SmmCommBufPayloadSize - OFFSET_OF (SMM_VARIABLE_COMMUNICATE_GET_NEXT_VARIABLE_NAME, Name);\r
+    OutVariableNameSize = SmmCommBufPayloadSize - OFFSET_OF (SMM_VARIABLE_COMMUNICATE_GET_NEXT_VARIABLE_NAME, Name);\r
   }\r
   //\r
   // Payload should be Guid + NameSize + MAX of Input & Output buffer\r
   //\r
-  PayloadSize = OFFSET_OF (SMM_VARIABLE_COMMUNICATE_GET_NEXT_VARIABLE_NAME, Name) + MAX (*VariableNameSize, StrSize (VariableName));\r
+  PayloadSize = OFFSET_OF (SMM_VARIABLE_COMMUNICATE_GET_NEXT_VARIABLE_NAME, Name) + MAX (OutVariableNameSize, InVariableNameSize);\r
 \r
 \r
   Status = InitCommunicateBuffer ((VOID **)&SmmGetNextVariableName, PayloadSize, SMM_VARIABLE_FUNCTION_GET_NEXT_VARIABLE_NAME);\r
@@ -343,13 +349,16 @@ RuntimeServiceGetNextVariableName (
   //\r
   // SMM comm buffer->NameSize is buffer size for return string\r
   //\r
-  SmmGetNextVariableName->NameSize = *VariableNameSize;\r
+  SmmGetNextVariableName->NameSize = OutVariableNameSize;\r
 \r
   CopyGuid (&SmmGetNextVariableName->Guid, VendorGuid);\r
   //\r
   // Copy whole string\r
   //\r
-  CopyMem (SmmGetNextVariableName->Name, VariableName, StrSize (VariableName));\r
+  CopyMem (SmmGetNextVariableName->Name, VariableName, InVariableNameSize);\r
+  if (OutVariableNameSize > InVariableNameSize) {\r
+    ZeroMem ((UINT8 *) SmmGetNextVariableName->Name + InVariableNameSize, OutVariableNameSize - InVariableNameSize);\r
+  }\r
 \r
   //\r
   // Send data to SMM\r
@@ -359,7 +368,13 @@ RuntimeServiceGetNextVariableName (
   //\r
   // Get data from SMM.\r
   //\r
-  *VariableNameSize = SmmGetNextVariableName->NameSize;    \r
+  if (Status == EFI_SUCCESS || Status == EFI_BUFFER_TOO_SMALL) {\r
+    //\r
+    // SMM CommBuffer NameSize can be a trimed value\r
+    // Only update VariableNameSize when needed\r
+    //\r
+    *VariableNameSize = SmmGetNextVariableName->NameSize;\r
+  }\r
   if (EFI_ERROR (Status)) {\r
     goto Done;\r
   }\r
@@ -402,6 +417,7 @@ RuntimeServiceSetVariable (
   EFI_STATUS                                Status;\r
   UINTN                                     PayloadSize; \r
   SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE  *SmmVariableHeader;\r
+  UINTN                                     VariableNameSize;\r
     \r
   //\r
   // Check input parameters.\r
@@ -423,8 +439,9 @@ RuntimeServiceSetVariable (
     //\r
     return EFI_INVALID_PARAMETER;\r
   }\r
+  VariableNameSize      = StrSize (VariableName);\r
 \r
-  if ((UINTN)(~0) - StrSize (VariableName) < OFFSET_OF (SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE, Name) + DataSize) {\r
+  if ((UINTN)(~0) - VariableNameSize < OFFSET_OF (SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE, Name) + DataSize) {\r
     //\r
     // Prevent PayloadSize overflow\r
     //\r
@@ -437,7 +454,7 @@ RuntimeServiceSetVariable (
   // Init the communicate buffer. The buffer data size is:\r
   // SMM_COMMUNICATE_HEADER_SIZE + SMM_VARIABLE_COMMUNICATE_HEADER_SIZE + PayloadSize.\r
   //\r
-  PayloadSize = OFFSET_OF (SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE, Name) + StrSize (VariableName) + DataSize;\r
+  PayloadSize = OFFSET_OF (SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE, Name) + VariableNameSize + DataSize;\r
   Status = InitCommunicateBuffer ((VOID **)&SmmVariableHeader, PayloadSize, SMM_VARIABLE_FUNCTION_SET_VARIABLE);\r
   if (EFI_ERROR (Status)) {\r
     goto Done;\r
@@ -446,7 +463,7 @@ RuntimeServiceSetVariable (
 \r
   CopyGuid ((EFI_GUID *) &SmmVariableHeader->Guid, VendorGuid);\r
   SmmVariableHeader->DataSize   = DataSize;\r
-  SmmVariableHeader->NameSize   = StrSize (VariableName);\r
+  SmmVariableHeader->NameSize   = VariableNameSize;\r
   SmmVariableHeader->Attributes = Attributes;\r
   CopyMem (SmmVariableHeader->Name, VariableName, SmmVariableHeader->NameSize);\r
   CopyMem ((UINT8 *) SmmVariableHeader->Name + SmmVariableHeader->NameSize, Data, DataSize);\r

diff --git a/SecurityPkg/VariableAuthenticated/RuntimeDxe/VariableSmm.c b/SecurityPkg/VariableAuthenticated/RuntimeDxe/VariableSmm.c
index 678cff3c7d2306e0e832434b5580bdfdae85e115..47bc390f6fbc4ebbab5ba07d42317872a6de2af8 100644 (file)
--- a/SecurityPkg/VariableAuthenticated/RuntimeDxe/VariableSmm.c
+++ b/SecurityPkg/VariableAuthenticated/RuntimeDxe/VariableSmm.c
@@ -93,6 +93,32 @@ InternalIsAddressInSmram (
   return FALSE;\r
 }\r
 \r
+/**\r
+  This function check if the address refered by Buffer and Length is valid.\r
+\r
+  @param Buffer  the buffer address to be checked.\r
+  @param Length  the buffer length to be checked.\r
+\r
+  @retval TRUE  this address is valid.\r
+  @retval FALSE this address is NOT valid.\r
+**/\r
+BOOLEAN\r
+InternalIsAddressValid (\r
+  IN UINTN                 Buffer,\r
+  IN UINTN                 Length\r
+  )\r
+{\r
+  if (Buffer > (MAX_ADDRESS - Length)) {\r
+    //\r
+    // Overflow happen\r
+    //\r
+    return FALSE;\r
+  }\r
+  if (InternalIsAddressInSmram ((EFI_PHYSICAL_ADDRESS)Buffer, (UINT64)Length)) {\r
+    return FALSE;\r
+  }\r
+  return TRUE;\r
+}\r
 \r
 /**\r
   Initializes a basic mutual exclusion lock.\r
@@ -423,6 +449,7 @@ SmmVariableHandler (
   SMM_VARIABLE_COMMUNICATE_QUERY_VARIABLE_INFO     *QueryVariableInfo;\r
   VARIABLE_INFO_ENTRY                              *VariableInfo;\r
   UINTN                                            InfoSize;\r
+  UINTN                                            NameBufferSize;\r
 \r
   //\r
   // If input is invalid, stop processing this SMI\r
@@ -435,8 +462,8 @@ SmmVariableHandler (
     return EFI_SUCCESS;\r
   }\r
 \r
-  if (InternalIsAddressInSmram ((EFI_PHYSICAL_ADDRESS)(UINTN)CommBuffer, *CommBufferSize)) {\r
-    DEBUG ((EFI_D_ERROR, "SMM communication buffer size is in SMRAM!\n"));\r
+  if (!InternalIsAddressValid ((UINTN)CommBuffer, *CommBufferSize)) {\r
+    DEBUG ((EFI_D_ERROR, "SMM communication buffer in SMRAM or overflow!\n"));\r
     return EFI_SUCCESS;\r
   }\r
   \r
@@ -445,6 +472,14 @@ SmmVariableHandler (
   switch (SmmVariableFunctionHeader->Function) {\r
     case SMM_VARIABLE_FUNCTION_GET_VARIABLE:\r
       SmmVariableHeader = (SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE *) SmmVariableFunctionHeader->Data;\r
+      if (((UINTN)(~0) - SmmVariableHeader->DataSize < OFFSET_OF(SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE, Name)) ||\r
+         ((UINTN)(~0) - SmmVariableHeader->NameSize < OFFSET_OF(SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE, Name) + SmmVariableHeader->DataSize)) {\r
+        //\r
+        // Prevent InfoSize overflow happen\r
+        //\r
+        Status = EFI_ACCESS_DENIED;\r
+        goto EXIT;\r
+      }\r
       InfoSize = OFFSET_OF(SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE, Name) \r
                  + SmmVariableHeader->DataSize + SmmVariableHeader->NameSize;\r
 \r
@@ -457,6 +492,14 @@ SmmVariableHandler (
         goto EXIT;\r
       }\r
 \r
+      if (SmmVariableHeader->NameSize < sizeof (CHAR16) || SmmVariableHeader->Name[SmmVariableHeader->NameSize/sizeof (CHAR16) - 1] != L'\0') {\r
+        //\r
+        // Make sure VariableName is A Null-terminated string.\r
+        //\r
+        Status = EFI_ACCESS_DENIED;\r
+        goto EXIT;\r
+      }\r
+\r
       Status = VariableServiceGetVariable (\r
                  SmmVariableHeader->Name,\r
                  &SmmVariableHeader->Guid,\r
@@ -468,6 +511,13 @@ SmmVariableHandler (
       \r
     case SMM_VARIABLE_FUNCTION_GET_NEXT_VARIABLE_NAME:\r
       GetNextVariableName = (SMM_VARIABLE_COMMUNICATE_GET_NEXT_VARIABLE_NAME *) SmmVariableFunctionHeader->Data;\r
+      if ((UINTN)(~0) - GetNextVariableName->NameSize < OFFSET_OF(SMM_VARIABLE_COMMUNICATE_GET_NEXT_VARIABLE_NAME, Name)) {\r
+        //\r
+        // Prevent InfoSize overflow happen\r
+        //\r
+        Status = EFI_ACCESS_DENIED;\r
+        goto EXIT;\r
+      }\r
       InfoSize = OFFSET_OF(SMM_VARIABLE_COMMUNICATE_GET_NEXT_VARIABLE_NAME, Name) + GetNextVariableName->NameSize;\r
 \r
       //\r
@@ -479,6 +529,15 @@ SmmVariableHandler (
         goto EXIT;\r
       }\r
 \r
+      NameBufferSize = *CommBufferSize - SMM_VARIABLE_COMMUNICATE_HEADER_SIZE -  OFFSET_OF(SMM_VARIABLE_COMMUNICATE_GET_NEXT_VARIABLE_NAME, Name);\r
+      if (NameBufferSize < sizeof (CHAR16) || GetNextVariableName->Name[NameBufferSize/sizeof (CHAR16) - 1] != L'\0') {\r
+        //\r
+        // Make sure input VariableName is A Null-terminated string.\r
+        //\r
+        Status = EFI_ACCESS_DENIED;\r
+        goto EXIT;\r
+      }\r
+\r
       Status = VariableServiceGetNextVariableName (\r
                  &GetNextVariableName->NameSize,\r
                  GetNextVariableName->Name,\r
@@ -488,6 +547,14 @@ SmmVariableHandler (
       \r
     case SMM_VARIABLE_FUNCTION_SET_VARIABLE:\r
       SmmVariableHeader = (SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE *) SmmVariableFunctionHeader->Data;\r
+      if (((UINTN)(~0) - SmmVariableHeader->DataSize < OFFSET_OF(SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE, Name)) ||\r
+         ((UINTN)(~0) - SmmVariableHeader->NameSize < OFFSET_OF(SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE, Name) + SmmVariableHeader->DataSize)) {\r
+        //\r
+        // Prevent InfoSize overflow happen\r
+        //\r
+        Status = EFI_ACCESS_DENIED;\r
+        goto EXIT;\r
+      }\r
       InfoSize = OFFSET_OF(SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE, Name)\r
                  + SmmVariableHeader->DataSize + SmmVariableHeader->NameSize;\r
 \r
@@ -501,6 +568,14 @@ SmmVariableHandler (
         goto EXIT;\r
       }\r
 \r
+      if (SmmVariableHeader->NameSize < sizeof (CHAR16) || SmmVariableHeader->Name[SmmVariableHeader->NameSize/sizeof (CHAR16) - 1] != L'\0') {\r
+        //\r
+        // Make sure VariableName is A Null-terminated string.\r
+        //\r
+        Status = EFI_ACCESS_DENIED;\r
+        goto EXIT;\r
+      }\r
+\r
       Status = VariableServiceSetVariable (\r
                  SmmVariableHeader->Name,\r
                  &SmmVariableHeader->Guid,\r
@@ -555,7 +630,7 @@ SmmVariableHandler (
       //\r
      \r
       if (InternalIsAddressInSmram ((EFI_PHYSICAL_ADDRESS)(UINTN)CommBufferSize, sizeof(UINTN))) {\r
-        DEBUG ((EFI_D_ERROR, "SMM communication buffer size is in SMRAM!\n"));\r
+        DEBUG ((EFI_D_ERROR, "SMM communication buffer in SMRAM!\n"));\r
         Status = EFI_ACCESS_DENIED;\r
         goto EXIT;\r
       }  \r

diff --git a/SecurityPkg/VariableAuthenticated/RuntimeDxe/VariableSmmRuntimeDxe.c b/SecurityPkg/VariableAuthenticated/RuntimeDxe/VariableSmmRuntimeDxe.c
index 5a02b77d537942a99814ca98767665eadc5062db..cdd407d66bea70b3b75dcd2ed992c916b80b8f0f 100644 (file)
--- a/SecurityPkg/VariableAuthenticated/RuntimeDxe/VariableSmmRuntimeDxe.c
+++ b/SecurityPkg/VariableAuthenticated/RuntimeDxe/VariableSmmRuntimeDxe.c
@@ -207,6 +207,7 @@ RuntimeServiceGetVariable (
   SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE  *SmmVariableHeader;\r
   UINTN                                     SmmCommBufPayloadSize;\r
   UINTN                                     TempDataSize;\r
+  UINTN                                     VariableNameSize;\r
 \r
   if (VariableName == NULL || VendorGuid == NULL || DataSize == NULL) {\r
     return EFI_INVALID_PARAMETER;\r
@@ -221,11 +222,12 @@ RuntimeServiceGetVariable (
   //\r
   SmmCommBufPayloadSize = mVariableBufferSize - (SMM_COMMUNICATE_HEADER_SIZE + SMM_VARIABLE_COMMUNICATE_HEADER_SIZE);\r
   TempDataSize          = *DataSize;\r
+  VariableNameSize      = StrSize (VariableName);\r
 \r
   //\r
   // If VariableName exceeds SMM payload limit. Return failure\r
   //\r
-  if (StrSize (VariableName) > SmmCommBufPayloadSize - OFFSET_OF (SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE, Name)) {\r
+  if (VariableNameSize > SmmCommBufPayloadSize - OFFSET_OF (SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE, Name)) {\r
     return EFI_INVALID_PARAMETER;\r
   }\r
 \r
@@ -235,13 +237,13 @@ RuntimeServiceGetVariable (
   // Init the communicate buffer. The buffer data size is:\r
   // SMM_COMMUNICATE_HEADER_SIZE + SMM_VARIABLE_COMMUNICATE_HEADER_SIZE + PayloadSize.\r
   //\r
-  if (TempDataSize > SmmCommBufPayloadSize - OFFSET_OF (SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE, Name) - StrSize (VariableName)) {\r
+  if (TempDataSize > SmmCommBufPayloadSize - OFFSET_OF (SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE, Name) - VariableNameSize) {\r
     //\r
     // If output data buffer exceed SMM payload limit. Trim output buffer to SMM payload size\r
     //\r
-    TempDataSize = SmmCommBufPayloadSize - OFFSET_OF (SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE, Name) - StrSize (VariableName);\r
+    TempDataSize = SmmCommBufPayloadSize - OFFSET_OF (SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE, Name) - VariableNameSize;\r
   }\r
-  PayloadSize = OFFSET_OF (SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE, Name) + StrSize (VariableName) + TempDataSize;\r
+  PayloadSize = OFFSET_OF (SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE, Name) + VariableNameSize + TempDataSize;\r
 \r
   Status = InitCommunicateBuffer ((VOID **)&SmmVariableHeader, PayloadSize, SMM_VARIABLE_FUNCTION_GET_VARIABLE);\r
   if (EFI_ERROR (Status)) {\r
@@ -251,7 +253,7 @@ RuntimeServiceGetVariable (
 \r
   CopyGuid (&SmmVariableHeader->Guid, VendorGuid);\r
   SmmVariableHeader->DataSize   = TempDataSize;\r
-  SmmVariableHeader->NameSize   = StrSize (VariableName);\r
+  SmmVariableHeader->NameSize   = VariableNameSize;\r
   if (Attributes == NULL) {\r
     SmmVariableHeader->Attributes = 0;\r
   } else {\r
@@ -315,6 +317,8 @@ RuntimeServiceGetNextVariableName (
   UINTN                                           PayloadSize;\r
   SMM_VARIABLE_COMMUNICATE_GET_NEXT_VARIABLE_NAME *SmmGetNextVariableName;\r
   UINTN                                           SmmCommBufPayloadSize;\r
+  UINTN                                           OutVariableNameSize;\r
+  UINTN                                           InVariableNameSize;\r
 \r
   if (VariableNameSize == NULL || VariableName == NULL || VendorGuid == NULL) {\r
     return EFI_INVALID_PARAMETER;\r
@@ -324,11 +328,13 @@ RuntimeServiceGetNextVariableName (
   // SMM Communication Buffer max payload size\r
   //\r
   SmmCommBufPayloadSize = mVariableBufferSize - (SMM_COMMUNICATE_HEADER_SIZE + SMM_VARIABLE_COMMUNICATE_HEADER_SIZE);\r
+  OutVariableNameSize   = *VariableNameSize;\r
+  InVariableNameSize    = StrSize (VariableName);\r
 \r
   //\r
   // If input string exceeds SMM payload limit. Return failure\r
   //\r
-  if (StrSize (VariableName) > SmmCommBufPayloadSize - OFFSET_OF (SMM_VARIABLE_COMMUNICATE_GET_NEXT_VARIABLE_NAME, Name)) {\r
+  if (InVariableNameSize > SmmCommBufPayloadSize - OFFSET_OF (SMM_VARIABLE_COMMUNICATE_GET_NEXT_VARIABLE_NAME, Name)) {\r
     return EFI_INVALID_PARAMETER;\r
   }\r
 \r
@@ -338,16 +344,16 @@ RuntimeServiceGetNextVariableName (
   // Init the communicate buffer. The buffer data size is:\r
   // SMM_COMMUNICATE_HEADER_SIZE + SMM_VARIABLE_COMMUNICATE_HEADER_SIZE + PayloadSize.\r
   //\r
-  if (*VariableNameSize > SmmCommBufPayloadSize - OFFSET_OF (SMM_VARIABLE_COMMUNICATE_GET_NEXT_VARIABLE_NAME, Name)) {\r
+  if (OutVariableNameSize > SmmCommBufPayloadSize - OFFSET_OF (SMM_VARIABLE_COMMUNICATE_GET_NEXT_VARIABLE_NAME, Name)) {\r
     //\r
     // If output buffer exceed SMM payload limit. Trim output buffer to SMM payload size\r
     //\r
-    *VariableNameSize = SmmCommBufPayloadSize - OFFSET_OF (SMM_VARIABLE_COMMUNICATE_GET_NEXT_VARIABLE_NAME, Name);\r
+    OutVariableNameSize = SmmCommBufPayloadSize - OFFSET_OF (SMM_VARIABLE_COMMUNICATE_GET_NEXT_VARIABLE_NAME, Name);\r
   }\r
   //\r
   // Payload should be Guid + NameSize + MAX of Input & Output buffer\r
   //\r
-  PayloadSize = OFFSET_OF (SMM_VARIABLE_COMMUNICATE_GET_NEXT_VARIABLE_NAME, Name) + MAX (*VariableNameSize, StrSize (VariableName));\r
+  PayloadSize = OFFSET_OF (SMM_VARIABLE_COMMUNICATE_GET_NEXT_VARIABLE_NAME, Name) + MAX (OutVariableNameSize, InVariableNameSize);\r
 \r
   Status = InitCommunicateBuffer ((VOID **)&SmmGetNextVariableName, PayloadSize, SMM_VARIABLE_FUNCTION_GET_NEXT_VARIABLE_NAME);\r
   if (EFI_ERROR (Status)) {\r
@@ -358,13 +364,16 @@ RuntimeServiceGetNextVariableName (
   //\r
   // SMM comm buffer->NameSize is buffer size for return string\r
   //\r
-  SmmGetNextVariableName->NameSize = *VariableNameSize;\r
+  SmmGetNextVariableName->NameSize = OutVariableNameSize;\r
 \r
   CopyGuid (&SmmGetNextVariableName->Guid, VendorGuid);\r
   //\r
   // Copy whole string\r
   //\r
-  CopyMem (SmmGetNextVariableName->Name, VariableName, StrSize (VariableName));\r
+  CopyMem (SmmGetNextVariableName->Name, VariableName, InVariableNameSize);\r
+  if (OutVariableNameSize > InVariableNameSize) {\r
+    ZeroMem ((UINT8 *) SmmGetNextVariableName->Name + InVariableNameSize, OutVariableNameSize - InVariableNameSize);\r
+  }\r
 \r
   //\r
   // Send data to SMM\r
@@ -374,7 +383,13 @@ RuntimeServiceGetNextVariableName (
   //\r
   // Get data from SMM.\r
   //\r
-  *VariableNameSize = SmmGetNextVariableName->NameSize;    \r
+  if (Status == EFI_SUCCESS || Status == EFI_BUFFER_TOO_SMALL) {\r
+    //\r
+    // SMM CommBuffer NameSize can be a trimed value\r
+    // Only update VariableNameSize when needed\r
+    //\r
+    *VariableNameSize = SmmGetNextVariableName->NameSize;\r
+  }\r
   if (EFI_ERROR (Status)) {\r
     goto Done;\r
   }\r
@@ -420,6 +435,7 @@ RuntimeServiceSetVariable (
   EFI_STATUS                                Status;\r
   UINTN                                     PayloadSize; \r
   SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE  *SmmVariableHeader;\r
+  UINTN                                     VariableNameSize;\r
     \r
   //\r
   // Check input parameters.\r
@@ -441,8 +457,9 @@ RuntimeServiceSetVariable (
     //\r
     return EFI_INVALID_PARAMETER;\r
   }\r
+  VariableNameSize      = StrSize (VariableName);\r
 \r
-  if ((UINTN)(~0) - StrSize (VariableName) < OFFSET_OF (SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE, Name) + DataSize) {\r
+  if ((UINTN)(~0) - VariableNameSize < OFFSET_OF (SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE, Name) + DataSize) {\r
     //\r
     // Prevent PayloadSize overflow\r
     //\r
@@ -455,7 +472,7 @@ RuntimeServiceSetVariable (
   // Init the communicate buffer. The buffer data size is:\r
   // SMM_COMMUNICATE_HEADER_SIZE + SMM_VARIABLE_COMMUNICATE_HEADER_SIZE + PayloadSize.\r
   //\r
-  PayloadSize = OFFSET_OF (SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE, Name) + StrSize (VariableName) + DataSize;\r
+  PayloadSize = OFFSET_OF (SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE, Name) + VariableNameSize + DataSize;\r
   Status = InitCommunicateBuffer ((VOID **)&SmmVariableHeader, PayloadSize, SMM_VARIABLE_FUNCTION_SET_VARIABLE);\r
   if (EFI_ERROR (Status)) {\r
     goto Done;\r
@@ -464,7 +481,7 @@ RuntimeServiceSetVariable (
 \r
   CopyGuid ((EFI_GUID *) &SmmVariableHeader->Guid, VendorGuid);\r
   SmmVariableHeader->DataSize   = DataSize;\r
-  SmmVariableHeader->NameSize   = StrSize (VariableName);\r
+  SmmVariableHeader->NameSize   = VariableNameSize;\r
   SmmVariableHeader->Attributes = Attributes;\r
   CopyMem (SmmVariableHeader->Name, VariableName, SmmVariableHeader->NameSize);\r
   CopyMem ((UINT8 *) SmmVariableHeader->Name + SmmVariableHeader->NameSize, Data, DataSize);\r