| Message ID | 20201025134941.4805-4-ard.biesheuvel@arm.com |
|---|---|
| State | New |
| Headers | show |
| Series | linux: implement LoadFile2 initrd loading | expand |
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]; > >
On Thu, 8 Apr 2021 at 20:45, Heinrich Schuchardt <xypron.glpk@gmx.de> 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@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. > 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? 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]; > > > > >
On 4/9/21 8:10 AM, Ard Biesheuvel wrote: > On Thu, 8 Apr 2021 at 20:45, Heinrich Schuchardt <xypron.glpk@gmx.de> 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@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. >> > > 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]; >>> >>> >>
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); 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];
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(-)