1 file changed, 30 insertions(+), 23 deletions(-)
Update check for enough space to occur prior to alignment offset.
This prevents cases where EfiFreeMemoryTop < EfiFreeMemoryBottom.
Signed-off-by: Jeff Brasen <jbrasen@nvidia.com>
---
.../MemoryAllocationLib.c | 53 +++++++++++--------
1 file changed, 30 insertions(+), 23 deletions(-)
diff --git a/EmbeddedPkg/Library/PrePiMemoryAllocationLib/MemoryAllocationLib.c b/EmbeddedPkg/Library/PrePiMemoryAllocationLib/MemoryAllocationLib.c
index 78f8da5e95..1956d644c3 100644
--- a/EmbeddedPkg/Library/PrePiMemoryAllocationLib/MemoryAllocationLib.c
+++ b/EmbeddedPkg/Library/PrePiMemoryAllocationLib/MemoryAllocationLib.c
@@ -38,37 +38,44 @@ AllocatePages (
Hob.Raw = GetHobList ();
- // Check to see if on 4k boundary
Offset = Hob.HandoffInformationTable->EfiFreeMemoryTop & 0xFFF;
+ //
+ // Verify that there is sufficient memory to satisfy the allocation and padding prior to updating anything
+ //
+ if ((Hob.HandoffInformationTable->EfiFreeMemoryTop - ((Pages * EFI_PAGE_SIZE) + sizeof (EFI_HOB_MEMORY_ALLOCATION)) - Offset) < Hob.HandoffInformationTable->EfiFreeMemoryBottom) {
+ if (Offset != 0) {
+ DEBUG ((DEBUG_ERROR, "Offset applied without enough space\r\n"));
+ } else {
+ DEBUG ((DEBUG_ERROR, "Out of memory\r\n"));
+ }
+
+ ASSERT (FALSE);
+ return 0;
+ }
+
+ // Check to see if on 4k boundary
if (Offset != 0) {
// If not aligned, make the allocation aligned.
Hob.HandoffInformationTable->EfiFreeMemoryTop -= Offset;
}
//
- // Verify that there is sufficient memory to satisfy the allocation
+ // Update the PHIT to reflect the memory usage
//
- if (Hob.HandoffInformationTable->EfiFreeMemoryTop - ((Pages * EFI_PAGE_SIZE) + sizeof (EFI_HOB_MEMORY_ALLOCATION)) < Hob.HandoffInformationTable->EfiFreeMemoryBottom) {
- return 0;
- } else {
- //
- // Update the PHIT to reflect the memory usage
- //
- Hob.HandoffInformationTable->EfiFreeMemoryTop -= Pages * EFI_PAGE_SIZE;
-
- // This routine used to create a memory allocation HOB a la PEI, but that's not
- // necessary for us.
-
- //
- // Create a memory allocation HOB.
- //
- BuildMemoryAllocationHob (
- Hob.HandoffInformationTable->EfiFreeMemoryTop,
- Pages * EFI_PAGE_SIZE,
- EfiBootServicesData
- );
- return (VOID *)(UINTN)Hob.HandoffInformationTable->EfiFreeMemoryTop;
- }
+ Hob.HandoffInformationTable->EfiFreeMemoryTop -= Pages * EFI_PAGE_SIZE;
+
+ // This routine used to create a memory allocation HOB a la PEI, but that's not
+ // necessary for us.
+
+ //
+ // Create a memory allocation HOB.
+ //
+ BuildMemoryAllocationHob (
+ Hob.HandoffInformationTable->EfiFreeMemoryTop,
+ Pages * EFI_PAGE_SIZE,
+ EfiBootServicesData
+ );
+ return (VOID *)(UINTN)Hob.HandoffInformationTable->EfiFreeMemoryTop;
}
/**
--
2.25.1
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#90936): https://edk2.groups.io/g/devel/message/90936
Mute This Topic: https://groups.io/mt/92093864/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
On Thu, 30 Jun 2022 at 21:06, Jeff Brasen <jbrasen@nvidia.com> wrote: > > Update check for enough space to occur prior to alignment offset. > This prevents cases where EfiFreeMemoryTop < EfiFreeMemoryBottom. > So prior to this patch, we would - check for enough space - apply the alignment - potentially exceed the available space due to alignment padding? > Signed-off-by: Jeff Brasen <jbrasen@nvidia.com> This patch got mangled so I cannot apply it from the list. > --- > .../MemoryAllocationLib.c | 53 +++++++++++-------- > 1 file changed, 30 insertions(+), 23 deletions(-) > > diff --git a/EmbeddedPkg/Library/PrePiMemoryAllocationLib/MemoryAllocationLib.c b/EmbeddedPkg/Library/PrePiMemoryAllocationLib/MemoryAllocationLib.c > index 78f8da5e95..1956d644c3 100644 > --- a/EmbeddedPkg/Library/PrePiMemoryAllocationLib/MemoryAllocationLib.c > +++ b/EmbeddedPkg/Library/PrePiMemoryAllocationLib/MemoryAllocationLib.c > @@ -38,37 +38,44 @@ AllocatePages ( > > Hob.Raw = GetHobList (); > > - // Check to see if on 4k boundary > Offset = Hob.HandoffInformationTable->EfiFreeMemoryTop & 0xFFF; > + // > + // Verify that there is sufficient memory to satisfy the allocation and padding prior to updating anything > + // > + if ((Hob.HandoffInformationTable->EfiFreeMemoryTop - ((Pages * EFI_PAGE_SIZE) + sizeof (EFI_HOB_MEMORY_ALLOCATION)) - Offset) < Hob.HandoffInformationTable->EfiFreeMemoryBottom) { > + if (Offset != 0) { > + DEBUG ((DEBUG_ERROR, "Offset applied without enough space\r\n")); > + } else { > + DEBUG ((DEBUG_ERROR, "Out of memory\r\n")); > + } > + > + ASSERT (FALSE); > + return 0; > + } > + > + // Check to see if on 4k boundary > if (Offset != 0) { > // If not aligned, make the allocation aligned. > Hob.HandoffInformationTable->EfiFreeMemoryTop -= Offset; > } > > // > - // Verify that there is sufficient memory to satisfy the allocation > + // Update the PHIT to reflect the memory usage > // > - if (Hob.HandoffInformationTable->EfiFreeMemoryTop - ((Pages * EFI_PAGE_SIZE) + sizeof (EFI_HOB_MEMORY_ALLOCATION)) < Hob.HandoffInformationTable->EfiFreeMemoryBottom) { > - return 0; > - } else { > - // > - // Update the PHIT to reflect the memory usage > - // > - Hob.HandoffInformationTable->EfiFreeMemoryTop -= Pages * EFI_PAGE_SIZE; > - > - // This routine used to create a memory allocation HOB a la PEI, but that's not > - // necessary for us. > - > - // > - // Create a memory allocation HOB. > - // > - BuildMemoryAllocationHob ( > - Hob.HandoffInformationTable->EfiFreeMemoryTop, > - Pages * EFI_PAGE_SIZE, > - EfiBootServicesData > - ); > - return (VOID *)(UINTN)Hob.HandoffInformationTable->EfiFreeMemoryTop; > - } > + Hob.HandoffInformationTable->EfiFreeMemoryTop -= Pages * EFI_PAGE_SIZE; > + > + // This routine used to create a memory allocation HOB a la PEI, but that's not > + // necessary for us. > + > + // > + // Create a memory allocation HOB. > + // > + BuildMemoryAllocationHob ( > + Hob.HandoffInformationTable->EfiFreeMemoryTop, > + Pages * EFI_PAGE_SIZE, > + EfiBootServicesData > + ); > + return (VOID *)(UINTN)Hob.HandoffInformationTable->EfiFreeMemoryTop; > } > > /** > -- > 2.25.1 > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#93370): https://edk2.groups.io/g/devel/message/93370 Mute This Topic: https://groups.io/mt/92093864/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=-=-=-=-=-=-=-=-=-=-=-
I'll post another version to fix the formatting issue in a bit, but before the patch the issue was we applied the alignment offset before we did a space check. -Jeff > -----Original Message----- > From: Ard Biesheuvel <ardb@kernel.org> > Sent: Wednesday, September 7, 2022 2:34 AM > To: Jeff Brasen <jbrasen@nvidia.com> > Cc: devel@edk2.groups.io; quic_llindhol@quicinc.com; > ardb+tianocore@kernel.org; abner.chang@hpe.com; daniel.schaefer@hpe.com > Subject: Re: [PATCH] EmbeddedPkg/PrePiMemoryAllocationLib: Add check for > space on offset allocation > > External email: Use caution opening links or attachments > > > On Thu, 30 Jun 2022 at 21:06, Jeff Brasen <jbrasen@nvidia.com> wrote: > > > > Update check for enough space to occur prior to alignment offset. > > This prevents cases where EfiFreeMemoryTop < EfiFreeMemoryBottom. > > > > So prior to this patch, we would > - check for enough space > - apply the alignment > - potentially exceed the available space due to alignment padding? > > > Signed-off-by: Jeff Brasen <jbrasen@nvidia.com> > > This patch got mangled so I cannot apply it from the list. > > > --- > > .../MemoryAllocationLib.c | 53 +++++++++++-------- > > 1 file changed, 30 insertions(+), 23 deletions(-) > > > > diff --git > > a/EmbeddedPkg/Library/PrePiMemoryAllocationLib/MemoryAllocationLib.c > > b/EmbeddedPkg/Library/PrePiMemoryAllocationLib/MemoryAllocationLib.c > > index 78f8da5e95..1956d644c3 100644 > > --- > > a/EmbeddedPkg/Library/PrePiMemoryAllocationLib/MemoryAllocationLib.c > > +++ b/EmbeddedPkg/Library/PrePiMemoryAllocationLib/MemoryAllocationLib > > +++ .c > > @@ -38,37 +38,44 @@ AllocatePages ( > > > > Hob.Raw = GetHobList (); > > > > - // Check to see if on 4k boundary > > Offset = Hob.HandoffInformationTable->EfiFreeMemoryTop & 0xFFF; > > + // > > + // Verify that there is sufficient memory to satisfy the allocation > > + and padding prior to updating anything // if > > + ((Hob.HandoffInformationTable->EfiFreeMemoryTop - ((Pages * > EFI_PAGE_SIZE) + sizeof (EFI_HOB_MEMORY_ALLOCATION)) - Offset) < > Hob.HandoffInformationTable->EfiFreeMemoryBottom) { > > + if (Offset != 0) { > > + DEBUG ((DEBUG_ERROR, "Offset applied without enough space\r\n")); > > + } else { > > + DEBUG ((DEBUG_ERROR, "Out of memory\r\n")); > > + } > > + > > + ASSERT (FALSE); > > + return 0; > > + } > > + > > + // Check to see if on 4k boundary > > if (Offset != 0) { > > // If not aligned, make the allocation aligned. > > Hob.HandoffInformationTable->EfiFreeMemoryTop -= Offset; > > } > > > > // > > - // Verify that there is sufficient memory to satisfy the allocation > > + // Update the PHIT to reflect the memory usage > > // > > - if (Hob.HandoffInformationTable->EfiFreeMemoryTop - ((Pages * > EFI_PAGE_SIZE) + sizeof (EFI_HOB_MEMORY_ALLOCATION)) < > Hob.HandoffInformationTable->EfiFreeMemoryBottom) { > > - return 0; > > - } else { > > - // > > - // Update the PHIT to reflect the memory usage > > - // > > - Hob.HandoffInformationTable->EfiFreeMemoryTop -= Pages * > EFI_PAGE_SIZE; > > - > > - // This routine used to create a memory allocation HOB a la PEI, but that's > not > > - // necessary for us. > > - > > - // > > - // Create a memory allocation HOB. > > - // > > - BuildMemoryAllocationHob ( > > - Hob.HandoffInformationTable->EfiFreeMemoryTop, > > - Pages * EFI_PAGE_SIZE, > > - EfiBootServicesData > > - ); > > - return (VOID *)(UINTN)Hob.HandoffInformationTable- > >EfiFreeMemoryTop; > > - } > > + Hob.HandoffInformationTable->EfiFreeMemoryTop -= Pages * > > + EFI_PAGE_SIZE; > > + > > + // This routine used to create a memory allocation HOB a la PEI, > > + but that's not // necessary for us. > > + > > + // > > + // Create a memory allocation HOB. > > + // > > + BuildMemoryAllocationHob ( > > + Hob.HandoffInformationTable->EfiFreeMemoryTop, > > + Pages * EFI_PAGE_SIZE, > > + EfiBootServicesData > > + ); > > + return (VOID > > + *)(UINTN)Hob.HandoffInformationTable->EfiFreeMemoryTop; > > } > > > > /** > > -- > > 2.25.1 > > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#93431): https://edk2.groups.io/g/devel/message/93431 Mute This Topic: https://groups.io/mt/92093864/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=-=-=-=-=-=-=-=-=-=-=-
© 2016 - 2024 Red Hat, Inc.