Hi Ard,
On Thu, Jan 08, 2015 at 05:51:47PM +0000, Ard Biesheuvel wrote:
> This fixes two minor issues in the implementation of get_memory_map():
> - Currently, it assumes that sizeof(efi_memory_desc_t) == desc_size,
> which is usually true, but not mandated by the spec. (This was added
> intentionally to allow future additions to the definition of
> efi_memory_desc_t). The way the loop is implemented currently, the
> added slack space may be insufficient if desc_size is larger, which in
> some corner cases could result in the loop never terminating.
> - It allocates 32 efi_memory_desc_t entries first (again, using the size
> of the struct instead of desc_size), and frees and reallocates if it
> turns out to be insufficient. Few implementations of UEFI have such small
> memory maps, which results in a unnecessary allocate/free pair on each
> invocation.
>
> Fix this by calling the get_memory_map() boot service first with a '0'
> input value for map size to retrieve the map size and desc size from the
> firmware and only then perform the allocation, using desc_size rather
> than sizeof(efi_memory_desc_t).
Is the desc_size guaranteed to be set up correctly if the size is too
small?
As far as I can see, for that case the spec only mandates that
MemoryMapSize is updated nd EFI_BUFFER_TOO_SMALL is returned.
It's not clear to me whether DescriptorSize or DescriptorVersion are
initialised in cases other than success.
Thanks,
Mark.
>
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@xxxxxxxxxx>
> ---
> drivers/firmware/efi/libstub/efi-stub-helper.c | 16 ++++++++++------
> 1 file changed, 10 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/firmware/efi/libstub/efi-stub-helper.c b/drivers/firmware/efi/libstub/efi-stub-helper.c
> index e766df60fbfb..caf91eab0bfc 100644
> --- a/drivers/firmware/efi/libstub/efi-stub-helper.c
> +++ b/drivers/firmware/efi/libstub/efi-stub-helper.c
> @@ -75,25 +75,29 @@ efi_status_t efi_get_memory_map(efi_system_table_t *sys_table_arg,
> unsigned long key;
> u32 desc_version;
>
> - *map_size = sizeof(*m) * 32;
> -again:
> + *map_size = 0;
> + *desc_size = 0;
> + key = 0;
> + status = efi_call_early(get_memory_map, map_size, NULL,
> + &key, desc_size, &desc_version);
> + if (status != EFI_BUFFER_TOO_SMALL)
> + return EFI_LOAD_ERROR;
> +
> /*
> * Add an additional efi_memory_desc_t because we're doing an
> * allocation which may be in a new descriptor region.
> */
> - *map_size += sizeof(*m);
> + *map_size += *desc_size;
> status = efi_call_early(allocate_pool, EFI_LOADER_DATA,
> *map_size, (void **)&m);
> if (status != EFI_SUCCESS)
> goto fail;
>
> - *desc_size = 0;
> - key = 0;
> status = efi_call_early(get_memory_map, map_size, m,
> &key, desc_size, &desc_version);
> if (status == EFI_BUFFER_TOO_SMALL) {
> efi_call_early(free_pool, m);
> - goto again;
> + return EFI_LOAD_ERROR;
> }
>
> if (status != EFI_SUCCESS)
> --
> 1.8.3.2
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-efi" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
--
To unsubscribe from this list: send the line "unsubscribe linux-efi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
