Update SMM variable DXE driver GetNextVariable interface to comply with UEFI spec

VariableNameSize is the returned buffer size. GetNextVariable should behavior correct if it is bigger than SMM communication buffer or less than string size of VariableName.

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>
Reviewed-by  : Zeng Star  <star.zeng@intel.com>

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

diff --git a/MdeModulePkg/Include/Guid/SmmVariableCommon.h b/MdeModulePkg/Include/Guid/SmmVariableCommon.h
index c7fde00ca2f318291cb9d31c61a4e3ac8f9e6f73..478fd056b07eeeb26f695182c1de71a4dd613d23 100644 (file)
--- a/MdeModulePkg/Include/Guid/SmmVariableCommon.h
+++ b/MdeModulePkg/Include/Guid/SmmVariableCommon.h
@@ -1,7 +1,7 @@
 /** @file\r
   The file defined some common structures used for communicating between SMM variable module and SMM variable wrapper module.\r
 \r
-Copyright (c) 2011, Intel Corporation. All rights reserved.<BR>\r
+Copyright (c) 2011 - 2013, Intel Corporation. All rights reserved.<BR>\r
 This program and the accompanying materials are licensed and made available under \r
 the terms and conditions of the BSD License that accompanies this distribution.  \r
 The full text of the license may be found at\r
@@ -87,7 +87,7 @@ typedef struct {
 ///\r
 typedef struct {\r
   EFI_GUID    Guid;\r
-  UINTN       NameSize;\r
+  UINTN       NameSize;     // Return name buffer size\r
   CHAR16      Name[1];\r
 } SMM_VARIABLE_COMMUNICATE_GET_NEXT_VARIABLE_NAME;\r
 \r

diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.c b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.c
index 2fca25981aaa1f1c945c330ac9d5fd1a6b51b491..c85d12d951364975e0a6f5d357af3564a662edd5 100644 (file)
--- a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.c
+++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.c
@@ -280,18 +280,21 @@ RuntimeServiceGetNextVariableName (
   EFI_STATUS                                      Status;\r
   UINTN                                           PayloadSize;\r
   SMM_VARIABLE_COMMUNICATE_GET_NEXT_VARIABLE_NAME *SmmGetNextVariableName;\r
+  UINTN                                           SmmCommBufPayloadSize;\r
 \r
   if (VariableNameSize == NULL || VariableName == NULL || VendorGuid == NULL) {\r
     return EFI_INVALID_PARAMETER;\r
   }\r
 \r
-  if (*VariableNameSize >= mVariableBufferSize) {\r
-    //\r
-    // VariableNameSize may be near MAX_ADDRESS incorrectly, this can cause the computed PayLoadSize to\r
-    // overflow to a small value and pass the check in InitCommunicateBuffer().\r
-    // To protect against this vulnerability, return EFI_INVALID_PARAMETER if VariableNameSize is >= mVariableBufferSize.\r
-    // And there will be further check to ensure the total size is also not > mVariableBufferSize.\r
-    //\r
+  //\r
+  // SMM Communication Buffer max payload size\r
+  //\r
+  SmmCommBufPayloadSize = mVariableBufferSize - (SMM_COMMUNICATE_HEADER_SIZE + SMM_VARIABLE_COMMUNICATE_HEADER_SIZE);\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
     return EFI_INVALID_PARAMETER;\r
   }\r
 \r
@@ -301,16 +304,34 @@ RuntimeServiceGetNextVariableName (
   // 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_GET_NEXT_VARIABLE_NAME, Name) + *VariableNameSize; \r
+  if (*VariableNameSize > 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
+  }\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
+\r
+\r
   Status = InitCommunicateBuffer ((VOID **)&SmmGetNextVariableName, PayloadSize, SMM_VARIABLE_FUNCTION_GET_NEXT_VARIABLE_NAME);\r
   if (EFI_ERROR (Status)) {\r
     goto Done;\r
   }\r
   ASSERT (SmmGetNextVariableName != NULL);\r
 \r
+  //\r
+  // SMM comm buffer->NameSize is buffer size for return string\r
+  //\r
   SmmGetNextVariableName->NameSize = *VariableNameSize;\r
+\r
   CopyGuid (&SmmGetNextVariableName->Guid, VendorGuid);\r
-  CopyMem (SmmGetNextVariableName->Name, VariableName, *VariableNameSize);\r
+  //\r
+  // Copy whole string\r
+  //\r
+  CopyMem (SmmGetNextVariableName->Name, VariableName, StrSize (VariableName));\r
 \r
   //\r
   // Send data to SMM\r

diff --git a/SecurityPkg/VariableAuthenticated/RuntimeDxe/VariableSmmRuntimeDxe.c b/SecurityPkg/VariableAuthenticated/RuntimeDxe/VariableSmmRuntimeDxe.c
index 103a12914a1c0a841f363d321ce8f90722e259be..f1111b3b6b3d6fbd4b0b731554f6d200be569a79 100644 (file)
--- a/SecurityPkg/VariableAuthenticated/RuntimeDxe/VariableSmmRuntimeDxe.c
+++ b/SecurityPkg/VariableAuthenticated/RuntimeDxe/VariableSmmRuntimeDxe.c
@@ -296,18 +296,21 @@ RuntimeServiceGetNextVariableName (
   EFI_STATUS                                      Status;\r
   UINTN                                           PayloadSize;\r
   SMM_VARIABLE_COMMUNICATE_GET_NEXT_VARIABLE_NAME *SmmGetNextVariableName;\r
+  UINTN                                           SmmCommBufPayloadSize;\r
 \r
   if (VariableNameSize == NULL || VariableName == NULL || VendorGuid == NULL) {\r
     return EFI_INVALID_PARAMETER;\r
   }\r
 \r
-  if (*VariableNameSize >= mVariableBufferSize) {\r
-    //\r
-    // VariableNameSize may be near MAX_ADDRESS incorrectly, this can cause the computed PayLoadSize to\r
-    // overflow to a small value and pass the check in InitCommunicateBuffer().\r
-    // To protect against this vulnerability, return EFI_INVALID_PARAMETER if VariableNameSize is >= mVariableBufferSize.\r
-    // And there will be further check to ensure the total size is also not > mVariableBufferSize.\r
-    //\r
+  //\r
+  // SMM Communication Buffer max payload size\r
+  //\r
+  SmmCommBufPayloadSize = mVariableBufferSize - (SMM_COMMUNICATE_HEADER_SIZE + SMM_VARIABLE_COMMUNICATE_HEADER_SIZE);\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
     return EFI_INVALID_PARAMETER;\r
   }\r
 \r
@@ -317,16 +320,33 @@ RuntimeServiceGetNextVariableName (
   // 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_GET_NEXT_VARIABLE_NAME, Name) + *VariableNameSize; \r
+  if (*VariableNameSize > 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
+  }\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
+\r
   Status = InitCommunicateBuffer ((VOID **)&SmmGetNextVariableName, PayloadSize, SMM_VARIABLE_FUNCTION_GET_NEXT_VARIABLE_NAME);\r
   if (EFI_ERROR (Status)) {\r
     goto Done;\r
   }\r
   ASSERT (SmmGetNextVariableName != NULL);\r
 \r
+  //\r
+  // SMM comm buffer->NameSize is buffer size for return string\r
+  //\r
   SmmGetNextVariableName->NameSize = *VariableNameSize;\r
+\r
   CopyGuid (&SmmGetNextVariableName->Guid, VendorGuid);\r
-  CopyMem (SmmGetNextVariableName->Name, VariableName, *VariableNameSize);\r
+  //\r
+  // Copy whole string\r
+  //\r
+  CopyMem (SmmGetNextVariableName->Name, VariableName, StrSize (VariableName));\r
 \r
   //\r
   // Send data to SMM\r