From 25a4e71aa6a908a81bb5be7fec16da02a5c1f2a5 Mon Sep 17 00:00:00 2001 From: czhang46 Date: Mon, 9 Jul 2012 08:26:35 +0000 Subject: Add SMRAM range check to variable SMM SMI handler. git-svn-id: https://edk2.svn.sourceforge.net/svnroot/edk2/trunk/edk2@13514 6f19259b-4bc3-4df7-8a09-765794883524 --- .../VariableAuthenticated/RuntimeDxe/VariableSmm.c | 129 +++++++++++++++++++-- .../RuntimeDxe/VariableSmm.inf | 1 + 2 files changed, 123 insertions(+), 7 deletions(-) diff --git a/SecurityPkg/VariableAuthenticated/RuntimeDxe/VariableSmm.c b/SecurityPkg/VariableAuthenticated/RuntimeDxe/VariableSmm.c index 37b6f11839..8247836a63 100644 --- a/SecurityPkg/VariableAuthenticated/RuntimeDxe/VariableSmm.c +++ b/SecurityPkg/VariableAuthenticated/RuntimeDxe/VariableSmm.c @@ -28,12 +28,17 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED. #include #include #include +#include + #include #include #include #include "Variable.h" +EFI_SMRAM_DESCRIPTOR *mSmramRanges; +UINTN mSmramRangeCount; + extern VARIABLE_INFO_ENTRY *gVariableInfo; EFI_HANDLE mSmmVariableHandle = NULL; EFI_HANDLE mVariableHandle = NULL; @@ -61,6 +66,34 @@ AtRuntime ( return mAtRuntime; } +/** + 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. @@ -372,7 +405,6 @@ SmmVariableGetStatistics ( @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. - @retval EFI_INVALID_PARAMETER Input parameter is invalid. **/ EFI_STATUS @@ -392,14 +424,39 @@ SmmVariableHandler ( VARIABLE_INFO_ENTRY *VariableInfo; UINTN InfoSize; - if (CommBuffer == NULL) { - return EFI_INVALID_PARAMETER; + // + // 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, @@ -411,6 +468,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, @@ -431,6 +499,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, @@ -452,17 +531,29 @@ 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; } - SmmVariableFunctionHeader->ReturnStatus = Status; +EXIT: + SmmVariableFunctionHeader->ReturnStatus = Status; return EFI_SUCCESS; } @@ -560,7 +651,9 @@ VariableServiceInitialize ( EFI_STATUS Status; EFI_HANDLE VariableHandle; VOID *SmmFtwRegistration; - + EFI_SMM_ACCESS2_PROTOCOL *SmmAccess; + UINTN Size; + // // Variable initialize. // @@ -579,6 +672,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 /// diff --git a/SecurityPkg/VariableAuthenticated/RuntimeDxe/VariableSmm.inf b/SecurityPkg/VariableAuthenticated/RuntimeDxe/VariableSmm.inf index a6343dbbbd..e0aa40ac6d 100644 --- a/SecurityPkg/VariableAuthenticated/RuntimeDxe/VariableSmm.inf +++ b/SecurityPkg/VariableAuthenticated/RuntimeDxe/VariableSmm.inf @@ -72,6 +72,7 @@ gEfiSmmFirmwareVolumeBlockProtocolGuid ## SOMETIMES_CONSUMES gEfiSmmVariableProtocolGuid ## ALWAYS_PRODUCES gEfiSmmFaultTolerantWriteProtocolGuid ## SOMETIMES_CONSUMES + gEfiSmmAccess2ProtocolGuid ## ALWAYS_CONSUMES [Guids] gEfiAuthenticatedVariableGuid ## PRODUCES ## Configuration Table Guid -- cgit v1.2.3