Fix the TOCTOU issue of CommBufferSize itself for SMM communicate handler input.

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

diff --git a/MdeModulePkg/Library/SmmCorePerformanceLib/SmmCorePerformanceLib.c b/MdeModulePkg/Library/SmmCorePerformanceLib/SmmCorePerformanceLib.c
index 2bfd62a2b9e58d4bbe4217d62f128e7f3fce92c9..f95079bd272201f8100bd38f0f1f58918ef36809 100644 (file)
--- a/MdeModulePkg/Library/SmmCorePerformanceLib/SmmCorePerformanceLib.c
+++ b/MdeModulePkg/Library/SmmCorePerformanceLib/SmmCorePerformanceLib.c
@@ -543,6 +543,7 @@ SmmPerformanceHandlerEx (
   GAUGE_DATA_ENTRY_EX       *GaugeDataEx;\r
   UINTN                     NumberOfEntries;\r
   UINTN                     LogEntryKey;\r
+  UINTN                     TempCommBufferSize;\r
 \r
   GaugeEntryExArray = NULL;\r
 \r
@@ -553,11 +554,13 @@ SmmPerformanceHandlerEx (
     return EFI_SUCCESS;\r
   }\r
 \r
-  if(*CommBufferSize < sizeof (SMM_PERF_COMMUNICATE_EX)) {\r
+  TempCommBufferSize = *CommBufferSize;\r
+\r
+  if(TempCommBufferSize < sizeof (SMM_PERF_COMMUNICATE_EX)) {\r
     return EFI_SUCCESS;\r
   }\r
 \r
-  if (!IsAddressValid ((UINTN)CommBuffer, *CommBufferSize)) {\r
+  if (!IsAddressValid ((UINTN)CommBuffer, TempCommBufferSize)) {\r
     DEBUG ((EFI_D_ERROR, "SmmPerformanceHandlerEx: SMM communcation data buffer in SMRAM or overflow!\n"));\r
     return EFI_SUCCESS;\r
   }\r
@@ -649,7 +652,8 @@ SmmPerformanceHandler (
   GAUGE_DATA_ENTRY      *GaugeData;\r
   UINTN                 NumberOfEntries;\r
   UINTN                 LogEntryKey;\r
-  \r
+  UINTN                 TempCommBufferSize;\r
+\r
   GaugeEntryExArray = NULL;\r
 \r
   //\r
@@ -659,11 +663,13 @@ SmmPerformanceHandler (
     return EFI_SUCCESS;\r
   }\r
 \r
-  if(*CommBufferSize < sizeof (SMM_PERF_COMMUNICATE)) {\r
+  TempCommBufferSize = *CommBufferSize;\r
+\r
+  if(TempCommBufferSize < sizeof (SMM_PERF_COMMUNICATE)) {\r
     return EFI_SUCCESS;\r
   }\r
 \r
-  if (!IsAddressValid ((UINTN)CommBuffer, *CommBufferSize)) {\r
+  if (!IsAddressValid ((UINTN)CommBuffer, TempCommBufferSize)) {\r
     DEBUG ((EFI_D_ERROR, "SmmPerformanceHandler: SMM communcation data buffer in SMRAM or overflow!\n"));\r
     return EFI_SUCCESS;\r
   }\r

diff --git a/MdeModulePkg/Universal/Acpi/FirmwarePerformanceDataTableSmm/FirmwarePerformanceSmm.c b/MdeModulePkg/Universal/Acpi/FirmwarePerformanceDataTableSmm/FirmwarePerformanceSmm.c
index f3472e26f3db70e3afadb5c7cb15395d3dff7011..9c5fd4db85a61ae2de20c3a4ac10732f20d38ef7 100644 (file)
--- a/MdeModulePkg/Universal/Acpi/FirmwarePerformanceDataTableSmm/FirmwarePerformanceSmm.c
+++ b/MdeModulePkg/Universal/Acpi/FirmwarePerformanceDataTableSmm/FirmwarePerformanceSmm.c
@@ -268,6 +268,7 @@ FpdtSmiHandler (
   SMM_BOOT_RECORD_COMMUNICATE  *SmmCommData;\r
   UINTN                        BootRecordSize;\r
   VOID                         *BootRecordData;\r
+  UINTN                        TempCommBufferSize;\r
 \r
   //\r
   // If input is invalid, stop processing this SMI\r
@@ -276,11 +277,13 @@ FpdtSmiHandler (
     return EFI_SUCCESS;\r
   }\r
 \r
-  if(*CommBufferSize < sizeof (SMM_BOOT_RECORD_COMMUNICATE)) {\r
+  TempCommBufferSize = *CommBufferSize;\r
+\r
+  if(TempCommBufferSize < sizeof (SMM_BOOT_RECORD_COMMUNICATE)) {\r
     return EFI_SUCCESS;\r
   }\r
   \r
-  if (!InternalIsAddressValid ((UINTN)CommBuffer, *CommBufferSize)) {\r
+  if (!InternalIsAddressValid ((UINTN)CommBuffer, TempCommBufferSize)) {\r
     DEBUG ((EFI_D_ERROR, "FpdtSmiHandler: SMM communication data buffer in SMRAM or overflow!\n"));\r
     return EFI_SUCCESS;\r
   }\r

diff --git a/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteSmm.c b/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteSmm.c
index 2580d478a38c1fb493699e83a0e10a5acaffa67b..2b3a63081dce9af81762253591d4801873c91c81 100644 (file)
--- a/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteSmm.c
+++ b/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteSmm.c
@@ -372,7 +372,7 @@ SmmFaultTolerantWriteHandler (
   UINTN                                            CommBufferPayloadSize;\r
   UINTN                                            PrivateDataSize;\r
   UINTN                                            Length;\r
-\r
+  UINTN                                            TempCommBufferSize;\r
 \r
   //\r
   // If input is invalid, stop processing this SMI\r
@@ -381,13 +381,15 @@ SmmFaultTolerantWriteHandler (
     return EFI_SUCCESS;\r
   }\r
 \r
-  if (*CommBufferSize < SMM_FTW_COMMUNICATE_HEADER_SIZE) {\r
+  TempCommBufferSize = *CommBufferSize;\r
+\r
+  if (TempCommBufferSize < SMM_FTW_COMMUNICATE_HEADER_SIZE) {\r
     DEBUG ((EFI_D_ERROR, "SmmFtwHandler: SMM communication buffer size invalid!\n"));\r
     return EFI_SUCCESS;\r
   }\r
-  CommBufferPayloadSize = *CommBufferSize - SMM_FTW_COMMUNICATE_HEADER_SIZE;\r
+  CommBufferPayloadSize = TempCommBufferSize - SMM_FTW_COMMUNICATE_HEADER_SIZE;\r
 \r
-  if (!InternalIsAddressValid ((UINTN)CommBuffer, *CommBufferSize)) {\r
+  if (!InternalIsAddressValid ((UINTN)CommBuffer, TempCommBufferSize)) {\r
     DEBUG ((EFI_D_ERROR, "SmmFtwHandler: SMM communication buffer in SMRAM or overflow!\n"));\r
     return EFI_SUCCESS;\r
   }\r

diff --git a/MdeModulePkg/Universal/LockBox/SmmLockBox/SmmLockBox.c b/MdeModulePkg/Universal/LockBox/SmmLockBox/SmmLockBox.c
index 4cb88106ee3b0cfec43e5f3bed9197662c645350..ad4b2645cb257eb9461df2a9393f88f0682a3389 100644 (file)
--- a/MdeModulePkg/Universal/LockBox/SmmLockBox/SmmLockBox.c
+++ b/MdeModulePkg/Universal/LockBox/SmmLockBox/SmmLockBox.c
@@ -321,17 +321,27 @@ SmmLockBoxHandler (
   )\r
 {\r
   EFI_SMM_LOCK_BOX_PARAMETER_HEADER *LockBoxParameterHeader;\r
+  UINTN                             TempCommBufferSize;\r
 \r
   DEBUG ((EFI_D_ERROR, "SmmLockBox SmmLockBoxHandler Enter\n"));\r
 \r
+  //\r
+  // If input is invalid, stop processing this SMI\r
+  //\r
+  if (CommBuffer == NULL || CommBufferSize == NULL) {\r
+    return EFI_SUCCESS;\r
+  }\r
+\r
+  TempCommBufferSize = *CommBufferSize;\r
+\r
   //\r
   // Sanity check\r
   //\r
-  if (*CommBufferSize < sizeof(EFI_SMM_LOCK_BOX_PARAMETER_HEADER)) {\r
+  if (TempCommBufferSize < sizeof(EFI_SMM_LOCK_BOX_PARAMETER_HEADER)) {\r
     DEBUG ((EFI_D_ERROR, "SmmLockBox Command Buffer Size invalid!\n"));\r
     return EFI_SUCCESS;\r
   }\r
-  if (!IsAddressValid ((UINTN)CommBuffer, *CommBufferSize)) {\r
+  if (!IsAddressValid ((UINTN)CommBuffer, TempCommBufferSize)) {\r
     DEBUG ((EFI_D_ERROR, "SmmLockBox Command Buffer in SMRAM or overflow!\n"));\r
     return EFI_SUCCESS;\r
   }\r
@@ -346,35 +356,35 @@ SmmLockBoxHandler (
 \r
   switch (LockBoxParameterHeader->Command) {\r
   case EFI_SMM_LOCK_BOX_COMMAND_SAVE:\r
-    if (*CommBufferSize < sizeof(EFI_SMM_LOCK_BOX_PARAMETER_SAVE)) {\r
+    if (TempCommBufferSize < sizeof(EFI_SMM_LOCK_BOX_PARAMETER_SAVE)) {\r
       DEBUG ((EFI_D_ERROR, "SmmLockBox Command Buffer Size for SAVE invalid!\n"));\r
       break;\r
     }\r
     SmmLockBoxSave ((EFI_SMM_LOCK_BOX_PARAMETER_SAVE *)(UINTN)LockBoxParameterHeader);\r
     break;\r
   case EFI_SMM_LOCK_BOX_COMMAND_UPDATE:\r
-    if (*CommBufferSize < sizeof(EFI_SMM_LOCK_BOX_PARAMETER_UPDATE)) {\r
+    if (TempCommBufferSize < sizeof(EFI_SMM_LOCK_BOX_PARAMETER_UPDATE)) {\r
       DEBUG ((EFI_D_ERROR, "SmmLockBox Command Buffer Size for UPDATE invalid!\n"));\r
       break;\r
     }\r
     SmmLockBoxUpdate ((EFI_SMM_LOCK_BOX_PARAMETER_UPDATE *)(UINTN)LockBoxParameterHeader);\r
     break;\r
   case EFI_SMM_LOCK_BOX_COMMAND_RESTORE:\r
-    if (*CommBufferSize < sizeof(EFI_SMM_LOCK_BOX_PARAMETER_RESTORE)) {\r
+    if (TempCommBufferSize < sizeof(EFI_SMM_LOCK_BOX_PARAMETER_RESTORE)) {\r
       DEBUG ((EFI_D_ERROR, "SmmLockBox Command Buffer Size for RESTORE invalid!\n"));\r
       break;\r
     }\r
     SmmLockBoxRestore ((EFI_SMM_LOCK_BOX_PARAMETER_RESTORE *)(UINTN)LockBoxParameterHeader);\r
     break;\r
   case EFI_SMM_LOCK_BOX_COMMAND_SET_ATTRIBUTES:\r
-    if (*CommBufferSize < sizeof(EFI_SMM_LOCK_BOX_PARAMETER_SET_ATTRIBUTES)) {\r
+    if (TempCommBufferSize < sizeof(EFI_SMM_LOCK_BOX_PARAMETER_SET_ATTRIBUTES)) {\r
       DEBUG ((EFI_D_ERROR, "SmmLockBox Command Buffer Size for SET_ATTRIBUTES invalid!\n"));\r
       break;\r
     }\r
     SmmLockBoxSetAttributes ((EFI_SMM_LOCK_BOX_PARAMETER_SET_ATTRIBUTES *)(UINTN)LockBoxParameterHeader);\r
     break;\r
   case EFI_SMM_LOCK_BOX_COMMAND_RESTORE_ALL_IN_PLACE:\r
-    if (*CommBufferSize < sizeof(EFI_SMM_LOCK_BOX_PARAMETER_RESTORE_ALL_IN_PLACE)) {\r
+    if (TempCommBufferSize < sizeof(EFI_SMM_LOCK_BOX_PARAMETER_RESTORE_ALL_IN_PLACE)) {\r
       DEBUG ((EFI_D_ERROR, "SmmLockBox Command Buffer Size for RESTORE_ALL_IN_PLACE invalid!\n"));\r
       break;\r
     }\r

diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c
index aea9d4bcfe5573fb70b83a9c00ec89efc566e882..1ffa74e6cc57c5048ec7d937136c5d77532a9441 100644 (file)
--- a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c
+++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c
@@ -501,6 +501,7 @@ SmmVariableHandler (
   UINTN                                            InfoSize;\r
   UINTN                                            NameBufferSize;\r
   UINTN                                            CommBufferPayloadSize;\r
+  UINTN                                            TempCommBufferSize;\r
 \r
   //\r
   // If input is invalid, stop processing this SMI\r
@@ -509,17 +510,19 @@ SmmVariableHandler (
     return EFI_SUCCESS;\r
   }\r
 \r
-  if (*CommBufferSize < SMM_VARIABLE_COMMUNICATE_HEADER_SIZE) {\r
+  TempCommBufferSize = *CommBufferSize;\r
+\r
+  if (TempCommBufferSize < SMM_VARIABLE_COMMUNICATE_HEADER_SIZE) {\r
     DEBUG ((EFI_D_ERROR, "SmmVariableHandler: SMM communication buffer size invalid!\n"));\r
     return EFI_SUCCESS;\r
   }\r
-  CommBufferPayloadSize = *CommBufferSize - SMM_VARIABLE_COMMUNICATE_HEADER_SIZE;\r
+  CommBufferPayloadSize = TempCommBufferSize - SMM_VARIABLE_COMMUNICATE_HEADER_SIZE;\r
   if (CommBufferPayloadSize > mVariableBufferPayloadSize) {\r
     DEBUG ((EFI_D_ERROR, "SmmVariableHandler: SMM communication buffer payload size invalid!\n"));\r
     return EFI_SUCCESS;\r
   }\r
 \r
-  if (!InternalIsAddressValid ((UINTN)CommBuffer, *CommBufferSize)) {\r
+  if (!InternalIsAddressValid ((UINTN)CommBuffer, TempCommBufferSize)) {\r
     DEBUG ((EFI_D_ERROR, "SmmVariableHandler: SMM communication buffer in SMRAM or overflow!\n"));\r
     return EFI_SUCCESS;\r
   }\r
@@ -699,7 +702,7 @@ SmmVariableHandler (
 \r
     case SMM_VARIABLE_FUNCTION_GET_STATISTICS:\r
       VariableInfo = (VARIABLE_INFO_ENTRY *) SmmVariableFunctionHeader->Data;\r
-      InfoSize = *CommBufferSize - SMM_VARIABLE_COMMUNICATE_HEADER_SIZE;\r
+      InfoSize = TempCommBufferSize - SMM_VARIABLE_COMMUNICATE_HEADER_SIZE;\r
 \r
       //\r
       // Do not need to check SmmVariableFunctionHeader->Data in SMRAM here. \r

diff --git a/SecurityPkg/VariableAuthenticated/RuntimeDxe/VariableSmm.c b/SecurityPkg/VariableAuthenticated/RuntimeDxe/VariableSmm.c
index cf866cecba4da9bb301794347f0fcbe91272aecb..0be4f254d788e848b7838d6b97c47f29120e741e 100644 (file)
--- a/SecurityPkg/VariableAuthenticated/RuntimeDxe/VariableSmm.c
+++ b/SecurityPkg/VariableAuthenticated/RuntimeDxe/VariableSmm.c
@@ -506,6 +506,7 @@ SmmVariableHandler (
   UINTN                                            InfoSize;\r
   UINTN                                            NameBufferSize;\r
   UINTN                                            CommBufferPayloadSize;\r
+  UINTN                                            TempCommBufferSize;\r
 \r
   //\r
   // If input is invalid, stop processing this SMI\r
@@ -514,17 +515,19 @@ SmmVariableHandler (
     return EFI_SUCCESS;\r
   }\r
 \r
-  if (*CommBufferSize < SMM_VARIABLE_COMMUNICATE_HEADER_SIZE) {\r
+  TempCommBufferSize = *CommBufferSize;\r
+\r
+  if (TempCommBufferSize < SMM_VARIABLE_COMMUNICATE_HEADER_SIZE) {\r
     DEBUG ((EFI_D_ERROR, "SmmVariableHandler: SMM communication buffer size invalid!\n"));\r
     return EFI_SUCCESS;\r
   }\r
-  CommBufferPayloadSize = *CommBufferSize - SMM_VARIABLE_COMMUNICATE_HEADER_SIZE;\r
+  CommBufferPayloadSize = TempCommBufferSize - SMM_VARIABLE_COMMUNICATE_HEADER_SIZE;\r
   if (CommBufferPayloadSize > mVariableBufferPayloadSize) {\r
     DEBUG ((EFI_D_ERROR, "SmmVariableHandler: SMM communication buffer payload size invalid!\n"));\r
     return EFI_SUCCESS;\r
   }\r
 \r
-  if (!InternalIsAddressValid ((UINTN)CommBuffer, *CommBufferSize)) {\r
+  if (!InternalIsAddressValid ((UINTN)CommBuffer, TempCommBufferSize)) {\r
     DEBUG ((EFI_D_ERROR, "SmmVariableHandler: SMM communication buffer in SMRAM or overflow!\n"));\r
     return EFI_SUCCESS;\r
   }\r
@@ -705,7 +708,7 @@ SmmVariableHandler (
 \r
     case SMM_VARIABLE_FUNCTION_GET_STATISTICS:\r
       VariableInfo = (VARIABLE_INFO_ENTRY *) SmmVariableFunctionHeader->Data;\r
-      InfoSize = *CommBufferSize - SMM_VARIABLE_COMMUNICATE_HEADER_SIZE;\r
+      InfoSize = TempCommBufferSize - SMM_VARIABLE_COMMUNICATE_HEADER_SIZE;\r
 \r
       //\r
       // Do not need to check SmmVariableFunctionHeader->Data in SMRAM here. \r