MdeModulePkg: Fix potential integer overflow issue

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

diff --git a/MdeModulePkg/Library/SmmCorePerformanceLib/SmmCorePerformanceLib.c b/MdeModulePkg/Library/SmmCorePerformanceLib/SmmCorePerformanceLib.c
index f28b657c94ddba3fb012296f8721b44b264f8c76..e59cc28d537fa8b1db93443a3ec06b6371dc20d6 100644 (file)
--- a/MdeModulePkg/Library/SmmCorePerformanceLib/SmmCorePerformanceLib.c
+++ b/MdeModulePkg/Library/SmmCorePerformanceLib/SmmCorePerformanceLib.c
@@ -482,7 +482,8 @@ SmmPerformanceHandlerEx (
   EFI_STATUS                Status;\r
   SMM_PERF_COMMUNICATE_EX   *SmmPerfCommData;\r
   GAUGE_DATA_ENTRY_EX       *GaugeEntryExArray;\r
   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
   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 = 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
          Status = EFI_INVALID_PARAMETER;\r
          break;\r
        }\r


@@ -529,19 +530,22 @@ SmmPerformanceHandlerEx (
        //\r
        // Sanity check\r
        //\r
        //\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
          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
        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
   EFI_STATUS            Status;\r
   SMM_PERF_COMMUNICATE  *SmmPerfCommData;\r
   GAUGE_DATA_ENTRY_EX   *GaugeEntryExArray;\r

-  UINTN                 DataSize;\r
+  UINT64                DataSize;\r

   UINTN                 Index;\r
   GAUGE_DATA_ENTRY      *GaugeData;\r
   UINTN                 NumberOfEntries;\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 = 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
          Status = EFI_INVALID_PARAMETER;\r
          break;\r
        }\r


@@ -638,8 +642,8 @@ SmmPerformanceHandler (
        //\r
        // Sanity check\r
        //\r
        //\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
          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
        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
            (UINT8 *) &GaugeData[Index],\r
            (UINT8 *) &GaugeEntryExArray[LogEntryKey++],\r
            sizeof (GAUGE_DATA_ENTRY)\r

diff --git a/MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxSmmLib.c b/MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxSmmLib.c
index a789daf9a8d4cf603bda7ca0ba5f4039fdafb0a2..23a786d56ec5e14a2aec72debb085d7b26376fdf 100644 (file)
--- a/MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxSmmLib.c
+++ b/MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxSmmLib.c
@@ -1,6 +1,6 @@
 /** @file\r
 \r
 /** @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
 \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
     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
   CopyMem ((VOID *)((UINTN)LockBox->SmramBuffer + Offset), Buffer, Length);\r
 \r
   //\r