On Thu, 3 Jul 2025 at 12:10, Ilias Apalodimas
<ilias.apalodi...@linaro.org> wrote:
>
> Hi Paul
>
>
> On Thu, 3 Jul 2025 at 09:28, Ying-Chun Liu (PaulLiu) <paul...@debian.org>
> wrote:
> >
> > From: "Ying-Chun Liu (PaulLiu)" <paul....@linaro.org>
> >
> > Add efi_realloc() for realloc memory that previously alloc by efi_alloc().
> >
> > Signed-off-by: Ying-Chun Liu (PaulLiu) <paul....@linaro.org>
> > Cc: Heinrich Schuchardt <xypron.g...@gmx.de>
> > Cc: Ilias Apalodimas <ilias.apalodi...@linaro.org>
> > Cc: Peter Robinson <pbrobin...@gmail.com>
> > Cc: Simon Glass <s...@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.apalodi...@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
