Revision: 14229
http://edk2.svn.sourceforge.net/edk2/?rev=14229&view=rev
Author: vanjeff
Date: 2013-04-01 03:43:40 +0000 (Mon, 01 Apr 2013)
Log Message:
-----------
Sync patches part of r13512, r13530, r13534, part of r13535 and r13725 main
trunk.
1. Include read data buffer in CommBufferSize when calculate the buffer size.
2. Add SMRAM range check to variable SMM SMI handler.
3. Fix a buffer overflow bug in VariableSmm driver.
4. Return EFI_UNSUPPORTED if READY_TO_BOOT function is invoked at SMM runtime.
5. MdeModulePkg/VariableSmm: Fix a VariableSmm bug when reading variable with
size 0.
Revision Links:
--------------
http://edk2.svn.sourceforge.net/edk2/?rev=13512&view=rev
http://edk2.svn.sourceforge.net/edk2/?rev=13530&view=rev
http://edk2.svn.sourceforge.net/edk2/?rev=13534&view=rev
http://edk2.svn.sourceforge.net/edk2/?rev=13535&view=rev
http://edk2.svn.sourceforge.net/edk2/?rev=13725&view=rev
Modified Paths:
--------------
branches/UDK2010.SR1/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c
branches/UDK2010.SR1/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.inf
branches/UDK2010.SR1/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.c
Modified:
branches/UDK2010.SR1/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c
===================================================================
---
branches/UDK2010.SR1/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c
2013-04-01 02:36:27 UTC (rev 14228)
+++
branches/UDK2010.SR1/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c
2013-04-01 03:43:40 UTC (rev 14229)
@@ -4,12 +4,23 @@
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 @@
#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 @@
}
/**
+ 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,13 +285,16 @@
/**
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
PcdVariableCollectStatistics should be set TRUE
to support it.
@@ -272,7 +319,7 @@
}
StatisticsInfoSize = sizeof (VARIABLE_INFO_ENTRY) + StrSize
(VariableInfo->Name);
- if (*InfoSize < sizeof (VARIABLE_INFO_ENTRY)) {
+ if (*InfoSize < StatisticsInfoSize) {
*InfoSize = StatisticsInfoSize;
return EFI_BUFFER_TOO_SMALL;
}
@@ -334,6 +381,12 @@
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 @@
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 < SMM_VARIABLE_COMMUNICATE_HEADER_SIZE) {
+ 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 @@
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 @@
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,
@@ -412,6 +513,10 @@
break;
case SMM_VARIABLE_FUNCTION_READY_TO_BOOT:
+ if (AtRuntime()) {
+ Status = EFI_UNSUPPORTED;
+ break;
+ }
ReclaimForOS ();
Status = EFI_SUCCESS;
break;
@@ -424,15 +529,28 @@
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 +650,9 @@
EFI_STATUS Status;
EFI_HANDLE VariableHandle;
VOID *SmmFtwRegistration;
-
+ EFI_SMM_ACCESS2_PROTOCOL *SmmAccess;
+ UINTN Size;
+
//
// Variable initialize.
//
@@ -551,6 +671,28 @@
);
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
///
Modified:
branches/UDK2010.SR1/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.inf
===================================================================
---
branches/UDK2010.SR1/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.inf
2013-04-01 02:36:27 UTC (rev 14228)
+++
branches/UDK2010.SR1/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.inf
2013-04-01 03:43:40 UTC (rev 14229)
@@ -8,16 +8,21 @@
# This module should be used with SMM Runtime DXE module together. The
# SMM Runtime DXE module would install variable arch protocol and variable
# write arch protocol based on SMM variable module.
+#
+# 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.
+#
# 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.
#
-# 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.
#
-#
##
[Defines]
@@ -62,6 +67,7 @@
gEfiSmmFirmwareVolumeBlockProtocolGuid ## SOMETIMES_CONSUMES
gEfiSmmVariableProtocolGuid ## ALWAYS_PRODUCES
gEfiSmmFaultTolerantWriteProtocolGuid ## SOMETIMES_CONSUMES
+ gEfiSmmAccess2ProtocolGuid ## ALWAYS_CONSUMES
[Guids]
gEfiVariableGuid ## PRODUCES ## Configuration
Table Guid
Modified:
branches/UDK2010.SR1/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.c
===================================================================
---
branches/UDK2010.SR1/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.c
2013-04-01 02:36:27 UTC (rev 14228)
+++
branches/UDK2010.SR1/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.c
2013-04-01 03:43:40 UTC (rev 14229)
@@ -156,9 +156,9 @@
//
// Init the communicate buffer. The buffer data size is:
- // SMM_COMMUNICATE_HEADER_SIZE + SMM_VARIABLE_COMMUNICATE_HEADER_SIZE +
PayloadSize.
+ // SMM_COMMUNICATE_HEADER_SIZE + SMM_VARIABLE_COMMUNICATE_HEADER_SIZE +
PayloadSize + DataSize.
//
- PayloadSize = OFFSET_OF (SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE, Name) +
StrSize (VariableName);
+ PayloadSize = OFFSET_OF (SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE, Name) +
StrSize (VariableName) + *DataSize;
Status = InitCommunicateBuffer ((VOID **)&SmmVariableHeader, PayloadSize,
SMM_VARIABLE_FUNCTION_GET_VARIABLE);
if (EFI_ERROR (Status)) {
return Status;
This was sent by the SourceForge.net collaborative development platform, the
world's largest Open Source development site.
------------------------------------------------------------------------------
Own the Future-Intel® Level Up Game Demo Contest 2013
Rise to greatness in Intel's independent game demo contest.
Compete for recognition, cash, and the chance to get your game
on Steam. $5K grand prize plus 10 genre and skill prizes.
Submit your demo by 6/6/13. http://p.sf.net/sfu/intel_levelupd2d
_______________________________________________
edk2-commits mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/edk2-commits
