[U-Boot] [PATCH v2] efi_loader: Check machine type in the image header

[Alexander Graf]

On 05.04.18 23:28, Ivan Gorinov wrote:
> Check FileHeader.Machine to make sure the EFI executable image is built
> for the same architecture. For example, 32-bit U-Boot on x86 will print
> an error message instead of loading an x86_64 image and crashing.
> 
> Signed-off-by: Ivan Gorinov <ivan.gorinov at intel.com>
> ---
>  include/pe.h                      | 24 ++++++++++++++++++++++++
>  lib/efi_loader/efi_image_loader.c | 24 ++++++++++++------------
>  2 files changed, 36 insertions(+), 12 deletions(-)
> 
> diff --git a/include/pe.h b/include/pe.h
> index c3a19ce..0dc33f0 100644
> --- a/include/pe.h
> +++ b/include/pe.h
> @@ -38,11 +38,35 @@ typedef struct _IMAGE_DOS_HEADER {
>  #define IMAGE_DOS_SIGNATURE		0x5A4D     /* MZ   */
>  #define IMAGE_NT_SIGNATURE		0x00004550 /* PE00 */
>  
> +#define IMAGE_FILE_MACHINE_I386		0x014c
>  #define IMAGE_FILE_MACHINE_ARM		0x01c0
>  #define IMAGE_FILE_MACHINE_THUMB	0x01c2
>  #define IMAGE_FILE_MACHINE_ARMNT	0x01c4
>  #define IMAGE_FILE_MACHINE_AMD64	0x8664
>  #define IMAGE_FILE_MACHINE_ARM64	0xaa64
> +#define IMAGE_FILE_MACHINE_RISCV32	0x5032
> +#define IMAGE_FILE_MACHINE_RISCV64	0x5064
> +
> +#if defined(CONFIG_ARM64)
> +#define TARGET_PE_MACHINE_TYPE IMAGE_FILE_MACHINE_ARM64
> +#elif defined(CONFIG_ARM)
> +#define TARGET_PE_MACHINE_TYPE IMAGE_FILE_MACHINE_THUMB

Are you sure we always have thumb as machine type here? Aren't we
compatible with either ARM or THUMB?

> +#endif
> +
> +#if defined(CONFIG_X86_64)
> +#define TARGET_PE_MACHINE_TYPE IMAGE_FILE_MACHINE_AMD64
> +#elif defined(CONFIG_X86)
> +#define TARGET_PE_MACHINE_TYPE IMAGE_FILE_MACHINE_I386
> +#endif
> +
> +#if defined(CONFIG_CPU_RISCV_32)
> +#define TARGET_PE_MACHINE_TYPE IMAGE_FILE_MACHINE_RISCV32
> +#endif
> +
> +#if defined(CONFIG_CPU_RISCV_64)
> +#define TARGET_PE_MACHINE_TYPE IMAGE_FILE_MACHINE_RISCV64
> +#endif
> +
>  #define IMAGE_NT_OPTIONAL_HDR32_MAGIC	0x10b
>  #define IMAGE_NT_OPTIONAL_HDR64_MAGIC	0x20b
>  #define IMAGE_SUBSYSTEM_EFI_APPLICATION	10
> diff --git a/lib/efi_loader/efi_image_loader.c b/lib/efi_loader/efi_image_loader.c
> index f588576..ac20488 100644
> --- a/lib/efi_loader/efi_image_loader.c
> +++ b/lib/efi_loader/efi_image_loader.c
> @@ -172,14 +172,6 @@ void *efi_load_pe(void *efi, struct efi_loaded_image *loaded_image_info)
>  	void *entry;
>  	uint64_t image_size;
>  	unsigned long virt_size = 0;
> -	bool can_run_nt64 = true;
> -	bool can_run_nt32 = true;
> -
> -#if defined(CONFIG_ARM64)
> -	can_run_nt32 = false;
> -#elif defined(CONFIG_ARM)
> -	can_run_nt64 = false;
> -#endif
>  
>  	dos = efi;
>  	if (dos->e_magic != IMAGE_DOS_SIGNATURE) {
> @@ -193,6 +185,16 @@ void *efi_load_pe(void *efi, struct efi_loaded_image *loaded_image_info)
>  		return NULL;
>  	}
>  
> +#ifdef TARGET_PE_MACHINE_TYPE

I don't think we should have the #ifdef here. Let' make sure we always
know which platform we want to run on.


Alex

> +
> +	if (nt->FileHeader.Machine != TARGET_PE_MACHINE_TYPE) {
> +		printf("%s: Machine type 0x%04x is not supported\n",
> +		       __func__, nt->FileHeader.Machine);
> +		return NULL;
> +	}
> +
> +#endif
> +
>  	/* Calculate upper virtual address boundary */
>  	num_sections = nt->FileHeader.NumberOfSections;
>  	sections = (void *)&nt->OptionalHeader +
> @@ -205,8 +207,7 @@ void *efi_load_pe(void *efi, struct efi_loaded_image *loaded_image_info)
>  	}
>  
>  	/* Read 32/64bit specific header bits */
> -	if (can_run_nt64 &&
> -	    (nt->OptionalHeader.Magic == IMAGE_NT_OPTIONAL_HDR64_MAGIC)) {
> +	if (nt->OptionalHeader.Magic == IMAGE_NT_OPTIONAL_HDR64_MAGIC) {
>  		IMAGE_NT_HEADERS64 *nt64 = (void *)nt;
>  		IMAGE_OPTIONAL_HEADER64 *opt = &nt64->OptionalHeader;
>  		image_size = opt->SizeOfImage;
> @@ -222,8 +223,7 @@ void *efi_load_pe(void *efi, struct efi_loaded_image *loaded_image_info)
>  		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)) {
> +	} else if (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);
>