[U-Boot] [PATCH 0/4] efi_loader: fixes for loaded image protocol

This patch series fixes various bugs in the handling of loaded images.
Heinrich Schuchardt (4): efi_loader: used efi_uintn_t for LoadImage efi_loader: correctly set ImageBase for loaded image efi_loader: ImageSize must be multiple of SectionAlignment efi_loader: correct types for EFI_LOADED_IMAGE_PROTOCOL
include/efi_api.h | 6 +++--- lib/efi_loader/efi_boottime.c | 4 ++-- lib/efi_loader/efi_image_loader.c | 9 ++++----- 3 files changed, 9 insertions(+), 10 deletions(-)

We generally use efi_uintn_t where the UEFI spec uses UINTN.
Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de --- include/efi_api.h | 2 +- lib/efi_loader/efi_boottime.c | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/include/efi_api.h b/include/efi_api.h index 28de93a132..f138fc90ec 100644 --- a/include/efi_api.h +++ b/include/efi_api.h @@ -107,7 +107,7 @@ struct efi_boot_services { efi_status_t (EFIAPI *load_image)(bool boot_policiy, efi_handle_t parent_image, struct efi_device_path *file_path, void *source_buffer, - unsigned long source_size, efi_handle_t *image); + efi_uintn_t source_size, efi_handle_t *image); efi_status_t (EFIAPI *start_image)(efi_handle_t handle, unsigned long *exitdata_size, s16 **exitdata); diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c index fd35ffa359..d15a131e74 100644 --- a/lib/efi_loader/efi_boottime.c +++ b/lib/efi_loader/efi_boottime.c @@ -1568,14 +1568,14 @@ static efi_status_t EFIAPI efi_load_image(bool boot_policy, efi_handle_t parent_image, struct efi_device_path *file_path, void *source_buffer, - unsigned long source_size, + efi_uintn_t source_size, efi_handle_t *image_handle) { struct efi_loaded_image *info; struct efi_object *obj; efi_status_t ret;
- EFI_ENTRY("%d, %p, %pD, %p, %ld, %p", boot_policy, parent_image, + EFI_ENTRY("%d, %p, %pD, %p, %zd, %p", boot_policy, parent_image, file_path, source_buffer, source_size, image_handle);
if (!image_handle || !parent_image) {

In the EFI_LOADED_IMAGE_PROTOCOL ImageBase and ImageSize should reflect where the loaded image can be found in memory. If we do a relocation this is the relocated location and size.
Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de --- lib/efi_loader/efi_image_loader.c | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/lib/efi_loader/efi_image_loader.c b/lib/efi_loader/efi_image_loader.c index cac64ba9fe..5125340fc0 100644 --- a/lib/efi_loader/efi_image_loader.c +++ b/lib/efi_loader/efi_image_loader.c @@ -124,7 +124,6 @@ void *efi_load_pe(void *efi, struct efi_loaded_image *loaded_image_info) unsigned long rel_size; int rel_idx = IMAGE_DIRECTORY_ENTRY_BASERELOC; void *entry; - uint64_t image_size; unsigned long virt_size = 0; bool can_run_nt64 = true; bool can_run_nt32 = true; @@ -163,7 +162,6 @@ void *efi_load_pe(void *efi, struct efi_loaded_image *loaded_image_info) (nt->OptionalHeader.Magic == IMAGE_NT_OPTIONAL_HDR64_MAGIC)) { IMAGE_NT_HEADERS64 *nt64 = (void *)nt; IMAGE_OPTIONAL_HEADER64 *opt = &nt64->OptionalHeader; - image_size = opt->SizeOfImage; efi_set_code_and_data_type(loaded_image_info, opt->Subsystem); efi_reloc = efi_alloc(virt_size, loaded_image_info->image_code_type); @@ -178,7 +176,6 @@ void *efi_load_pe(void *efi, struct efi_loaded_image *loaded_image_info) } else if (can_run_nt32 && (nt->OptionalHeader.Magic == IMAGE_NT_OPTIONAL_HDR32_MAGIC)) { IMAGE_OPTIONAL_HEADER32 *opt = &nt->OptionalHeader; - image_size = opt->SizeOfImage; efi_set_code_and_data_type(loaded_image_info, opt->Subsystem); efi_reloc = efi_alloc(virt_size, loaded_image_info->image_code_type); @@ -219,8 +216,8 @@ void *efi_load_pe(void *efi, struct efi_loaded_image *loaded_image_info) invalidate_icache_all();
/* Populate the loaded image interface bits */ - loaded_image_info->image_base = efi; - loaded_image_info->image_size = image_size; + loaded_image_info->image_base = efi_reloc; + loaded_image_info->image_size = virt_size;
return entry; }

According to the Portable Executable and Common Object File Format Specification the image size must be a multiple of the alignment of sections.
Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de --- lib/efi_loader/efi_image_loader.c | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/lib/efi_loader/efi_image_loader.c b/lib/efi_loader/efi_image_loader.c index 5125340fc0..07251693cb 100644 --- a/lib/efi_loader/efi_image_loader.c +++ b/lib/efi_loader/efi_image_loader.c @@ -173,6 +173,7 @@ void *efi_load_pe(void *efi, struct efi_loaded_image *loaded_image_info) entry = efi_reloc + opt->AddressOfEntryPoint; rel_size = opt->DataDirectory[rel_idx].Size; rel = efi_reloc + opt->DataDirectory[rel_idx].VirtualAddress; + virt_size = ALIGN(virt_size, opt->SectionAlignment); } else if (can_run_nt32 && (nt->OptionalHeader.Magic == IMAGE_NT_OPTIONAL_HDR32_MAGIC)) { IMAGE_OPTIONAL_HEADER32 *opt = &nt->OptionalHeader; @@ -187,6 +188,7 @@ void *efi_load_pe(void *efi, struct efi_loaded_image *loaded_image_info) entry = efi_reloc + opt->AddressOfEntryPoint; rel_size = opt->DataDirectory[rel_idx].Size; rel = efi_reloc + opt->DataDirectory[rel_idx].VirtualAddress; + virt_size = ALIGN(virt_size, opt->SectionAlignment); } else { printf("%s: Invalid optional header magic %x\n", __func__, nt->OptionalHeader.Magic);

We should not use void * but specific types for * device_handle * file_path
Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de --- include/efi_api.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/include/efi_api.h b/include/efi_api.h index f138fc90ec..833ef8b4a6 100644 --- a/include/efi_api.h +++ b/include/efi_api.h @@ -318,8 +318,8 @@ struct efi_loaded_image { u32 revision; void *parent_handle; struct efi_system_table *system_table; - void *device_handle; - void *file_path; + efi_handle_t device_handle; + struct efi_device_path *file_path; void *reserved; u32 load_options_size; void *load_options;
participants (1)
-
Heinrich Schuchardt