Re: [PATCH v2 2/2] efi: Put Linux specific magic number in the DOS header

[Daniel Kiper <daniel.kiper@oracle.com>]

On Tue, Nov 29, 2022 at 06:56:16PM +0100, Ard Biesheuvel wrote:
> GRUB currently relies on the magic number in the image header of ARM and
> arm64 EFI kernel images to decide whether or not the image in question
> is a bootable kernel.
>
> However, the purpose of the magic number is to identify the image as one
> that implements the bare metal boot protocol, and so GRUB, which only
> does EFI boot, can only boot images that could potentially be booted in
> a non-EFI manner as well.
>
> This is problematic for the new zboot decompressor image format, as it
> can only boot in EFI mode, and must therefore not use the bare metal
> boot magic number in its header.
>
> For this reason, the strict magic number was dropped from GRUB, to
> permit essentially any kind of EFI executable to be booted via the
> 'linux' command, blurring the line between the linux loader and the
> chainloader.
>
> So let's use the same field in the DOS header that RISC-V and arm64
> already use for their 'bare metal' magic numbers to store a 'generic
> Linux kernel' magic number, which can be used to identify bootable
> kernel images in PE format which don't necessarily implement a bare
> metal boot protocol in the same binary. Note that, in the context of
> EFI, the MSDOS header is only described in terms of the fields that it

s/MSDOS/MS-DOS/ to be consistent with other places in the patch...

> shares with the hybrid PE/COFF image format, (i.e., the magic number at
> offset #0 and the PE header offset at byte offset #0x3c). Since we aim

s/the magic number at offset #0/MS-DOS EXE magic number at offset #0/?

"the magic number at offset #0" itself is confusing in the context of
number of "magic number" phrases in the patch... :-)

> for compatibility with EFI only, and not with MS-DOS or MS-Windows, we
> can use the remaining space in the MS-DOS header however we want.
>
> Let's set the generic magic number for x86 images as well: existing
> bootloaders already have their own methods to identify x86 Linux images
> that can be booted in a non-EFI manner, and having the magic number in
> place there will ease any future transitions in loader implementations
> to merge the x86 and non-x86 EFI boot paths.
>
> Note that 32-bit ARM already uses the same location in the header for a
> different purpose, but the ARM support is already widely implemented and
> the EFI zboot decompressor is not available on ARM anyway, so we just
> disregard it here.
>
> Cc: Huacai Chen <chenhuacai@kernel.org>
> Cc: Atish Patra <atishp@rivosinc.com>
> Cc: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
> Cc: Daniel Kiper <daniel.kiper@oracle.com>
> Cc: Leif Lindholm <quic_llindhol@quicinc.com>
> Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
> ---
>  arch/loongarch/kernel/head.S                | 3 ++-
>  arch/x86/boot/header.S                      | 3 ++-
>  drivers/firmware/efi/libstub/zboot-header.S | 3 ++-
>  include/linux/pe.h                          | 7 +++++++
>  4 files changed, 13 insertions(+), 3 deletions(-)
>
> diff --git a/arch/loongarch/kernel/head.S b/arch/loongarch/kernel/head.S
> index 84970e2666588963..caa74439700eee93 100644
> --- a/arch/loongarch/kernel/head.S
> +++ b/arch/loongarch/kernel/head.S
> @@ -25,7 +25,8 @@ _head:
>  	.dword	kernel_entry		/* Kernel entry point */
>  	.dword	_end - _text		/* Kernel image effective size */
>  	.quad	0			/* Kernel image load offset from start of RAM */
> -	.org	0x3c			/* 0x20 ~ 0x3b reserved */
> +	.org	0x38			/* 0x20 ~ 0x38 reserved */
> +	.long	LINUX_PE_MAGIC
>  	.long	pe_header - _head	/* Offset to the PE header */
>
>  pe_header:
> diff --git a/arch/x86/boot/header.S b/arch/x86/boot/header.S
> index f912d777013052ea..be8f78a7ee325475 100644
> --- a/arch/x86/boot/header.S
> +++ b/arch/x86/boot/header.S
> @@ -80,10 +80,11 @@ bs_die:
>  	ljmp	$0xf000,$0xfff0
>
>  #ifdef CONFIG_EFI_STUB
> -	.org	0x3c
> +	.org	0x38
>  	#
>  	# Offset to the PE header.
>  	#
> +	.long	LINUX_PE_MAGIC
>  	.long	pe_header
>  #endif /* CONFIG_EFI_STUB */
>
> diff --git a/drivers/firmware/efi/libstub/zboot-header.S b/drivers/firmware/efi/libstub/zboot-header.S
> index bc2d7750d7f14174..ec4525d40e0cf6d6 100644
> --- a/drivers/firmware/efi/libstub/zboot-header.S
> +++ b/drivers/firmware/efi/libstub/zboot-header.S
> @@ -20,7 +20,8 @@ __efistub_efi_zboot_header:
>  	.long		__efistub__gzdata_size - 12		// payload size
>  	.long		0, 0					// reserved
>  	.asciz		COMP_TYPE				// compression type
> -	.org		.Ldoshdr + 0x3c
> +	.org		.Ldoshdr + 0x38
> +	.long		LINUX_PE_MAGIC
>  	.long		.Lpehdr - .Ldoshdr			// PE header offset
>
>  .Lpehdr:
> diff --git a/include/linux/pe.h b/include/linux/pe.h
> index 056a1762de904fc1..1db4c944efd78f51 100644
> --- a/include/linux/pe.h
> +++ b/include/linux/pe.h
> @@ -31,6 +31,13 @@
>  #define LINUX_EFISTUB_MAJOR_VERSION		0x1
>  #define LINUX_EFISTUB_MINOR_VERSION		0x1
>
> +/*
> + * LINUX_PE_MAGIC appears at offset 0x38 into the MSDOS header of EFI bootable

s/MSDOS/MS-DOS/

> + * Linux kernel images that target the architecture as specified by the PE/COFF
> + * header machine type field.
> + */
> +#define LINUX_PE_MAGIC	0x818223cd
> +
>  #define MZ_MAGIC	0x5a4d	/* "MZ" */
>
>  #define PE_MAGIC		0x00004550	/* "PE\0\0" */

If you take into account these minor things mentioned above you can add
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com> to your both patches.

Atish, could you respin your GRUB "Unify ARM64 & RISC-V Linux Loader"
patch set when both Ard's patches are in at least "efi" Linux kernel
branch? Hmmm... I am not sure it will go through "efi" branch. Ard?

Daniel