Skip to content

Commit

Permalink
StandaloneMmPkg/Core: Fix potential memory leak issue
Browse files Browse the repository at this point in the history
In MmCoreFfsFindMmDriver(),
- ScratchBuffer is not freed in the error return path that DstBuffer page
allocation fails. Free ScratchBuffer before return with error.
- If the decoded buffer is identical to the data in InputSection,
ExtractGuidedSectionDecode() will change the value of DstBuffer rather
than changing the contents of the buffer that DstBuffer points at, in
which case freeing DstBuffer is wrong. Introduce a local variable
AllocatedDstBuffer for buffer free, free AllocatedDstBuffer immediately
if it is not used.

Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
Cc: Sami Mujawar <sami.mujawar@arm.com>
Cc: Ray Ni <ray.ni@intel.com>
Signed-off-by: Wei6 Xu <wei6.xu@intel.com>
Reviewed-by: Ray Ni <ray.ni@intel.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
  • Loading branch information
xuweiintel authored and mergify[bot] committed Dec 19, 2023
1 parent c012284 commit 0904161
Showing 1 changed file with 22 additions and 9 deletions.
31 changes: 22 additions & 9 deletions StandaloneMmPkg/Core/FwVol.c
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,7 @@ MmCoreFfsFindMmDriver (
UINT32 DstBufferSize;
VOID *ScratchBuffer;
UINT32 ScratchBufferSize;
VOID *AllocatedDstBuffer;
VOID *DstBuffer;
UINT16 SectionAttribute;
UINT32 AuthenticationStatus;
Expand Down Expand Up @@ -148,25 +149,35 @@ MmCoreFfsFindMmDriver (
//
// Allocate destination buffer, extra one page for adjustment
//
DstBuffer = (VOID *)(UINTN)AllocatePages (EFI_SIZE_TO_PAGES (DstBufferSize));
if (DstBuffer == NULL) {
AllocatedDstBuffer = (VOID *)(UINTN)AllocatePages (EFI_SIZE_TO_PAGES (DstBufferSize));
if (AllocatedDstBuffer == NULL) {
FreePages (ScratchBuffer, EFI_SIZE_TO_PAGES (ScratchBufferSize));
return EFI_OUT_OF_RESOURCES;
}

//
// Call decompress function
//
Status = ExtractGuidedSectionDecode (
Section,
&DstBuffer,
ScratchBuffer,
&AuthenticationStatus
);
DstBuffer = AllocatedDstBuffer;
Status = ExtractGuidedSectionDecode (
Section,
&DstBuffer,
ScratchBuffer,
&AuthenticationStatus
);
FreePages (ScratchBuffer, EFI_SIZE_TO_PAGES (ScratchBufferSize));
if (EFI_ERROR (Status)) {
goto FreeDstBuffer;
}

//
// Free allocated DstBuffer if it is not used
//
if (DstBuffer != AllocatedDstBuffer) {
FreePages (AllocatedDstBuffer, EFI_SIZE_TO_PAGES (DstBufferSize));
AllocatedDstBuffer = NULL;
}

DEBUG ((
DEBUG_INFO,
"Processing compressed firmware volume (AuthenticationStatus == %x)\n",
Expand Down Expand Up @@ -210,7 +221,9 @@ MmCoreFfsFindMmDriver (
return EFI_SUCCESS;

FreeDstBuffer:
FreePages (DstBuffer, EFI_SIZE_TO_PAGES (DstBufferSize));
if (AllocatedDstBuffer != NULL) {
FreePages (AllocatedDstBuffer, EFI_SIZE_TO_PAGES (DstBufferSize));
}

return Status;
}

0 comments on commit 0904161

Please sign in to comment.