[PATCH v8 3/5] lib: efi_loader: efi_memory.c: add efi_realloc() for realloc memory

[Ilias Apalodimas]

On Thu, 3 Jul 2025 at 12:10, Ilias Apalodimas
<ilias.apalodimas at linaro.org> wrote:
>
> Hi Paul
>
>
> On Thu, 3 Jul 2025 at 09:28, Ying-Chun Liu (PaulLiu) <paulliu at debian.org> wrote:
> >
> > From: "Ying-Chun Liu (PaulLiu)" <paul.liu at linaro.org>
> >
> > Add efi_realloc() for realloc memory that previously alloc by efi_alloc().
> >
> > Signed-off-by: Ying-Chun Liu (PaulLiu) <paul.liu at linaro.org>
> > Cc: Heinrich Schuchardt <xypron.glpk at gmx.de>
> > Cc: Ilias Apalodimas <ilias.apalodimas at linaro.org>
> > Cc: Peter Robinson <pbrobinson at gmail.com>
> > Cc: Simon Glass <sjg at chromium.org>
> > ---
> > V4: Fix efi_realloc return failure code.
> > V6: Return error status when efi_alloc failed.
> > ---
> >  include/efi_loader.h        | 10 +++++++
> >  lib/efi_loader/efi_memory.c | 58 +++++++++++++++++++++++++++++++++++++
> >  2 files changed, 68 insertions(+)
> >
> > diff --git a/include/efi_loader.h b/include/efi_loader.h
> > index d0c72d0bc58..13ca2ec9a4e 100644
> > --- a/include/efi_loader.h
> > +++ b/include/efi_loader.h
> > @@ -878,6 +878,16 @@ efi_status_t efi_next_variable_name(efi_uintn_t *size, u16 **buf,
> >  #define efi_size_in_pages(size) (((size) + EFI_PAGE_MASK) >> EFI_PAGE_SHIFT)
> >  /* Allocate boot service data pool memory */
> >  void *efi_alloc(size_t len);
> > +/**
> > + * efi_realloc() - reallocate boot services data pool memory
> > + *
> > + * Reallocate memory from pool for a new size and copy the data from old one.
> > + *
> > + * @ptr:       pointer to the buffer
> > + * @size:      number of bytes to allocate
> > + * Return:     EFI status to indicate success or not
> > + */
> > +efi_status_t efi_realloc(void **ptr, size_t len);
> >  /* Allocate pages on the specified alignment */
> >  void *efi_alloc_aligned_pages(u64 len, int memory_type, size_t align);
> >  /* More specific EFI memory allocator, called by EFI payloads */
> > diff --git a/lib/efi_loader/efi_memory.c b/lib/efi_loader/efi_memory.c
> > index 0abb1f6159a..1570e300570 100644
> > --- a/lib/efi_loader/efi_memory.c
> > +++ b/lib/efi_loader/efi_memory.c
> > @@ -666,6 +666,64 @@ void *efi_alloc(size_t size)
> >         return buf;
> >  }
> >
> > +/**
> > + * efi_realloc() - reallocate boot services data pool memory
> > + *
> > + * Reallocate memory from pool for a new size and copy the data from old one.
> > + *
> > + * @ptr:       pointer to old buffer
> > + * @size:      number of bytes to allocate
> > + * Return:     EFI status to indicate success or not
> > + */
> > +efi_status_t efi_realloc(void **ptr, size_t size)
> > +{
> > +       efi_status_t ret;
> > +       void *new_ptr;
> > +       struct efi_pool_allocation *alloc;
> > +       u64 num_pages = efi_size_in_pages(size +
> > +                                         sizeof(struct efi_pool_allocation));
> > +       size_t old_size;
> > +
> > +       if (!*ptr) {
> > +               *ptr = efi_alloc(size);
> > +               if (*ptr)
> > +                       return EFI_SUCCESS;
> > +               return EFI_OUT_OF_RESOURCES;
> > +       }
> > +
> > +       ret = efi_check_allocated((uintptr_t)*ptr, true);
> > +       if (ret != EFI_SUCCESS)
> > +               return ret;
> > +
>
> Don't we also need a check here that the memory type you are trying to
> reallocate is Boot Services Data?
> If someone uses this currently to reallocate memory, it will change
> whatever type he has to BDS.
>
> You either add a check or better don't use efi_alloc(). Use
> efi_allocate_pool() and preserve the type.

I am fine picking this up as is, since we dont have any consumers yet
-- as long as you send a follow up with either the check of conver
efi_realloc to efi_allocate_pool and preserve the type

Reviewed-by: Ilias Apalodimas <ilias.apalodimas at linaro.org> # if
memory is realloced it's explicitly converted to BootServicesData

>
> > +       alloc = container_of(*ptr, struct efi_pool_allocation, data);
> > +
> > +       /* Check that this memory was allocated by efi_allocate_pool() */
> > +       if (((uintptr_t)alloc & EFI_PAGE_MASK) ||
> > +           alloc->checksum != checksum(alloc)) {
> > +               printf("%s: illegal realloc 0x%p\n", __func__, *ptr);
> > +               return EFI_INVALID_PARAMETER;
> > +       }
> > +
> > +       /* Don't realloc. The actual size in pages is the same. */
> > +       if (alloc->num_pages == num_pages)
> > +               return EFI_SUCCESS;
>
> What if the size is smaller? Can't we just free the extra pages
> instead of reallocing?
>
> > +
> > +       old_size = alloc->num_pages * EFI_PAGE_SIZE -
> > +               sizeof(struct efi_pool_allocation);
> > +
> > +       new_ptr = efi_alloc(size);
> > +
> > +       /* copy old data to new alloced buffer */
> > +       memcpy(new_ptr, *ptr, min(size, old_size));
> > +
> > +       /* free the old buffer */
> > +       efi_free_pool(*ptr);
> > +
> > +       *ptr = new_ptr;
> > +
> > +       return EFI_SUCCESS;
> > +}
> > +
> >  /**
> >   * efi_free_pool() - free memory from pool
> >   *
> > --
> > 2.39.5
> >
>
> Thanks
> /Ilias