[edk2-devel] [PATCH] EmbeddedPkg/PrePiMemoryAllocationLib: Add check for space on offset allocation

Jeff Brasen via groups.io posted 1 patch 1 year, 10 months ago
Failed in applying to current master (apply log)
There is a newer version of this series
1 file changed, 30 insertions(+), 23 deletions(-)
[edk2-devel] [PATCH] EmbeddedPkg/PrePiMemoryAllocationLib: Add check for space on offset allocation
Posted by Jeff Brasen via groups.io 1 year, 10 months ago
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]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH] EmbeddedPkg/PrePiMemoryAllocationLib: Add check for space on offset allocation
Posted by Ard Biesheuvel 1 year, 7 months ago
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]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH] EmbeddedPkg/PrePiMemoryAllocationLib: Add check for space on offset allocation
Posted by Jeff Brasen via groups.io 1 year, 7 months ago
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]
-=-=-=-=-=-=-=-=-=-=-=-