Re: [PATCH v3] util/mkimage: Fix wrong PE32+ section sizes for some arches

[Daniel Kiper]

On Tue, Apr 27, 2021 at 12:48:29PM +0200, John Paul Adrian Glaubitz wrote:
> On 4/27/21 12:25 PM, Javier Martinez Canillas wrote:
> > Commit f60ba9e5945 (util/mkimage: Refactor section setup to use a helper)
> > added a helper function to setup PE sections. But it also changed how the
> > raw data offsets were calculated since all the section sizes are aligned.
> >
> > However, for some platforms, i.e ia64-efi and arm64-efi, the kernel image
> > size is not aligned using the section alignment.
> >
> > This leads to the situation in which the mods section offset in its PE
> > section header does not match its real placement in the PE file.
> > So, finally the GRUB is not able to locate and load built-in modules.
> >
> > The problem surfaces on ia64-efi and arm64-efi because both platforms
> > require additional relocation data which is added behind .bss section.
> > So, we have to add some padding behind this extra data to make the
> > beginning of mods section properly aligned in the PE file.
> >
> > Fix it by aligning the kernel_size to the section alignment. That makes
> > the sizes and offsets in the PE section headers to match relevant sections
> > in the PE32+ binary file.
> >
> > Reported-by: John Paul Adrian Glaubitz <glaubitz@physik.fu-berlin.de>
> > Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
> > ---
> >
> > Changes in v3:
> > - Drop the RFC prefix since I'm confident now about the solution.
> > - Improve commit message (suggested by Daniel Kiper).
> > - Don't use virtual memory addresses to calculate the kernel size,
> >   but instead use the raw data sizes (suggested by Daniel Kiper).
> >
> > Changes in v2:
> > - Align up for any arch in the is_relocatable (image_target) patch and
> >   not only for MKIMAGE_ELF64 or EM_AARCH64 (suggested by Daniel Kiper).
> >
> >  util/grub-mkimagexx.c | 4 ++++
> >  1 file changed, 4 insertions(+)
> >
> > diff --git a/util/grub-mkimagexx.c b/util/grub-mkimagexx.c
> > index 00f49ccaaaf..d78fa3e5330 100644
> > --- a/util/grub-mkimagexx.c
> > +++ b/util/grub-mkimagexx.c
> > @@ -2388,6 +2388,10 @@ SUFFIX (grub_mkimage_load_image) (const char 
> > *kernel_path,
> >       layout->kernel_size += ALIGN_UP (layout->got_size, 16);
> >     }
> >  #endif
> > +
> > +      if (image_target->id == IMAGE_EFI)
> > +        layout->kernel_size = ALIGN_UP (layout->kernel_size,
> > +                                        GRUB_PE32_FILE_ALIGNMENT);
> >      }
> >    else
> >      {
> >
>
> Works on my ia64 machine.
>
> Tested-by: John Paul Adrian Glaubitz <glaubitz@physik.fu-berlin.de>

Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>

Javier, Adrian, thank you for working patiently on this fix.

Daniel