On Thu, 8 Apr 2021 at 20:57, Heinrich Schuchardt <xypron.glpk@xxxxxx> wrote:
>
> On 10/25/20 2:49 PM, Ard Biesheuvel wrote:
> > The way we load the Linux and PE/COFF image headers depends on a fixed
> > placement of the COFF header at offset 0x40 into the file. This is a
> > reasonable default, given that this is where Linux emits it today.
> > However, in order to comply with the PE/COFF spec, which permits this
> > header to appear anywhere in the file, let's ensure that we read the
> > header from where it actually appears in the file if it is not located
> > at offset 0x40.
> >
> > Signed-off-by: Ard Biesheuvel <ard.biesheuvel@xxxxxxx>
> > ---
> > grub-core/loader/arm64/linux.c | 15 +++++++++++++++
> > 1 file changed, 15 insertions(+)
> >
> > diff --git a/grub-core/loader/arm64/linux.c b/grub-core/loader/arm64/linux.c
> > index 915b6ad7292d..28ff8584a3b5 100644
> > --- a/grub-core/loader/arm64/linux.c
> > +++ b/grub-core/loader/arm64/linux.c
> > @@ -66,6 +66,21 @@ grub_arch_efi_linux_load_image_header (grub_file_t file,
> > grub_dprintf ("linux", "UEFI stub kernel:\n");
> > grub_dprintf ("linux", "PE/COFF header @ %08x\n", lh->hdr_offset);
> >
> > + /*
> > + * The PE/COFF spec permits the COFF header to appear anywhere in the file, so
> > + * we need to double check whether it was where we expected it, and if not, we
> > + * must load it from the correct offset into the coff_image_header field of
> > + * struct linux_arch_kernel_header.
> > + */
> > + if ((grub_uint8_t *) lh + lh->hdr_offset != (grub_uint8_t *) &lh->coff_image_header)
> > + {
> > + grub_file_seek (file, lh->hdr_offset);
>
> Isn't this overly complicated? Why don't we first read the whole file
> into memory and then analyze it instead of using multiple accesses which
> only slows down the process?
>
Given that the condition will never hold in practice, as the offset is
always going to be 0x40, this change is not expected to affect
performance at all.
Doing a complete overhaul of the PE image loading logic for this seems
unwise to me.
