* [PATCH] EmbeddedPkg/PrePiMemoryAllocationLib: Add check for space on offset allocation
@ 2022-06-30 19:06 Jeff Brasen
2022-09-07 8:33 ` Ard Biesheuvel
0 siblings, 1 reply; 3+ messages in thread
From: Jeff Brasen @ 2022-06-30 19:06 UTC (permalink / raw)
To: devel
Cc: quic_llindhol, ardb+tianocore, abner.chang, daniel.schaefer,
Jeff Brasen
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
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [PATCH] EmbeddedPkg/PrePiMemoryAllocationLib: Add check for space on offset allocation
2022-06-30 19:06 [PATCH] EmbeddedPkg/PrePiMemoryAllocationLib: Add check for space on offset allocation Jeff Brasen
@ 2022-09-07 8:33 ` Ard Biesheuvel
2022-09-07 14:46 ` Jeff Brasen
0 siblings, 1 reply; 3+ messages in thread
From: Ard Biesheuvel @ 2022-09-07 8:33 UTC (permalink / raw)
To: Jeff Brasen
Cc: devel, quic_llindhol, ardb+tianocore, abner.chang,
daniel.schaefer
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
>
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH] EmbeddedPkg/PrePiMemoryAllocationLib: Add check for space on offset allocation
2022-09-07 8:33 ` Ard Biesheuvel
@ 2022-09-07 14:46 ` Jeff Brasen
0 siblings, 0 replies; 3+ messages in thread
From: Jeff Brasen @ 2022-09-07 14:46 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: devel@edk2.groups.io, quic_llindhol@quicinc.com,
ardb+tianocore@kernel.org, abner.chang@hpe.com,
daniel.schaefer@hpe.com
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
> >
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2022-09-07 14:46 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-06-30 19:06 [PATCH] EmbeddedPkg/PrePiMemoryAllocationLib: Add check for space on offset allocation Jeff Brasen
2022-09-07 8:33 ` Ard Biesheuvel
2022-09-07 14:46 ` Jeff Brasen
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox