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

[John Paul Adrian Glaubitz]

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>

-- 
 .''`.  John Paul Adrian Glaubitz
: :' :  Debian Developer - glaubitz@debian.org
`. `'   Freie Universitaet Berlin - glaubitz@physik.fu-berlin.de
  `-    GPG: 62FF 8A75 84E0 2956 9546  0006 7426 3B37 F5B5 F913