[PATCH EDK2 v1 1/1] MdeModulePkg/PiSmmCore:Avoid overflow risk


wenyi,xie
 

As the CommunicationBuffer plus BufferSize may overflow, check the
value first before using.

Cc: Jian J Wang <jian.j.wang@...>
Cc: Liming Gao <gaoliming@...>
Cc: Eric Dong <eric.dong@...>
Cc: Ray Ni <ray.ni@...>
Signed-off-by: Wenyi Xie <xiewenyi2@...>
---
MdeModulePkg/Core/PiSmmCore/PiSmmCore.c | 22 +++++++++++++-------
MdeModulePkg/Core/PiSmmCore/PiSmmIpl.c | 5 +++++
2 files changed, 19 insertions(+), 8 deletions(-)

diff --git a/MdeModulePkg/Core/PiSmmCore/PiSmmCore.c b/MdeModulePkg/Core/PiSmmCore/PiSmmCore.c
index 9e5c6cbe33dd..fcf8c61d7f1b 100644
--- a/MdeModulePkg/Core/PiSmmCore/PiSmmCore.c
+++ b/MdeModulePkg/Core/PiSmmCore/PiSmmCore.c
@@ -613,23 +613,28 @@ SmmEndOfS3ResumeHandler (
@retval FALSE Buffer doesn't overlap.

**/
-BOOLEAN
+EFI_STATUS
InternalIsBufferOverlapped (
IN UINT8 *Buff1,
IN UINTN Size1,
IN UINT8 *Buff2,
- IN UINTN Size2
+ IN UINTN Size2,
+ IN BOOLEAN *IsOverlapped
)
{
+ *IsOverlapped = TRUE;
+ if (((UINTN)Buff1 > MAX_UINTN - Size1) || ((UINTN)Buff2 > MAX_UINTN - Size2)) {
+ return EFI_INVALID_PARAMETER;
+ }
//
// 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))) {
- return FALSE;
+ *IsOverlapped = FALSE;
}

- return TRUE;
+ return EFI_SUCCESS;
}

/**
@@ -693,17 +698,18 @@ SmmEntryPoint (
//
// Synchronous SMI for SMM Core or request from Communicate protocol
//
- IsOverlapped = InternalIsBufferOverlapped (
+ Status = InternalIsBufferOverlapped (
(UINT8 *)CommunicationBuffer,
BufferSize,
(UINT8 *)gSmmCorePrivate,
- sizeof (*gSmmCorePrivate)
+ sizeof (*gSmmCorePrivate),
+ &IsOverlapped
);
- if (!SmmIsBufferOutsideSmmValid ((UINTN)CommunicationBuffer, BufferSize) || IsOverlapped) {
+ if (!SmmIsBufferOutsideSmmValid ((UINTN)CommunicationBuffer, BufferSize) || EFI_ERROR(Status) || IsOverlapped) {
//
// If CommunicationBuffer is not in valid address scope,
// or there is overlap between gSmmCorePrivate and CommunicationBuffer,
- // return EFI_INVALID_PARAMETER
+ // return EFI_ACCESS_DENIED
//
gSmmCorePrivate->CommunicationBuffer = NULL;
gSmmCorePrivate->ReturnStatus = EFI_ACCESS_DENIED;
diff --git a/MdeModulePkg/Core/PiSmmCore/PiSmmIpl.c b/MdeModulePkg/Core/PiSmmCore/PiSmmIpl.c
index 4f00cebaf5ed..bd13cf97ec93 100644
--- a/MdeModulePkg/Core/PiSmmCore/PiSmmIpl.c
+++ b/MdeModulePkg/Core/PiSmmCore/PiSmmIpl.c
@@ -525,6 +525,11 @@ SmmCommunicationCommunicate (

CommunicateHeader = (EFI_SMM_COMMUNICATE_HEADER *)CommBuffer;

+ if (CommunicateHeader->MessageLength > MAX_UINTN - OFFSET_OF (EFI_SMM_COMMUNICATE_HEADER, Data)) {
+ DEBUG ((DEBUG_ERROR, "MessageLength is invalid!\n"));
+ return EFI_INVALID_PARAMETER;
+ }
+
if (CommSize == NULL) {
TempCommSize = OFFSET_OF (EFI_SMM_COMMUNICATE_HEADER, Data) + CommunicateHeader->MessageLength;
} else {
--
2.20.1.windows.1

Join devel@edk2.groups.io to automatically receive all group messages.