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

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

[Heinrich Schuchardt]

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(-)

-- 
2.16.3

[U-Boot] [PATCH v2 1/6] efi_loader: use efi_uintn_t for LoadImage

[Heinrich Schuchardt]

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) {
-- 
2.16.3

[U-Boot] [PATCH v2 2/6] efi_loader: save image relocation address and size

[Heinrich Schuchardt]

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;
 }
-- 
2.16.3

[U-Boot] [PATCH v2 3/6] efi_loader: ImageSize must be multiple of SectionAlignment

[Heinrich Schuchardt]

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);
-- 
2.16.3

[U-Boot] [PATCH v2 4/6] efi_loader: correct types for EFI_LOADED_IMAGE_PROTOCOL

[Heinrich Schuchardt]

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;
-- 
2.16.3

[U-Boot] [PATCH v2 5/6] efi_loader: new functions to print loaded image information

[Heinrich Schuchardt]

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)
 {
-- 
2.16.3

[U-Boot] [PATCH v2 6/6] arm: print information about loaded UEFI images

[Heinrich Schuchardt]

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 ();
 }
-- 
2.16.3

[U-Boot] [PATCH v2 5/6] efi_loader: new functions to print loaded image information

[Alexander Graf]

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