Prevents potential math over and underflows when comparing buffers for SMM validity.
Original patch uploaded to bugzilla by Bret Barkelew <b...@corthon.com>. I've adapted the patch to latest master, uncristified, dropped MU comments. Ref: CVE-2021-38578 Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=3387 Signed-off-by: Gerd Hoffmann <kra...@redhat.com> --- MdeModulePkg/Core/PiSmmCore/PiSmmCore.inf | 1 + MdeModulePkg/Core/PiSmmCore/PiSmmIpl.inf | 1 + MdeModulePkg/Core/PiSmmCore/PiSmmCore.h | 1 + MdeModulePkg/Core/PiSmmCore/PiSmmCore.c | 32 ++++++++++++++++------- MdeModulePkg/Core/PiSmmCore/PiSmmIpl.c | 10 +++++-- 5 files changed, 34 insertions(+), 11 deletions(-) diff --git a/MdeModulePkg/Core/PiSmmCore/PiSmmCore.inf b/MdeModulePkg/Core/PiSmmCore/PiSmmCore.inf index c8bfae3860fc..3df44b38f13c 100644 --- a/MdeModulePkg/Core/PiSmmCore/PiSmmCore.inf +++ b/MdeModulePkg/Core/PiSmmCore/PiSmmCore.inf @@ -60,6 +60,7 @@ [LibraryClasses] PerformanceLib HobLib SmmMemLib + SafeIntLib [Protocols] gEfiDxeSmmReadyToLockProtocolGuid ## UNDEFINED # SmiHandlerRegister diff --git a/MdeModulePkg/Core/PiSmmCore/PiSmmIpl.inf b/MdeModulePkg/Core/PiSmmCore/PiSmmIpl.inf index 6109d6b5449c..ddeb39cee266 100644 --- a/MdeModulePkg/Core/PiSmmCore/PiSmmIpl.inf +++ b/MdeModulePkg/Core/PiSmmCore/PiSmmIpl.inf @@ -46,6 +46,7 @@ [LibraryClasses] DxeServicesLib PcdLib ReportStatusCodeLib + SafeIntLib [Protocols] gEfiSmmBase2ProtocolGuid ## PRODUCES diff --git a/MdeModulePkg/Core/PiSmmCore/PiSmmCore.h b/MdeModulePkg/Core/PiSmmCore/PiSmmCore.h index 71422b9dfcdf..b8a490a8c3b5 100644 --- a/MdeModulePkg/Core/PiSmmCore/PiSmmCore.h +++ b/MdeModulePkg/Core/PiSmmCore/PiSmmCore.h @@ -54,6 +54,7 @@ #include <Library/PerformanceLib.h> #include <Library/HobLib.h> #include <Library/SmmMemLib.h> +#include <Library/SafeIntLib.h> #include "PiSmmCorePrivateData.h" #include "HeapGuard.h" diff --git a/MdeModulePkg/Core/PiSmmCore/PiSmmCore.c b/MdeModulePkg/Core/PiSmmCore/PiSmmCore.c index 9e5c6cbe33dd..5ef8b31ee361 100644 --- a/MdeModulePkg/Core/PiSmmCore/PiSmmCore.c +++ b/MdeModulePkg/Core/PiSmmCore/PiSmmCore.c @@ -610,6 +610,7 @@ SmmEndOfS3ResumeHandler ( @param[in] Size2 Size of Buff2 @retval TRUE Buffers overlap in memory. + @retval TRUE Math error. @retval FALSE Buffer doesn't overlap. **/ @@ -621,11 +622,21 @@ InternalIsBufferOverlapped ( IN UINTN Size2 ) { + UINTN End1; + UINTN End2; + + // Prevents potential math over and underflows. + if (EFI_ERROR (SafeUintnAdd ((UINTN)Buff1, Size1, &End1)) || + EFI_ERROR (SafeUintnAdd ((UINTN)Buff2, Size2, &End2))) + { + return TRUE; + } + // // If buff1's end is less than the start of buff2, then it's ok. // Also, if buff1's start is beyond buff2's end, then it's ok. // - if (((Buff1 + Size1) <= Buff2) || (Buff1 >= (Buff2 + Size2))) { + if ((End1 <= (UINTN)Buff2) || ((UINTN)Buff1 >= End2)) { return FALSE; } @@ -699,7 +710,10 @@ SmmEntryPoint ( (UINT8 *)gSmmCorePrivate, sizeof (*gSmmCorePrivate) ); - if (!SmmIsBufferOutsideSmmValid ((UINTN)CommunicationBuffer, BufferSize) || IsOverlapped) { + if (!SmmIsBufferOutsideSmmValid ((UINTN)CommunicationBuffer, BufferSize) || + IsOverlapped || + EFI_ERROR (SafeUintnSub (BufferSize, OFFSET_OF (EFI_SMM_COMMUNICATE_HEADER, Data), &BufferSize))) + { // // If CommunicationBuffer is not in valid address scope, // or there is overlap between gSmmCorePrivate and CommunicationBuffer, @@ -709,13 +723,13 @@ SmmEntryPoint ( gSmmCorePrivate->ReturnStatus = EFI_ACCESS_DENIED; } else { CommunicateHeader = (EFI_SMM_COMMUNICATE_HEADER *)CommunicationBuffer; - BufferSize -= OFFSET_OF (EFI_SMM_COMMUNICATE_HEADER, Data); - Status = SmiManage ( - &CommunicateHeader->HeaderGuid, - NULL, - CommunicateHeader->Data, - &BufferSize - ); + // BufferSize was updated by the SafeUintnSub() call above. + Status = SmiManage ( + &CommunicateHeader->HeaderGuid, + NULL, + CommunicateHeader->Data, + &BufferSize + ); // // Update CommunicationBuffer, BufferSize and ReturnStatus // Communicate service finished, reset the pointer to CommBuffer to NULL diff --git a/MdeModulePkg/Core/PiSmmCore/PiSmmIpl.c b/MdeModulePkg/Core/PiSmmCore/PiSmmIpl.c index 4f00cebaf5ed..f08ca55e26b7 100644 --- a/MdeModulePkg/Core/PiSmmCore/PiSmmIpl.c +++ b/MdeModulePkg/Core/PiSmmCore/PiSmmIpl.c @@ -34,6 +34,7 @@ #include <Library/UefiRuntimeLib.h> #include <Library/PcdLib.h> #include <Library/ReportStatusCodeLib.h> +#include <Library/SafeIntLib.h> #include "PiSmmCorePrivateData.h" @@ -1354,6 +1355,7 @@ SmmSplitSmramEntry ( @param[in] ReservedRangeToCompare Pointer to EFI_SMM_RESERVED_SMRAM_REGION to compare. @retval TRUE There is overlap. + @retval TRUE Math error. @retval FALSE There is no overlap. **/ @@ -1366,8 +1368,12 @@ SmmIsSmramOverlap ( UINT64 RangeToCompareEnd; UINT64 ReservedRangeToCompareEnd; - RangeToCompareEnd = RangeToCompare->CpuStart + RangeToCompare->PhysicalSize; - ReservedRangeToCompareEnd = ReservedRangeToCompare->SmramReservedStart + ReservedRangeToCompare->SmramReservedSize; + // Prevents potential math over and underflows. + if (EFI_ERROR (SafeUint64Add ((UINT64)RangeToCompare->CpuStart, RangeToCompare->PhysicalSize, &RangeToCompareEnd)) || + EFI_ERROR (SafeUint64Add ((UINT64)ReservedRangeToCompare->SmramReservedStart, ReservedRangeToCompare->SmramReservedSize, &ReservedRangeToCompareEnd))) + { + return TRUE; + } if ((RangeToCompare->CpuStart >= ReservedRangeToCompare->SmramReservedStart) && (RangeToCompare->CpuStart < ReservedRangeToCompareEnd)) -- 2.37.3 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#94705): https://edk2.groups.io/g/devel/message/94705 Mute This Topic: https://groups.io/mt/94114555/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-