[U-Boot] [PATCH v2 0/6] efi_loader: fixes for loaded image protocol
This patch series fixes various bugs in the handling of loaded images.
The following new functionality is provided:
If a crash occurs the relocation information of loaded EFI images is displayed.
--- v2 Merge with "efi_loader: print information about loaded UEFI images" patch series.
GRUB does not allow the relocated address to be used as ImageBase in the loaded image information. So the relocation address has to be stored in an additional field. ---
Heinrich Schuchardt (6): efi_loader: use efi_uintn_t for LoadImage efi_loader: save image relocation address and size efi_loader: ImageSize must be multiple of SectionAlignment efi_loader: correct types for EFI_LOADED_IMAGE_PROTOCOL efi_loader: new functions to print loaded image information arm: print information about loaded UEFI images
arch/arm/lib/interrupts.c | 17 +++++++++++++ include/efi_api.h | 8 ++++--- include/efi_loader.h | 4 ++++ lib/efi_loader/efi_boottime.c | 4 ++-- lib/efi_loader/efi_image_loader.c | 50 +++++++++++++++++++++++++++++++++++++++ 5 files changed, 78 insertions(+), 5 deletions(-)
We generally use efi_uintn_t where the UEFI spec uses UINTN.
Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de --- v2 Fix typo in commit message. --- 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) {
For analyzing crash output the relocation address and size are needed. Save them in the loaded image info.
Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de --- v2 GRUB does not allow the relocated address to be used as ImageBase in the loaded image information. So the relocation address has to be stored in an additional field. --- include/efi_api.h | 2 ++ lib/efi_loader/efi_image_loader.c | 3 +++ 2 files changed, 5 insertions(+)
diff --git a/include/efi_api.h b/include/efi_api.h index f138fc90ec..ca8e7849ff 100644 --- a/include/efi_api.h +++ b/include/efi_api.h @@ -331,6 +331,8 @@ struct efi_loaded_image {
/* Below are efi loader private fields */ #ifdef CONFIG_EFI_LOADER + void *reloc_base; + aligned_u64 reloc_size; efi_status_t exit_status; struct jmp_buf_data exit_jmp; #endif diff --git a/lib/efi_loader/efi_image_loader.c b/lib/efi_loader/efi_image_loader.c index cac64ba9fe..7f602bbf4c 100644 --- a/lib/efi_loader/efi_image_loader.c +++ b/lib/efi_loader/efi_image_loader.c @@ -221,6 +222,8 @@ void *efi_load_pe(void *efi, struct efi_loaded_image *loaded_image_info) /* Populate the loaded image interface bits */ loaded_image_info->image_base = efi; loaded_image_info->image_size = image_size; + loaded_image_info->reloc_base = efi_reloc; + loaded_image_info->reloc_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 --- v2 no change --- 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 7f602bbf4c..770efeffa3 100644 --- a/lib/efi_loader/efi_image_loader.c +++ b/lib/efi_loader/efi_image_loader.c @@ -176,6 +176,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; @@ -191,6 +192,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 --- v2 no change --- 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 ca8e7849ff..c8a41a499d 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;
Introduce functions to print information about loaded images.
If we want to analyze an exception in an EFI image we need the offset between the PC and the start of the loaded image.
With efi_print_image_info() we can print the necessary information for a single image, e.g.
UEFI image start 0x7fdb4000, size 0xa7b60 pc offset 0x72ca /\snp.efi
efi_print_image_infos() provides output for all loaded images.
Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de --- v2 GRUB does not allow the relocated address to be used as ImageBase in the loaded image information. So the relocation address has to be stored in an additional field. --- include/efi_loader.h | 4 ++++ lib/efi_loader/efi_image_loader.c | 45 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 49 insertions(+)
diff --git a/include/efi_loader.h b/include/efi_loader.h index 0df482ee21..ee553c667f 100644 --- a/include/efi_loader.h +++ b/include/efi_loader.h @@ -306,6 +306,10 @@ efi_status_t efi_setup_loaded_image( struct efi_device_path *file_path); efi_status_t efi_load_image_from_path(struct efi_device_path *file_path, void **buffer); +/* Print information about a loaded image */ +efi_status_t efi_print_image_info(struct efi_loaded_image *image, void *pc); +/* Print information about all loaded images */ +void efi_print_image_infos(void *pc);
#ifdef CONFIG_EFI_LOADER_BOUNCE_BUFFER extern void *efi_bounce_buffer; diff --git a/lib/efi_loader/efi_image_loader.c b/lib/efi_loader/efi_image_loader.c index 770efeffa3..663def1039 100644 --- a/lib/efi_loader/efi_image_loader.c +++ b/lib/efi_loader/efi_image_loader.c @@ -22,6 +22,51 @@ const efi_guid_t efi_simple_file_system_protocol_guid = EFI_SIMPLE_FILE_SYSTEM_PROTOCOL_GUID; const efi_guid_t efi_file_info_guid = EFI_FILE_INFO_GUID;
+/* + * Print information about a loaded image. + * + * If the program counter is located within the image the offset to the base + * address is shown. + * + * @image: loaded image + * @pc: program counter (use NULL to suppress offset output) + * @return: status code + */ +efi_status_t efi_print_image_info(struct efi_loaded_image *image, void *pc) +{ + if (!image) + return EFI_INVALID_PARAMETER; + printf("UEFI image\nstart 0x%p, size 0x%llx\n", + image->reloc_base, image->reloc_size); + if (pc && pc >= image->reloc_base && + pc < image->reloc_base + image->reloc_size) + printf("pc offset 0x%zx\n", pc - image->reloc_base); + if (image->file_path) + printf("%pD\n", image->file_path); + return EFI_SUCCESS; +} + +/* + * Print information about all loaded images. + * + * @pc: program counter (use NULL to suppress offset output) + */ +void efi_print_image_infos(void *pc) +{ + struct efi_object *efiobj; + struct efi_handler *handler; + + list_for_each_entry(efiobj, &efi_obj_list, link) { + list_for_each_entry(handler, &efiobj->protocols, link) { + if (!guidcmp(handler->guid, &efi_guid_loaded_image)) { + printf("\n"); + efi_print_image_info( + handler->protocol_interface, pc); + } + } + } +} + static efi_status_t efi_loader_relocate(const IMAGE_BASE_RELOCATION *rel, unsigned long rel_size, void *efi_reloc) {
On 03.04.18 22:29, Heinrich Schuchardt wrote:
Introduce functions to print information about loaded images.
If we want to analyze an exception in an EFI image we need the offset between the PC and the start of the loaded image.
With efi_print_image_info() we can print the necessary information for a single image, e.g.
UEFI image start 0x7fdb4000, size 0xa7b60 pc offset 0x72ca /\snp.efi
efi_print_image_infos() provides output for all loaded images.
Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de
Sorry, I didn't see the new version earlier. The same comment applies though. Please condense the output down to a single line.
Alex
If an exception occurs in a UEFI loaded image we need the start address of the image to determine the relocation offset.
This patch adds the necessary lines after the registers in the crash dump. A possible output would be:
UEFI image start 0x7fdb4000, size 0xa7b60 pc offset 0x72ca /\snp.efi
With the offset 0x72ca we can now find the relevant instruction in the disassembled 'snp.efi' binary.
Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de --- v2 no change --- arch/arm/lib/interrupts.c | 17 +++++++++++++++++ 1 file changed, 17 insertions(+)
diff --git a/arch/arm/lib/interrupts.c b/arch/arm/lib/interrupts.c index 80869adb61..a28fef8f46 100644 --- a/arch/arm/lib/interrupts.c +++ b/arch/arm/lib/interrupts.c @@ -20,6 +20,7 @@ */
#include <common.h> +#include <efi_loader.h> #include <asm/proc-armv/ptrace.h> #include <asm/u-boot-arm.h> #include <efi_loader.h> @@ -51,6 +52,14 @@ void bad_mode (void) reset_cpu (0); }
+static void show_efi_loaded_images(struct pt_regs *regs) +{ +#if defined(CONFIG_EFI_LOADER) && \ + !defined(CONFIG_SPL_BUILD) && !defined(API_BUILD) + efi_print_image_infos((void *)instruction_pointer(regs)); +#endif +} + void show_regs (struct pt_regs *regs) { unsigned long __maybe_unused flags; @@ -77,6 +86,7 @@ void show_regs (struct pt_regs *regs) printf("sp : %08lx ip : %08lx fp : %08lx\n", regs->ARM_sp, regs->ARM_ip, regs->ARM_fp); printf ("r10: %08lx r9 : %08lx r8 : %08lx\n", + regs->ARM_r10, regs->ARM_r9, regs->ARM_r8); printf ("r7 : %08lx r6 : %08lx r5 : %08lx r4 : %08lx\n", regs->ARM_r7, regs->ARM_r6, regs->ARM_r5, regs->ARM_r4); @@ -106,6 +116,7 @@ void do_undefined_instruction (struct pt_regs *pt_regs) printf ("undefined instruction\n"); fixup_pc(pt_regs, -4); show_regs (pt_regs); + show_efi_loaded_images(pt_regs); bad_mode (); }
@@ -115,6 +126,7 @@ void do_software_interrupt (struct pt_regs *pt_regs) printf ("software interrupt\n"); fixup_pc(pt_regs, -4); show_regs (pt_regs); + show_efi_loaded_images(pt_regs); bad_mode (); }
@@ -124,6 +136,7 @@ void do_prefetch_abort (struct pt_regs *pt_regs) printf ("prefetch abort\n"); fixup_pc(pt_regs, -8); show_regs (pt_regs); + show_efi_loaded_images(pt_regs); bad_mode (); }
@@ -133,6 +146,7 @@ void do_data_abort (struct pt_regs *pt_regs) printf ("data abort\n"); fixup_pc(pt_regs, -8); show_regs (pt_regs); + show_efi_loaded_images(pt_regs); bad_mode (); }
@@ -142,6 +156,7 @@ void do_not_used (struct pt_regs *pt_regs) printf ("not used\n"); fixup_pc(pt_regs, -8); show_regs (pt_regs); + show_efi_loaded_images(pt_regs); bad_mode (); }
@@ -151,6 +166,7 @@ void do_fiq (struct pt_regs *pt_regs) printf ("fast interrupt request\n"); fixup_pc(pt_regs, -8); show_regs (pt_regs); + show_efi_loaded_images(pt_regs); bad_mode (); }
@@ -160,5 +176,6 @@ void do_irq (struct pt_regs *pt_regs) printf ("interrupt request\n"); fixup_pc(pt_regs, -8); show_regs (pt_regs); + show_efi_loaded_images(pt_regs); bad_mode (); }
participants (2)
-
Alexander Graf -
Heinrich Schuchardt