Re: [PATCH v2 3/8] efi: move MS-DOS stub out of generic PE header definition

[Heinrich Schuchardt]

On 10/25/20 2:49 PM, Ard Biesheuvel wrote:
> The PE/COFF spec permits the COFF signature and file header to appear
> anywhere in the file, and the actual offset is recorded in 4 byte
> little endian field at offset 0x3c of the image.
>
> When GRUB is emitted as a PE/COFF binary, we reuse the 128 byte MS-DOS
> stub (even for non-x86 architectures), putting the COFF signature and
> file header at offset 0x80. However, other PE/COFF images may use
> different values, and non-x86 Linux kernels use an offset of 0x40
> instead.
>
> So let's get rid of the grub_pe32_header struct from pe32.h, given that
> it does not represent anything defined by the PE/COFF spec. Instead,
> use the GRUB_PE32_MSDOS_STUB_SIZE macro explicitly to reference the
> COFF header in the only place in the code where we rely on this.
>
> The remaining fields are moved into a struct grub_coff_image_header,
> which we will use later to access COFF header fields of arbitrary
> images (and which may therefore appear at different offsets)
>
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@arm.com>
> ---
>   grub-core/kern/efi/efi.c | 5 +++--
>   include/grub/efi/pe32.h  | 5 +----
>   2 files changed, 4 insertions(+), 6 deletions(-)
>
> diff --git a/grub-core/kern/efi/efi.c b/grub-core/kern/efi/efi.c
> index e0165e74c587..9e5a72be538d 100644
> --- a/grub-core/kern/efi/efi.c
> +++ b/grub-core/kern/efi/efi.c
> @@ -282,7 +282,7 @@ grub_addr_t
>   grub_efi_modules_addr (void)
>   {
>     grub_efi_loaded_image_t *image;
> -  struct grub_pe32_header *header;
> +  struct grub_coff_image_header *header;
>     struct grub_pe32_coff_header *coff_header;
>     struct grub_pe32_section_table *sections;
>     struct grub_pe32_section_table *section;
> @@ -293,7 +293,8 @@ grub_efi_modules_addr (void)
>     if (! image)
>       return 0;
>
> -  header = image->image_base;
> +  header = (struct grub_coff_image_header *) ((char *) image->image_base
> +                                             + GRUB_PE32_MSDOS_STUB_SIZE);

After checking that the file starts with the letters 'MZ' we can find at
file offset 0x3c the position of the 'PE\0\0' indicating the start of
the COFF header. This avoids relying upon any fixed offset and conforms
to the PE Format specification.

See
https://docs.microsoft.com/en-us/windows/win32/debug/pe-format#ms-dos-stub-image-only

Best regards

Heinrich

>     coff_header = &(header->coff_header);
>     sections
>       = (struct grub_pe32_section_table *) ((char *) coff_header
> diff --git a/include/grub/efi/pe32.h b/include/grub/efi/pe32.h
> index 0ed8781f0376..a2da4b318c85 100644
> --- a/include/grub/efi/pe32.h
> +++ b/include/grub/efi/pe32.h
> @@ -254,11 +254,8 @@ struct grub_pe32_section_table
>
>   #define GRUB_PE32_SIGNATURE_SIZE 4
>
> -struct grub_pe32_header
> +struct grub_coff_image_header
>   {
> -  /* This should be filled in with GRUB_PE32_MSDOS_STUB.  */
> -  grub_uint8_t msdos_stub[GRUB_PE32_MSDOS_STUB_SIZE];
> -
>     /* This is always PE\0\0.  */
>     char signature[GRUB_PE32_SIGNATURE_SIZE];