diff options
author | czhang46 <czhang46@6f19259b-4bc3-4df7-8a09-765794883524> | 2012-07-13 05:15:06 +0000 |
---|---|---|
committer | czhang46 <czhang46@6f19259b-4bc3-4df7-8a09-765794883524> | 2012-07-13 05:15:06 +0000 |
commit | 2445a70e62e5dd9678bfd29bed15e22343871803 (patch) | |
tree | 6e25d2ab5ef72a5cd1219a6e1fd6c3d87e5eac58 /MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c | |
parent | 7a0c1361fde4f1f49e36ac8da42f441750e1cb4d (diff) | |
download | edk2-2445a70e62e5dd9678bfd29bed15e22343871803.tar.gz edk2-2445a70e62e5dd9678bfd29bed15e22343871803.tar.bz2 edk2-2445a70e62e5dd9678bfd29bed15e22343871803.zip |
Add SMRAM range check to variable SMM SMI handler.
Signed-off-by: Chao Zhang <chao.b.zhang@intel.com>
Reviewed-by: Fu, Siyuan<siyuan.fu@intel.com>
git-svn-id: https://edk2.svn.sourceforge.net/svnroot/edk2/trunk/edk2@13530 6f19259b-4bc3-4df7-8a09-765794883524
Diffstat (limited to 'MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c')
-rw-r--r-- | MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c | 168 |
1 files changed, 153 insertions, 15 deletions
diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c index 8d949c3965..d4403a2530 100644 --- a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c +++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c @@ -4,11 +4,22 @@ implements an SMI handler to communicate with the DXE runtime driver
to provide variable services.
-Copyright (c) 2010 - 2011, Intel Corporation. All rights reserved.<BR>
-This program and the accompanying materials
-are licensed and made available under the terms and conditions of the BSD License
-which accompanies this distribution. The full text of the license may be found at
-http://opensource.org/licenses/bsd-license.php
+ Caution: This module requires additional review when modified.
+ This driver will have external input - variable data and communicate buffer in SMM mode.
+ This external input must be validated carefully to avoid security issue like
+ buffer overflow, integer overflow.
+
+ SmmVariableHandler() will receive untrusted input and do basic validation.
+
+ Each sub function VariableServiceGetVariable(), VariableServiceGetNextVariableName(),
+ VariableServiceSetVariable(), VariableServiceQueryVariableInfo(), ReclaimForOS(),
+ SmmVariableGetStatistics() should also do validation based on its own knowledge.
+
+Copyright (c) 2010 - 2012, Intel Corporation. All rights reserved.<BR>
+This program and the accompanying materials
+are licensed and made available under the terms and conditions of the BSD License
+which accompanies this distribution. The full text of the license may be found at
+http://opensource.org/licenses/bsd-license.php
THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
@@ -17,12 +28,17 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED. #include <Protocol/SmmVariable.h>
#include <Protocol/SmmFirmwareVolumeBlock.h>
#include <Protocol/SmmFaultTolerantWrite.h>
+#include <Protocol/SmmAccess2.h>
+
#include <Library/SmmServicesTableLib.h>
#include <Guid/VariableFormat.h>
#include <Guid/SmmVariableCommon.h>
#include "Variable.h"
+EFI_SMRAM_DESCRIPTOR *mSmramRanges;
+UINTN mSmramRangeCount;
+
extern VARIABLE_INFO_ENTRY *gVariableInfo;
EFI_HANDLE mSmmVariableHandle = NULL;
EFI_HANDLE mVariableHandle = NULL;
@@ -51,6 +67,34 @@ AtRuntime ( }
/**
+ This function check if the address is in SMRAM.
+
+ @param Buffer the buffer address to be checked.
+ @param Length the buffer length to be checked.
+
+ @retval TRUE this address is in SMRAM.
+ @retval FALSE this address is NOT in SMRAM.
+**/
+BOOLEAN
+InternalIsAddressInSmram (
+ IN EFI_PHYSICAL_ADDRESS Buffer,
+ IN UINT64 Length
+ )
+{
+ UINTN Index;
+
+ for (Index = 0; Index < mSmramRangeCount; Index ++) {
+ if (((Buffer >= mSmramRanges[Index].CpuStart) && (Buffer < mSmramRanges[Index].CpuStart + mSmramRanges[Index].PhysicalSize)) ||
+ ((mSmramRanges[Index].CpuStart >= Buffer) && (mSmramRanges[Index].CpuStart < Buffer + Length))) {
+ return TRUE;
+ }
+ }
+
+ return FALSE;
+}
+
+
+/**
Initializes a basic mutual exclusion lock.
This function initializes a basic mutual exclusion lock to the released state
@@ -241,12 +285,15 @@ GetFvbCountAndBuffer ( /**
Get the variable statistics information from the information buffer pointed by gVariableInfo.
- @param[in, out] InfoEntry A pointer to the buffer of variable information entry.
- On input, point to the variable information returned last time. if
- InfoEntry->VendorGuid is zero, return the first information.
- On output, point to the next variable information.
- @param[in, out] InfoSize On input, the size of the variable information buffer.
- On output, the returned variable information size.
+ Caution: This function may be invoked at SMM runtime.
+ InfoEntry and InfoSize are external input. Care must be taken to make sure not security issue at runtime.
+
+ @param[in, out] InfoEntry A pointer to the buffer of variable information entry.
+ On input, point to the variable information returned last time. if
+ InfoEntry->VendorGuid is zero, return the first information.
+ On output, point to the next variable information.
+ @param[in, out] InfoSize On input, the size of the variable information buffer.
+ On output, the returned variable information size.
@retval EFI_SUCCESS The variable information is found and returned successfully.
@retval EFI_UNSUPPORTED No variable inoformation exists in variable driver. The
@@ -334,6 +381,12 @@ SmmVariableGetStatistics ( This SMI handler provides services for the variable wrapper driver.
+ Caution: This function may receive untrusted input.
+ This variable data and communicate buffer are external input, so this function will do basic validation.
+ Each sub function VariableServiceGetVariable(), VariableServiceGetNextVariableName(),
+ VariableServiceSetVariable(), VariableServiceQueryVariableInfo(), ReclaimForOS(),
+ SmmVariableGetStatistics() should also do validation based on its own knowledge.
+
@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.
@@ -366,12 +419,38 @@ SmmVariableHandler ( VARIABLE_INFO_ENTRY *VariableInfo;
UINTN InfoSize;
- ASSERT (CommBuffer != NULL);
+ //
+ // If input is invalid, stop processing this SMI
+ //
+ if (CommBuffer == NULL || CommBufferSize == NULL) {
+ return EFI_SUCCESS;
+ }
+
+ if (*CommBufferSize < sizeof(SMM_VARIABLE_COMMUNICATE_HEADER) - 1) {
+ return EFI_SUCCESS;
+ }
+
+ if (InternalIsAddressInSmram ((EFI_PHYSICAL_ADDRESS)(UINTN)CommBuffer, *CommBufferSize)) {
+ DEBUG ((EFI_D_ERROR, "SMM communication buffer size is in SMRAM!\n"));
+ return EFI_SUCCESS;
+ }
SmmVariableFunctionHeader = (SMM_VARIABLE_COMMUNICATE_HEADER *)CommBuffer;
switch (SmmVariableFunctionHeader->Function) {
case SMM_VARIABLE_FUNCTION_GET_VARIABLE:
- SmmVariableHeader = (SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE *) SmmVariableFunctionHeader->Data;
+ SmmVariableHeader = (SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE *) SmmVariableFunctionHeader->Data;
+ InfoSize = OFFSET_OF(SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE, Name)
+ + SmmVariableHeader->DataSize + SmmVariableHeader->NameSize;
+
+ //
+ // SMRAM range check already covered before
+ //
+ if (InfoSize > *CommBufferSize - OFFSET_OF (SMM_VARIABLE_COMMUNICATE_HEADER, Data)) {
+ DEBUG ((EFI_D_ERROR, "Data size exceed communication buffer size limit!\n"));
+ Status = EFI_ACCESS_DENIED;
+ goto EXIT;
+ }
+
Status = VariableServiceGetVariable (
SmmVariableHeader->Name,
&SmmVariableHeader->Guid,
@@ -383,6 +462,17 @@ SmmVariableHandler ( case SMM_VARIABLE_FUNCTION_GET_NEXT_VARIABLE_NAME:
GetNextVariableName = (SMM_VARIABLE_COMMUNICATE_GET_NEXT_VARIABLE_NAME *) SmmVariableFunctionHeader->Data;
+ InfoSize = OFFSET_OF(SMM_VARIABLE_COMMUNICATE_GET_NEXT_VARIABLE_NAME, Name) + GetNextVariableName->NameSize;
+
+ //
+ // SMRAM range check already covered before
+ //
+ if (InfoSize > *CommBufferSize - OFFSET_OF (SMM_VARIABLE_COMMUNICATE_HEADER, Data)) {
+ DEBUG ((EFI_D_ERROR, "Data size exceed communication buffer size limit!\n"));
+ Status = EFI_ACCESS_DENIED;
+ goto EXIT;
+ }
+
Status = VariableServiceGetNextVariableName (
&GetNextVariableName->NameSize,
GetNextVariableName->Name,
@@ -403,6 +493,17 @@ SmmVariableHandler ( case SMM_VARIABLE_FUNCTION_QUERY_VARIABLE_INFO:
QueryVariableInfo = (SMM_VARIABLE_COMMUNICATE_QUERY_VARIABLE_INFO *) SmmVariableFunctionHeader->Data;
+ InfoSize = sizeof(SMM_VARIABLE_COMMUNICATE_QUERY_VARIABLE_INFO);
+
+ //
+ // SMRAM range check already covered before
+ //
+ if (InfoSize > *CommBufferSize - OFFSET_OF (SMM_VARIABLE_COMMUNICATE_HEADER, Data)) {
+ DEBUG ((EFI_D_ERROR, "Data size exceed communication buffer size limit!\n"));
+ Status = EFI_ACCESS_DENIED;
+ goto EXIT;
+ }
+
Status = VariableServiceQueryVariableInfo (
QueryVariableInfo->Attributes,
&QueryVariableInfo->MaximumVariableStorageSize,
@@ -424,15 +525,28 @@ SmmVariableHandler ( case SMM_VARIABLE_FUNCTION_GET_STATISTICS:
VariableInfo = (VARIABLE_INFO_ENTRY *) SmmVariableFunctionHeader->Data;
InfoSize = *CommBufferSize - OFFSET_OF (SMM_VARIABLE_COMMUNICATE_HEADER, Data);
+
+ //
+ // Do not need to check SmmVariableFunctionHeader->Data in SMRAM here.
+ // It is covered by previous CommBuffer check
+ //
+
+ if (InternalIsAddressInSmram ((EFI_PHYSICAL_ADDRESS)(UINTN)CommBufferSize, sizeof(UINTN))) {
+ DEBUG ((EFI_D_ERROR, "SMM communication buffer size is in SMRAM!\n"));
+ Status = EFI_ACCESS_DENIED;
+ goto EXIT;
+ }
+
Status = SmmVariableGetStatistics (VariableInfo, &InfoSize);
*CommBufferSize = InfoSize + OFFSET_OF (SMM_VARIABLE_COMMUNICATE_HEADER, Data);
break;
default:
- ASSERT (FALSE);
Status = EFI_UNSUPPORTED;
}
+EXIT:
+
SmmVariableFunctionHeader->ReturnStatus = Status;
return EFI_SUCCESS;
@@ -532,7 +646,9 @@ VariableServiceInitialize ( EFI_STATUS Status;
EFI_HANDLE VariableHandle;
VOID *SmmFtwRegistration;
-
+ EFI_SMM_ACCESS2_PROTOCOL *SmmAccess;
+ UINTN Size;
+
//
// Variable initialize.
//
@@ -551,6 +667,28 @@ VariableServiceInitialize ( );
ASSERT_EFI_ERROR (Status);
+ //
+ // Get SMRAM information
+ //
+ Status = gBS->LocateProtocol (&gEfiSmmAccess2ProtocolGuid, NULL, (VOID **)&SmmAccess);
+ ASSERT_EFI_ERROR (Status);
+
+ Size = 0;
+ Status = SmmAccess->GetCapabilities (SmmAccess, &Size, NULL);
+ ASSERT (Status == EFI_BUFFER_TOO_SMALL);
+
+ Status = gSmst->SmmAllocatePool (
+ EfiRuntimeServicesData,
+ Size,
+ (VOID **)&mSmramRanges
+ );
+ ASSERT_EFI_ERROR (Status);
+
+ Status = SmmAccess->GetCapabilities (SmmAccess, &Size, mSmramRanges);
+ ASSERT_EFI_ERROR (Status);
+
+ mSmramRangeCount = Size / sizeof (EFI_SMRAM_DESCRIPTOR);
+
///
/// Register SMM variable SMI handler
///
|