efi_loader: Check machine type in the image header
PATCH 1/1 efi_loader: Check machine type in the image header
Check FileHeader.Machine to make sure the EFI executable image is built for the same architecture. After this change, 32-bit U-Boot on x86 will print an error message instead of loading an x86_64 image and crashing.
--- include/pe.h | 1 + lib/efi_loader/efi_image_loader.c | 13 +++++++++++++ 2 files changed, 14 insertions(+)
include/pe.h +1 -0
@@ -38,6 +38,7 @@ typedef struct _IMAGE_DOS_HEADER {#define IMAGE_DOS_SIGNATURE 0x5A4D/* MZ */ #define IMAGE_NT_SIGNATURE 0x00004550/* PE00 */ +#define IMAGE_FILE_MACHINE_INTEL386 0x014c#define IMAGE_FILE_MACHINE_ARM 0x01c0#define IMAGE_FILE_MACHINE_THUMB 0x01c2#define IMAGE_FILE_MACHINE_ARMNT 0x01c4
lib/efi_loader/efi_image_loader.c +13 -0
@@ -126,15 +126,23 @@ void *efi_load_pe(void *efi, struct efi_loaded_image *loaded_image_info)void *entry;uint64_t image_size;unsigned long virt_size =0 ; +int machine_type =0 ;bool can_run_nt64 = true;bool can_run_nt32 = true;#if defined(CONFIG_ARM64 ) + machine_type =IMAGE_FILE_MACHINE_ARM64 ; can_run_nt32 = false;#elif defined(CONFIG_ARM ) can_run_nt64 = false;#endif +#if defined(CONFIG_X86_64 ) + machine_type =IMAGE_FILE_MACHINE_AMD64 ; +#elif defined(CONFIG_X86 ) + machine_type =IMAGE_FILE_MACHINE_INTEL386 ; +#endif + dos = efi;if (dos->e_magic !=IMAGE_DOS_SIGNATURE ) {printf ("%s: Invalid DOS Signature\n" , __func__);
@@ -147,6 +155,11 @@ void *efi_load_pe(void *efi, struct efi_loaded_image *loaded_image_info)return NULL ; } +if (machine_type && nt->FileHeader.Machine != machine_type) { +printf ("%s: Incorrect machine type\n" , __func__); +return NULL ; + } +/* Calculate upper virtual address boundary */ num_sections = nt->FileHeader.NumberOfSections; sections = (void *)&nt->OptionalHeader +
Signature · 1 line
2.7.4
PATCH 1/1 efi_loader: Check machine type in the image header
Quoted 15 lines · Ivan Gorinov
Check FileHeader.Machine to make sure the EFI executable image is built for the same architecture. After this change, 32-bit U-Boot on x86 will print an error message instead of loading an
x86_64image and crashing.Signed-off-by Ivan Gorinov <[email protected]>--- include/pe.h | 1 + lib/efi_loader/efi_image_loader.c | 13 +++++++++++++ 2 files changed, 14 insertions(+)include/pe.h +0
-0
include/pe.h typedef struct _IMAGE_DOS_HEADER { · line 41
Quoted 4 lines
include/pe.h +1
-0@@ -38,6 +38,7 @@ typedef struct _IMAGE_DOS_HEADER {#define IMAGE_DOS_SIGNATURE 0x5A4D/* MZ */ #define IMAGE_NT_SIGNATURE 0x00004550/* PE00 */ +#define IMAGE_FILE_MACHINE_INTEL386 0x014c
Please, use the names from the "Microsoft Portable Executable and Common Object File Format Specification":
IMAGE_FILE_MACHINE_I386 0x14c
// Intel 386 or later processors and compatible processors
include/pe.h typedef struct _IMAGE_DOS_HEADER { · line 44
Quoted 3 lines
include/pe.h +0
-0#define IMAGE_FILE_MACHINE_ARM 0x01c0#define IMAGE_FILE_MACHINE_THUMB 0x01c2#define IMAGE_FILE_MACHINE_ARMNT 0x01c4
Please, also add
IMAGE_FILE_MACHINE_RISCV32 0x5032 RISC-V 32-bit address spaceIMAGE_FILE_MACHINE_RISCV64 0x5064 RISC-V 64-bit address space
lib/efi_loader/efi_image_loader.c void *efi_load_pe(void *efi, struct efi_loaded_image *loaded_image_info) · line 129
Quoted 9 lines
lib/efi_loader/efi_image_loader.c +1
-0@@ -126,15 +126,23 @@ void *efi_load_pe(void *efi, struct efi_loaded_image *loaded_image_info)void *entry;uint64_t image_size;unsigned long virt_size =0 ; +int machine_type =0 ;
Why do you need a variable for the check?
Isn't this more concise:
switch (nt->FileHeader.Machine) {
#ifdef CONFIG_ARM64
case IMAGE_FILE_MACHINE_ARM64:
break;
#elif defined(CONFIG_ARM)
case IMAGE_FILE_MACHINE_THUMB:
break;
#elif defined(CONGIG_X86_64)
case IMAGE_FILE_MACHINE_AMD64:
break;
#elif defined(CONFIG_X86)
case IMAGE_FILE_MACHINE_I386:
break;
#elif defined(CONFIG_CPU_RISCV_64)
case IMAGE_FILE_MACHINE_RISCV64:
break;
#elif defined(CONFIG_CPU_RISCV_32)
case IMAGE_FILE_MACHINE_RISCV32:
break;
#endif
default:
printf("%s: Image type 0x%04x not supported\n", \
__func__, nt->FileHeader.Machine);
return NULL;
}lib/efi_loader/efi_image_loader.c void *efi_load_pe(void *efi, struct efi_loaded_image *loaded_image_info) · line 138
Quoted 9 lines
lib/efi_loader/efi_image_loader.c +1
-0bool can_run_nt64 = true;bool can_run_nt32 = true;#if defined(CONFIG_ARM64 ) + machine_type =IMAGE_FILE_MACHINE_ARM64 ; can_run_nt32 = false;#elif defined(CONFIG_ARM ) can_run_nt64 = false;#endif
If we correctly check the machine type, can't we eliminate can_run_nt64 and can_run_nt32? If you would leave the variables you would have to set them for all architectures.
Below we check the optional header type against the bitness. The PE spec does not forbid to use PE32+ for 32 bit images.
Best regards
Heinrich
Quoted 23 lines
lib/efi_loader/efi_image_loader.c +11
-0+#if defined(CONFIG_X86_64 ) + machine_type =IMAGE_FILE_MACHINE_AMD64 ; +#elif defined(CONFIG_X86 ) + machine_type =IMAGE_FILE_MACHINE_INTEL386 ; +#endif + dos = efi;if (dos->e_magic !=IMAGE_DOS_SIGNATURE ) {printf ("%s: Invalid DOS Signature\n" , __func__);@@ -147,6 +155,11 @@ void *efi_load_pe(void *efi, struct efi_loaded_image *loaded_image_info)return NULL ; } +if (machine_type && nt->FileHeader.Machine != machine_type) { +printf ("%s: Incorrect machine type\n" , __func__); +return NULL ; + } +/* Calculate upper virtual address boundary */ num_sections = nt->FileHeader.NumberOfSections; sections = (void *)&nt->OptionalHeader +