MdeModulePkg: Fix potential integer overflow issue
authorRuiyu Ni <ruiyu.ni@intel.com>
Fri, 10 Jul 2015 02:16:42 +0000 (02:16 +0000)
committerniruiyu <niruiyu@Edk2>
Fri, 10 Jul 2015 02:16:42 +0000 (02:16 +0000)
In certain rare circumstance, the data passed from outside of SMM may be
invalid resulting the integer overflow. The issue are found by code review.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ruiyu Ni <ruiyu.ni@intel.com>
Reviewed-by: Star Zeng <star.zeng@intel.com>
git-svn-id: https://svn.code.sf.net/p/edk2/code/trunk/edk2@17908 6f19259b-4bc3-4df7-8a09-765794883524

MdeModulePkg/Library/SmmCorePerformanceLib/SmmCorePerformanceLib.c
MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxSmmLib.c

index f28b657..e59cc28 100644 (file)
@@ -482,7 +482,8 @@ SmmPerformanceHandlerEx (
   EFI_STATUS                Status;\r
   SMM_PERF_COMMUNICATE_EX   *SmmPerfCommData;\r
   GAUGE_DATA_ENTRY_EX       *GaugeEntryExArray;\r
-  UINTN                     DataSize;\r
+  UINT64                    DataSize;\r
+  UINTN                     Index;\r
   GAUGE_DATA_ENTRY_EX       *GaugeDataEx;\r
   UINTN                     NumberOfEntries;\r
   UINTN                     LogEntryKey;\r
@@ -521,7 +522,7 @@ SmmPerformanceHandlerEx (
       NumberOfEntries = SmmPerfCommData->NumberOfEntries;\r
       LogEntryKey = SmmPerfCommData->LogEntryKey;\r
        if (GaugeDataEx == NULL || NumberOfEntries == 0 || LogEntryKey > mGaugeData->NumberOfEntries ||\r
-           NumberOfEntries > mGaugeData->NumberOfEntries || (LogEntryKey + NumberOfEntries) > mGaugeData->NumberOfEntries) {\r
+           NumberOfEntries > mGaugeData->NumberOfEntries || LogEntryKey > (mGaugeData->NumberOfEntries - NumberOfEntries)) {\r
          Status = EFI_INVALID_PARAMETER;\r
          break;\r
        }\r
@@ -529,19 +530,22 @@ SmmPerformanceHandlerEx (
        //\r
        // Sanity check\r
        //\r
-       DataSize = NumberOfEntries * sizeof(GAUGE_DATA_ENTRY_EX);\r
-       if (!SmmIsBufferOutsideSmmValid ((UINTN)GaugeDataEx, DataSize)) {\r
+       DataSize = MultU64x32 (NumberOfEntries, sizeof(GAUGE_DATA_ENTRY_EX));\r
+       if (!SmmIsBufferOutsideSmmValid ((UINTN) GaugeDataEx, DataSize)) {\r
          DEBUG ((EFI_D_ERROR, "SmmPerformanceHandlerEx: SMM Performance Data buffer in SMRAM or overflow!\n"));\r
          Status = EFI_ACCESS_DENIED;\r
          break;\r
        }\r
 \r
        GaugeEntryExArray = (GAUGE_DATA_ENTRY_EX *) (mGaugeData + 1);\r
-       CopyMem(\r
-         (UINT8 *) GaugeDataEx,\r
-         (UINT8 *) &GaugeEntryExArray[LogEntryKey],\r
-         DataSize\r
-         );\r
+\r
+       for (Index = 0; Index < NumberOfEntries; Index++) {\r
+         CopyMem (\r
+           (UINT8 *) &GaugeDataEx[Index],\r
+           (UINT8 *) &GaugeEntryExArray[LogEntryKey++],\r
+           sizeof (GAUGE_DATA_ENTRY_EX)\r
+           );\r
+       }\r
        Status = EFI_SUCCESS;\r
        break;\r
 \r
@@ -590,7 +594,7 @@ SmmPerformanceHandler (
   EFI_STATUS            Status;\r
   SMM_PERF_COMMUNICATE  *SmmPerfCommData;\r
   GAUGE_DATA_ENTRY_EX   *GaugeEntryExArray;\r
-  UINT                DataSize;\r
+  UINT64                DataSize;\r
   UINTN                 Index;\r
   GAUGE_DATA_ENTRY      *GaugeData;\r
   UINTN                 NumberOfEntries;\r
@@ -630,7 +634,7 @@ SmmPerformanceHandler (
        NumberOfEntries = SmmPerfCommData->NumberOfEntries;\r
        LogEntryKey = SmmPerfCommData->LogEntryKey;\r
        if (GaugeData == NULL || NumberOfEntries == 0 || LogEntryKey > mGaugeData->NumberOfEntries ||\r
-           NumberOfEntries > mGaugeData->NumberOfEntries || (LogEntryKey + NumberOfEntries) > mGaugeData->NumberOfEntries) {\r
+           NumberOfEntries > mGaugeData->NumberOfEntries || LogEntryKey > (mGaugeData->NumberOfEntries - NumberOfEntries)) {\r
          Status = EFI_INVALID_PARAMETER;\r
          break;\r
        }\r
@@ -638,8 +642,8 @@ SmmPerformanceHandler (
        //\r
        // Sanity check\r
        //\r
-       DataSize = NumberOfEntries * sizeof(GAUGE_DATA_ENTRY);\r
-       if (!SmmIsBufferOutsideSmmValid ((UINTN)GaugeData, DataSize)) {\r
+       DataSize = MultU64x32 (NumberOfEntries, sizeof(GAUGE_DATA_ENTRY));\r
+       if (!SmmIsBufferOutsideSmmValid ((UINTN) GaugeData, DataSize)) {\r
          DEBUG ((EFI_D_ERROR, "SmmPerformanceHandler: SMM Performance Data buffer in SMRAM or overflow!\n"));\r
          Status = EFI_ACCESS_DENIED;\r
          break;\r
@@ -648,7 +652,7 @@ SmmPerformanceHandler (
        GaugeEntryExArray = (GAUGE_DATA_ENTRY_EX *) (mGaugeData + 1);\r
 \r
        for (Index = 0; Index < NumberOfEntries; Index++) {\r
-         CopyMem(\r
+         CopyMem (\r
            (UINT8 *) &GaugeData[Index],\r
            (UINT8 *) &GaugeEntryExArray[LogEntryKey++],\r
            sizeof (GAUGE_DATA_ENTRY)\r
index a789daf..23a786d 100644 (file)
@@ -1,6 +1,6 @@
 /** @file\r
 \r
-Copyright (c) 2010 - 2014, Intel Corporation. All rights reserved.<BR>\r
+Copyright (c) 2010 - 2015, 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
@@ -394,6 +394,7 @@ UpdateLockBox (
     DEBUG ((EFI_D_INFO, "SmmLockBoxSmmLib UpdateLockBox - Exit (%r)\n", EFI_BUFFER_TOO_SMALL));\r
     return EFI_BUFFER_TOO_SMALL;\r
   }\r
+  ASSERT ((UINTN)LockBox->SmramBuffer <= (MAX_ADDRESS - Offset));\r
   CopyMem ((VOID *)((UINTN)LockBox->SmramBuffer + Offset), Buffer, Length);\r
 \r
   //\r