On 1/9/23 08:18, Ilias Apalodimas wrote:
On Thu, Jan 05, 2023 at 09:25:35PM +0100, Heinrich Schuchardt wrote:
Carve out code from efidebug command used to read the memory map.
Signed-off-by: Heinrich Schuchardt heinrich.schuchardt@canonical.com
v2: new patch
cmd/efidebug.c | 18 ++++-------------- include/efi_loader.h | 3 +++ lib/efi_loader/efi_memory.c | 34 ++++++++++++++++++++++++++++++++++ 3 files changed, 41 insertions(+), 14 deletions(-)
diff --git a/cmd/efidebug.c b/cmd/efidebug.c index 569003ae2e..e6959ede93 100644 --- a/cmd/efidebug.c +++ b/cmd/efidebug.c @@ -591,25 +591,15 @@ static void print_memory_attributes(u64 attributes) static int do_efi_show_memmap(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) {
- struct efi_mem_desc *memmap = NULL, *map;
- efi_uintn_t map_size = 0;
- struct efi_mem_desc *memmap, *map;
- efi_uintn_t map_size; const char *type; int i; efi_status_t ret;
- ret = efi_get_memory_map(&map_size, memmap, NULL, NULL, NULL);
- if (ret == EFI_BUFFER_TOO_SMALL) {
map_size += sizeof(struct efi_mem_desc); /* for my own */ret = efi_allocate_pool(EFI_BOOT_SERVICES_DATA, map_size,(void *)&memmap);if (ret != EFI_SUCCESS)return CMD_RET_FAILURE;ret = efi_get_memory_map(&map_size, memmap, NULL, NULL, NULL);- }
- if (ret != EFI_SUCCESS) {
efi_free_pool(memmap);
- ret = efi_get_memory_map_alloc(&map_size, &memmap);
- if (ret != EFI_SUCCESS) return CMD_RET_FAILURE;
}
printf("Type Start%.*s End%.*s Attributes\n", EFI_PHYS_ADDR_WIDTH - 5, spc, EFI_PHYS_ADDR_WIDTH - 3, spc);
diff --git a/include/efi_loader.h b/include/efi_loader.h index 0899e293e5..02d151b715 100644 --- a/include/efi_loader.h +++ b/include/efi_loader.h @@ -734,6 +734,9 @@ efi_status_t efi_allocate_pool(enum efi_memory_type pool_type, efi_uintn_t size, void **buffer); /* EFI pool memory free function. */ efi_status_t efi_free_pool(void *buffer); +/* Allocate and retrieve EFI memory map */ +efi_status_t efi_get_memory_map_alloc(efi_uintn_t *map_size,
struct efi_mem_desc **memory_map);diff --git a/lib/efi_loader/efi_memory.c b/lib/efi_loader/efi_memory.c index 8d347f101f..32254d2433 100644 --- a/lib/efi_loader/efi_memory.c +++ b/lib/efi_loader/efi_memory.c @@ -736,6 +736,40 @@ efi_status_t efi_get_memory_map(efi_uintn_t *memory_map_size, return EFI_SUCCESS; }
+/**
- efi_get_memory_map_alloc() - allocate map describing memory usage
- The caller is responsible for calling FreePool() if the call succeeds.
- @memory_map buffer to which the memory map is written
- @map_size size of the memory map
- Return: status code
- */
+efi_status_t efi_get_memory_map_alloc(efi_uintn_t *map_size,
struct efi_mem_desc **memory_map)+{
- efi_status_t ret;
- *memory_map = NULL;
- *map_size = 0;
- ret = efi_get_memory_map(map_size, *memory_map, NULL, NULL, NULL);
Although this is correct and efi_get_memory_map() will only return EFI_BUFFER_TOO_SMALL, since we initialize the map_size to 0, I don't know if code analysis tools are smart enough to understand this. Perhaps we should initialize ret?
After an assignment ret cannot be uninitialized.
Did you find a path through efi_get_memory_map() returning an undefined value?
Best regards
Heinrich
- if (ret == EFI_BUFFER_TOO_SMALL) {
*map_size += sizeof(struct efi_mem_desc); /* for the map */ret = efi_allocate_pool(EFI_BOOT_SERVICES_DATA, *map_size,(void **)memory_map);if (ret != EFI_SUCCESS)return ret;ret = efi_get_memory_map(map_size, *memory_map,NULL, NULL, NULL);if (ret != EFI_SUCCESS) {efi_free_pool(*memory_map);*memory_map = NULL;}- }
- return ret;
+}
- /**
- efi_add_conventional_memory_map() - add a RAM memory area to the map
-- 2.37.2
Reviewed-by: Ilias Apalodimas ilias.apalodimas@linaro.org