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...@intel.com>
Cc: Star Zeng <star.z...@intel.com>
---
 .../SmmCorePerformanceLib/SmmCorePerformanceLib.c  | 32 ++++++++++++----------
 .../Library/SmmLockBoxLib/SmmLockBoxSmmLib.c       |  3 +-
 2 files changed, 20 insertions(+), 15 deletions(-)

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


------------------------------------------------------------------------------
Don't Limit Your Business. Reach for the Cloud.
GigeNET's Cloud Solutions provide you with the tools and support that
you need to offload your IT needs and focus on growing your business.
Configured For All Businesses. Start Your Cloud Today.
https://www.gigenetcloud.com/
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/edk2-devel

Reply via email to