On 4/9/21 8:10 AM, Ard Biesheuvel wrote:
On Thu, 8 Apr 2021 at 20:45, Heinrich Schuchardt <xypron.glpk@xxxxxx> wrote:
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@xxxxxxx>
---
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.
I think you might be missing the point here. This is not about
arbitrary PE/COFF images that GRUB loads, this is about the PE/COFF
image that is constructed by the GRUB build system to carry GRUB
itself: what would be the point of extracting the offset from the
image if we always put the PE header at the same offset?
We should not assume that a file /boot/grub/x86_64-efi/core.efi contains
a GRUB EFI binary. We should check the header. If we can't find 'MZ' at
offset 0 and 'PE\0\0' at the position indicated by the offset at 0x3c,
it is definitively not a valid EFI binary.
I would prefer one common function for checking EFI headers instead of
grub_arch_efi_linux_check_image() per architecture.
Best regards
Heinrich
In any case, I don't care deeply one way or the other, so feel free to
propose an alternative to this patch.
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];
