All of lore.kernel.org
 help / color / mirror / Atom feed
* [U-Boot] [PATCH v2] efi_loader: Check machine type in the image header
@ 2018-04-05 21:28 Ivan Gorinov
  2018-04-05 22:43 ` Alexander Graf
  0 siblings, 1 reply; 6+ messages in thread
From: Ivan Gorinov @ 2018-04-05 21:28 UTC (permalink / raw)
  To: u-boot

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@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
+#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
+
+	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);
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 6+ messages in thread
* [U-Boot] [PATCH v2] efi_loader: Check machine type in the image header
  2018-04-05 21:28 [U-Boot] [PATCH v2] efi_loader: Check machine type in the image header Ivan Gorinov
@ 2018-04-05 22:43 ` Alexander Graf
  2018-04-05 23:03   ` Heinrich Schuchardt
  0 siblings, 1 reply; 6+ messages in thread
From: Alexander Graf @ 2018-04-05 22:43 UTC (permalink / raw)
  To: u-boot



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

^ permalink raw reply	[flat|nested] 6+ messages in thread
* [U-Boot] [PATCH v2] efi_loader: Check machine type in the image header
  2018-04-05 22:43 ` Alexander Graf
@ 2018-04-05 23:03   ` Heinrich Schuchardt
  2018-04-05 23:23     ` Alexander Graf
  0 siblings, 1 reply; 6+ messages in thread
From: Heinrich Schuchardt @ 2018-04-05 23:03 UTC (permalink / raw)
  To: u-boot

On 04/06/2018 12:43 AM, Alexander Graf wrote:
> 
> 
> 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@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?

The value 0x01c2 means ARM or THUMB
It is used by Linux, GRUB, iPXE.

Regards

Heinrich

> 
>> +#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);
>>
> 

^ permalink raw reply	[flat|nested] 6+ messages in thread
* [U-Boot] [PATCH v2] efi_loader: Check machine type in the image header
  2018-04-05 23:03   ` Heinrich Schuchardt
@ 2018-04-05 23:23     ` Alexander Graf
  2018-04-06  6:47       ` Heinrich Schuchardt
  0 siblings, 1 reply; 6+ messages in thread
From: Alexander Graf @ 2018-04-05 23:23 UTC (permalink / raw)
  To: u-boot



On 06.04.18 01:03, Heinrich Schuchardt wrote:
> On 04/06/2018 12:43 AM, Alexander Graf wrote:
>>
>>
>> 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@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?
> 
> The value 0x01c2 means ARM or THUMB
> It is used by Linux, GRUB, iPXE.

I'm not sure that's fully accurate. ARM means some old legacy one, but
ARMNT and THUMB can both be used, no?

  https://www.npmjs.com/package/binarycpu

What I'm trying to say is that a 1:1 matching might not be the only
thing we want.


Alex

^ permalink raw reply	[flat|nested] 6+ messages in thread
* [U-Boot] [PATCH v2] efi_loader: Check machine type in the image header
  2018-04-05 23:23     ` Alexander Graf
@ 2018-04-06  6:47       ` Heinrich Schuchardt
  2018-04-06  7:25         ` Alexander Graf
  0 siblings, 1 reply; 6+ messages in thread
From: Heinrich Schuchardt @ 2018-04-06  6:47 UTC (permalink / raw)
  To: u-boot

On 04/06/2018 01:23 AM, Alexander Graf wrote:
> 
> 
> On 06.04.18 01:03, Heinrich Schuchardt wrote:
>> On 04/06/2018 12:43 AM, Alexander Graf wrote:
>>>
>>>
>>> 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@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?
>>
>> The value 0x01c2 means ARM or THUMB
>> It is used by Linux, GRUB, iPXE.
> 
> I'm not sure that's fully accurate. ARM means some old legacy one, but
> ARMNT and THUMB can both be used, no?
> 
>   https://www.npmjs.com/package/binarycpu
> 
> What I'm trying to say is that a 1:1 matching might not be the only
> thing we want.

Revision 8.3 – February 6th, 2013 of the PECOFF spec is the most explicit:

IMAGE_FILE_MACHINE_ARMNT 0x1c4 ARMv7 (or higher) Thumb mode only
IMAGE_FILE_MACHINE_THUMB 0x1c2 ARM or Thumb (“interworking”)

EDK2 has only these values (with some names not matching the standard):

IMAGE_FILE_MACHINE_I386            0x014c
IMAGE_FILE_MACHINE_IA64            0x0200
IMAGE_FILE_MACHINE_EBC             0x0EBC
IMAGE_FILE_MACHINE_X64             0x8664
IMAGE_FILE_MACHINE_ARMTHUMB_MIXED  0x01c2
IMAGE_FILE_MACHINE_ARM64           0xAA64

On ARM only IMAGE_FILE_MACHINE_ARMTHUMB_MIXED and
IMAGE_FILE_MACHINE_ARMTHUMB_MIXED are IMAGE_FILE_MACHINE_EBC are
supported. But we do not support IMAGE_FILE_MACHINE_EBC.

So EDK2 would not be able load a 0x01c4 ARM binary.

Best regards

Heinrich

^ permalink raw reply	[flat|nested] 6+ messages in thread
* [U-Boot] [PATCH v2] efi_loader: Check machine type in the image header
  2018-04-06  6:47       ` Heinrich Schuchardt
@ 2018-04-06  7:25         ` Alexander Graf
  0 siblings, 0 replies; 6+ messages in thread
From: Alexander Graf @ 2018-04-06  7:25 UTC (permalink / raw)
  To: u-boot



On 06.04.18 08:47, Heinrich Schuchardt wrote:
> On 04/06/2018 01:23 AM, Alexander Graf wrote:
>>
>>
>> On 06.04.18 01:03, Heinrich Schuchardt wrote:
>>> On 04/06/2018 12:43 AM, Alexander Graf wrote:
>>>>
>>>>
>>>> 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@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?
>>>
>>> The value 0x01c2 means ARM or THUMB
>>> It is used by Linux, GRUB, iPXE.
>>
>> I'm not sure that's fully accurate. ARM means some old legacy one, but
>> ARMNT and THUMB can both be used, no?
>>
>>   https://www.npmjs.com/package/binarycpu
>>
>> What I'm trying to say is that a 1:1 matching might not be the only
>> thing we want.
> 
> Revision 8.3 – February 6th, 2013 of the PECOFF spec is the most explicit:
> 
> IMAGE_FILE_MACHINE_ARMNT 0x1c4 ARMv7 (or higher) Thumb mode only
> IMAGE_FILE_MACHINE_THUMB 0x1c2 ARM or Thumb (“interworking”)
> 
> EDK2 has only these values (with some names not matching the standard):
> 
> IMAGE_FILE_MACHINE_I386            0x014c
> IMAGE_FILE_MACHINE_IA64            0x0200
> IMAGE_FILE_MACHINE_EBC             0x0EBC
> IMAGE_FILE_MACHINE_X64             0x8664
> IMAGE_FILE_MACHINE_ARMTHUMB_MIXED  0x01c2
> IMAGE_FILE_MACHINE_ARM64           0xAA64
> 
> On ARM only IMAGE_FILE_MACHINE_ARMTHUMB_MIXED and
> IMAGE_FILE_MACHINE_ARMTHUMB_MIXED are IMAGE_FILE_MACHINE_EBC are
> supported. But we do not support IMAGE_FILE_MACHINE_EBC.
> 
> So EDK2 would not be able load a 0x01c4 ARM binary.

Ok, so let's leave it at one target for now. If later on someone wants
to enable thumb only mode for example, they'll have to come up with a
way to identify it more smartly.


Alex

^ permalink raw reply	[flat|nested] 6+ messages in thread
end of thread, other threads:[~2018-04-06  7:25 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-04-05 21:28 [U-Boot] [PATCH v2] efi_loader: Check machine type in the image header Ivan Gorinov
2018-04-05 22:43 ` Alexander Graf
2018-04-05 23:03   ` Heinrich Schuchardt
2018-04-05 23:23     ` Alexander Graf
2018-04-06  6:47       ` Heinrich Schuchardt
2018-04-06  7:25         ` Alexander Graf

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.