diff --git a/MdeModulePkg/Library/SmmCorePerformanceLib/SmmCorePerformanceLib.c b/MdeModulePkg/Library/SmmCorePerformanceLib/SmmCorePerformanceLib.c
index 214d044bcb..314d46d03a 100644
--- a/MdeModulePkg/Library/SmmCorePerformanceLib/SmmCorePerformanceLib.c
+++ b/MdeModulePkg/Library/SmmCorePerformanceLib/SmmCorePerformanceLib.c
@@ -9,6 +9,13 @@
This library is mainly used by SMM Core to start performance logging to ensure that
SMM Performance and PerformanceEx Protocol are installed at the very beginning of SMM phase.
+ Caution: This module requires additional review when modified.
+ This driver will have external input - performance data and communicate buffer in SMM mode.
+ This external input must be validated carefully to avoid security issue like
+ buffer overflow, integer overflow.
+
+ SmmPerformanceHandlerEx(), SmmPerformanceHandler() will receive untrusted input and do basic validation.
+
Copyright (c) 2011 - 2012, Intel Corporation. All rights reserved.
This program and the accompanying materials
are licensed and made available under the terms and conditions of the BSD License
@@ -474,6 +481,9 @@ IsAddressInSmram (
Communication service SMI Handler entry.
This SMI handler provides services for the performance wrapper driver.
+
+ Caution: This function may receive untrusted input.
+ Communicate buffer and buffer size are external input, so this function will do basic validation.
@param[in] DispatchHandle The unique handle assigned to this handler by SmiHandlerRegister().
@param[in] RegisterContext Points to an optional handler context which was specified when the
@@ -506,8 +516,22 @@ SmmPerformanceHandlerEx (
GaugeEntryExArray = NULL;
- ASSERT (CommBuffer != NULL);
+ //
+ // If input is invalid, stop processing this SMI
+ //
+ if (CommBuffer == NULL || CommBufferSize == NULL) {
+ return EFI_SUCCESS;
+ }
+ if(*CommBufferSize < sizeof (SMM_PERF_COMMUNICATE_EX)) {
+ return EFI_SUCCESS;
+ }
+
+ if (IsAddressInSmram ((EFI_PHYSICAL_ADDRESS)(UINTN)CommBuffer, *CommBufferSize)) {
+ DEBUG ((EFI_D_ERROR, "SMM communcation data buffer is in SMRAM!\n"));
+ return EFI_SUCCESS;
+ }
+
SmmPerfCommData = (SMM_PERF_COMMUNICATE_EX *)CommBuffer;
switch (SmmPerfCommData->Function) {
@@ -528,9 +552,9 @@ SmmPerformanceHandlerEx (
//
DataSize = SmmPerfCommData->NumberOfEntries * sizeof(GAUGE_DATA_ENTRY_EX);
if (IsAddressInSmram ((EFI_PHYSICAL_ADDRESS)(UINTN)SmmPerfCommData->GaugeDataEx, DataSize)) {
- DEBUG ((EFI_D_ERROR, "Smm Performance Data buffer is in SMRAM!\n"));
+ DEBUG ((EFI_D_ERROR, "SMM Performance Data buffer is in SMRAM!\n"));
Status = EFI_ACCESS_DENIED;
- break ;
+ break;
}
GaugeEntryExArray = (GAUGE_DATA_ENTRY_EX *) (mGaugeData + 1);
@@ -543,11 +567,12 @@ SmmPerformanceHandlerEx (
break;
default:
- ASSERT (FALSE);
Status = EFI_UNSUPPORTED;
}
+
SmmPerfCommData->ReturnStatus = Status;
+
return EFI_SUCCESS;
}
@@ -556,6 +581,9 @@ SmmPerformanceHandlerEx (
This SMI handler provides services for the performance wrapper driver.
+ Caution: This function may receive untrusted input.
+ Communicate buffer and buffer size are external input, so this function will do basic validation.
+
@param[in] DispatchHandle The unique handle assigned to this handler by SmiHandlerRegister().
@param[in] RegisterContext Points to an optional handler context which was specified when the
handler was registered.
@@ -586,10 +614,24 @@ SmmPerformanceHandler (
UINTN DataSize;
UINTN Index;
UINTN LogEntryKey;
-
+
GaugeEntryExArray = NULL;
- ASSERT (CommBuffer != NULL);
+ //
+ // If input is invalid, stop processing this SMI
+ //
+ if (CommBuffer == NULL || CommBufferSize == NULL) {
+ return EFI_SUCCESS;
+ }
+
+ if(*CommBufferSize < sizeof (SMM_PERF_COMMUNICATE)) {
+ return EFI_SUCCESS;
+ }
+
+ if (IsAddressInSmram ((EFI_PHYSICAL_ADDRESS)(UINTN)CommBuffer, *CommBufferSize)) {
+ DEBUG ((EFI_D_ERROR, "SMM communcation data buffer is in SMRAM!\n"));
+ return EFI_SUCCESS;
+ }
SmmPerfCommData = (SMM_PERF_COMMUNICATE *)CommBuffer;
@@ -611,9 +653,9 @@ SmmPerformanceHandler (
//
DataSize = SmmPerfCommData->NumberOfEntries * sizeof(GAUGE_DATA_ENTRY);
if (IsAddressInSmram ((EFI_PHYSICAL_ADDRESS)(UINTN)SmmPerfCommData->GaugeData, DataSize)) {
- DEBUG ((EFI_D_ERROR, "Smm Performance Data buffer is in SMRAM!\n"));
+ DEBUG ((EFI_D_ERROR, "SMM Performance Data buffer is in SMRAM!\n"));
Status = EFI_ACCESS_DENIED;
- break ;
+ break;
}
GaugeEntryExArray = (GAUGE_DATA_ENTRY_EX *) (mGaugeData + 1);
@@ -630,11 +672,12 @@ SmmPerformanceHandler (
break;
default:
- ASSERT (FALSE);
Status = EFI_UNSUPPORTED;
}
+
SmmPerfCommData->ReturnStatus = Status;
+
return EFI_SUCCESS;
}
diff --git a/MdeModulePkg/Universal/Acpi/FirmwarePerformanceDataTableSmm/FirmwarePerformanceSmm.c b/MdeModulePkg/Universal/Acpi/FirmwarePerformanceDataTableSmm/FirmwarePerformanceSmm.c
index 771c7f95d9..a3f8159b98 100644
--- a/MdeModulePkg/Universal/Acpi/FirmwarePerformanceDataTableSmm/FirmwarePerformanceSmm.c
+++ b/MdeModulePkg/Universal/Acpi/FirmwarePerformanceDataTableSmm/FirmwarePerformanceSmm.c
@@ -4,6 +4,13 @@
This module registers report status code listener to collect performance data
for SMM driver boot records and S3 Suspend Performance Record.
+ Caution: This module requires additional review when modified.
+ This driver will have external input - communicate buffer in SMM mode.
+ This external input must be validated carefully to avoid security issue like
+ buffer overflow, integer overflow.
+
+ FpdtSmiHandler() will receive untrusted input and do basic validation.
+
Copyright (c) 2011 - 2012, Intel Corporation. All rights reserved.
This program and the accompanying materials
are licensed and made available under the terms and conditions of the BSD License
@@ -204,6 +211,9 @@ InternalIsAddressInSmram (
This SMI handler provides services for report SMM boot records.
+ Caution: This function may receive untrusted input.
+ Communicate buffer and buffer size are external input, so this function will do basic validation.
+
@param[in] DispatchHandle The unique handle assigned to this handler by SmiHandlerRegister().
@param[in] RegisterContext Points to an optional handler context which was specified when the
handler was registered.
@@ -211,10 +221,14 @@ InternalIsAddressInSmram (
be conveyed from a non-SMM environment into an SMM environment.
@param[in, out] CommBufferSize The size of the CommBuffer.
- @retval EFI_SUCCESS The interrupt was handled and quiesced. No other handlers should still be called.
- @retval EFI_INVALID_PARAMETER The interrupt parameter is not valid.
- @retval EFI_ACCESS_DENIED The interrupt buffer can't be written.
- @retval EFI_UNSUPPORTED The interrupt is not supported.
+ @retval EFI_SUCCESS The interrupt was handled and quiesced. No other handlers
+ should still be called.
+ @retval EFI_WARN_INTERRUPT_SOURCE_QUIESCED The interrupt has been quiesced but other handlers should
+ still be called.
+ @retval EFI_WARN_INTERRUPT_SOURCE_PENDING The interrupt is still pending and other handlers should still
+ be called.
+ @retval EFI_INTERRUPT_PENDING The interrupt could not be quiesced.
+
**/
EFI_STATUS
EFIAPI
@@ -227,15 +241,27 @@ FpdtSmiHandler (
{
EFI_STATUS Status;
SMM_BOOT_RECORD_COMMUNICATE *SmmCommData;
-
- ASSERT (CommBuffer != NULL);
- if (CommBuffer == NULL || *CommBufferSize < sizeof (SMM_BOOT_RECORD_COMMUNICATE)) {
- return EFI_INVALID_PARAMETER;
+
+ //
+ // If input is invalid, stop processing this SMI
+ //
+ if (CommBuffer == NULL || CommBufferSize == NULL) {
+ return EFI_SUCCESS;
+ }
+
+ if(*CommBufferSize < sizeof (SMM_BOOT_RECORD_COMMUNICATE)) {
+ return EFI_SUCCESS;
+ }
+
+ if (InternalIsAddressInSmram ((EFI_PHYSICAL_ADDRESS)(UINTN)CommBuffer, *CommBufferSize)) {
+ DEBUG ((EFI_D_ERROR, "SMM communication data buffer is in SMRAM!\n"));
+ return EFI_SUCCESS;
}
- Status = EFI_SUCCESS;
SmmCommData = (SMM_BOOT_RECORD_COMMUNICATE*)CommBuffer;
+ Status = EFI_SUCCESS;
+
switch (SmmCommData->Function) {
case SMM_FPDT_FUNCTION_GET_BOOT_RECORD_SIZE :
SmmCommData->BootRecordSize = mBootRecordSize;
@@ -246,13 +272,13 @@ FpdtSmiHandler (
Status = EFI_INVALID_PARAMETER;
break;
}
-
+
//
// Sanity check
//
SmmCommData->BootRecordSize = mBootRecordSize;
if (InternalIsAddressInSmram ((EFI_PHYSICAL_ADDRESS)(UINTN)SmmCommData->BootRecordData, mBootRecordSize)) {
- DEBUG ((EFI_D_ERROR, "Smm Data buffer is in SMRAM!\n"));
+ DEBUG ((EFI_D_ERROR, "SMM Data buffer is in SMRAM!\n"));
Status = EFI_ACCESS_DENIED;
break;
}
@@ -265,11 +291,11 @@ FpdtSmiHandler (
break;
default:
- ASSERT (FALSE);
Status = EFI_UNSUPPORTED;
}
SmmCommData->ReturnStatus = Status;
+
return EFI_SUCCESS;
}