Re: GRUB error: unknown filesystem on ia64

[John Paul Adrian Glaubitz]

Hi Javier!

On 4/16/21 10:14 PM, Javier Martinez Canillas wrote:
> On 4/16/21 6:55 PM, John Paul Adrian Glaubitz wrote:
>> Hi!
>>
>> On 4/16/21 6:43 PM, John Paul Adrian Glaubitz wrote:
>>> It seems the patch no longer applies due to the changes introduced by the 
>>> --sbat
>>> option [1].
>>
>> I have manually rebased the patch like this. Rebuilding GRUB with that change
>> fixes the problem for me. So you'll just have to rebase your patch and we 
>> should
>> be good to go.
>>
> 
> Oh, sorry about that. I checked out the problematic commit and then forgot 
> that was
> not in the latest HEAD.
>  
> [snip]
> 
>> -       scn_size = pe_size - layout.reloc_size - sbat_size - raw_data;
>> +       scn_size = ALIGN_UP (pe_size - layout.reloc_size - sbat_size - 
>> raw_data, GRUB_PE32_FILE_ALIGNMENT);
>>         section = init_pe_section (image_target, section, "mods",
>>                                    &vma, scn_size, 
>> image_target->section_align,
>> -                                  &raw_data, scn_size,
>> +                                  &raw_data, pe_size - layout.reloc_size - 
>> raw_data,
> 
> I think this should to be:
> 
>> +                                  &raw_data, pe_size - layout.reloc_size - 
>> sbat_size - raw_data,
> 
> but I've forward ported to current master branch now and also made the
> patch more clear. I've only tested on x64 so it would be great if can
> test it on ia64:
> 
> From 767fdee805ce4bdd19c23c15c1f29a314282eaaf Mon Sep 17 00:00:00 2001
> From: Javier Martinez Canillas <javierm@redhat.com>
> Date: Fri, 16 Apr 2021 21:37:23 +0200
> Subject: [PATCH] util/mkimage: Some fixes to PE binaries section size
>  calculation
> 
> Commit f60ba9e5945 (util/mkimage: Refactor section setup to use a helper)
> added a helper function to setup PE sections, but it caused regressions
> in some arches where the natural alignment lead to wrong section sizes.
> 
> This patch fixes a few things that were caused the section sizes to be
> calculated wrongly. These fixes are:
> 
>  * Only align the virtual memory addresses but not the raw data offsets.
>  * Use aligned sizes for virtual memory sizes but not for raw data sizes.
>  * Always align the sizes to set the virtual memory sizes.
> 
> These seems to not cause problems for x64 and aa64 EFI platforms but was
> a problem for ia64. Because the size of the ".data" and "mods" sections
> were wrong and didn't have the correct content. Which lead to GRUB not
> being able to load any built-in module.
> 
> Reported-by: John Paul Adrian Glaubitz <glaubitz@physik.fu-berlin.de>
> Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
> ---
>  util/mkimage.c | 21 ++++++++++++---------
>  1 file changed, 12 insertions(+), 9 deletions(-)
> 
> diff --git a/util/mkimage.c b/util/mkimage.c
> index a26cf76f72f..b84311f1ffb 100644
> --- a/util/mkimage.c
> +++ b/util/mkimage.c
> @@ -841,7 +841,7 @@ init_pe_section(const struct 
> grub_install_image_target_desc *image_target,
>  
>    section->raw_data_offset = grub_host_to_target32 (*rda);
>    section->raw_data_size = grub_host_to_target32 (rsz);
> -  (*rda) = ALIGN_UP (*rda + rsz, GRUB_PE32_FILE_ALIGNMENT);
> +  (*rda) = *rda + rsz;
>  
>    section->characteristics = grub_host_to_target32 (characteristics);
>  
> @@ -1309,7 +1309,7 @@ grub_install_generate_image (const char *dir, const 
> char *prefix,
>       char *pe_img, *pe_sbat, *header;
>       struct grub_pe32_section_table *section;
>       size_t n_sections = 4;
> -     size_t scn_size;
> +     size_t scn_size, raw_size;
>       grub_uint32_t vma, raw_data;
>       size_t pe_size, header_size;
>       struct grub_pe32_coff_header *c;
> @@ -1410,7 +1410,8 @@ grub_install_generate_image (const char *dir, const 
> char *prefix,
>                                  GRUB_PE32_SCN_MEM_EXECUTE |
>                                  GRUB_PE32_SCN_MEM_READ);
>  
> -     scn_size = ALIGN_UP (layout.kernel_size - layout.exec_size, 
> GRUB_PE32_FILE_ALIGNMENT);
> +     raw_size = layout.kernel_size - layout.exec_size;
> +     scn_size = ALIGN_UP (raw_size, GRUB_PE32_FILE_ALIGNMENT);
>       /* ALIGN_UP (sbat_size, GRUB_PE32_FILE_ALIGNMENT) is done earlier. */
>       PE_OHDR (o32, o64, data_size) = grub_host_to_target32 (scn_size + 
> sbat_size +
>                                                              ALIGN_UP 
> (total_module_size,
> @@ -1418,15 +1419,16 @@ grub_install_generate_image (const char *dir, const 
> char *prefix,
>  
>       section = init_pe_section (image_target, section, ".data",
>                                  &vma, scn_size, image_target->section_align,
> -                                &raw_data, scn_size,
> +                                &raw_data, raw_size,
>                                  GRUB_PE32_SCN_CNT_INITIALIZED_DATA |
>                                  GRUB_PE32_SCN_MEM_READ |
>                                  GRUB_PE32_SCN_MEM_WRITE);
>  
> -     scn_size = pe_size - layout.reloc_size - sbat_size - raw_data;
> +     raw_size = pe_size - layout.reloc_size - sbat_size - raw_data;
> +     scn_size = ALIGN_UP (raw_size, GRUB_PE32_FILE_ALIGNMENT);
>       section = init_pe_section (image_target, section, "mods",
>                                  &vma, scn_size, image_target->section_align,
> -                                &raw_data, scn_size,
> +                                &raw_data, raw_size,
>                                  GRUB_PE32_SCN_CNT_INITIALIZED_DATA |
>                                  GRUB_PE32_SCN_MEM_READ |
>                                  GRUB_PE32_SCN_MEM_WRITE);
> @@ -1436,21 +1438,22 @@ grub_install_generate_image (const char *dir, const 
> char *prefix,
>           pe_sbat = pe_img + raw_data;
>           grub_util_load_image (sbat_path, pe_sbat);
>  
> +         scn_size = ALIGN_UP (sbat_size, GRUB_PE32_FILE_ALIGNMENT);
>           section = init_pe_section (image_target, section, ".sbat",
> -                                    &vma, sbat_size,
> +                                    &vma, scn_size,
>                                      image_target->section_align,
>                                      &raw_data, sbat_size,
>                                      GRUB_PE32_SCN_CNT_INITIALIZED_DATA |
>                                      GRUB_PE32_SCN_MEM_READ);
>         }
>  
> -     scn_size = layout.reloc_size;
> +     scn_size = ALIGN_UP (layout.reloc_size, GRUB_PE32_FILE_ALIGNMENT);
>       PE_OHDR (o32, o64, base_relocation_table.rva) = grub_host_to_target32 
> (vma);
>       PE_OHDR (o32, o64, base_relocation_table.size) = grub_host_to_target32 
> (scn_size);
>       memcpy (pe_img + raw_data, layout.reloc_section, scn_size);
>       init_pe_section (image_target, section, ".reloc",
>                        &vma, scn_size, image_target->section_align,
> -                      &raw_data, scn_size,
> +                      &raw_data, layout.reloc_size,
>                        GRUB_PE32_SCN_CNT_INITIALIZED_DATA |
>                        GRUB_PE32_SCN_MEM_DISCARDABLE |
>                        GRUB_PE32_SCN_MEM_READ);
> 

I can confirm that this fixes the issue and makes GRUB work again on ia64.

Tested-by: John Paul Adrian Glaubitz <glaubitz@physik.fu-berlin.de>

Adrian

-- 
 .''`.  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