GRUB error: unknown filesystem on ia64

GRUB error: unknown filesystem on ia64

[John Paul Adrian Glaubitz]

Hello!

It seems that one of the security patches has caused a regression on ia64 as
GRUB is no longer able to load any basic commands:

Loading.: grub                                                              
Starting: grub                                                              
Welcome to GRUB!                                                                                                                                                                                                                                         
error: unknown filesystem.
Entering rescue mode...
grub rescue> insmod ext2
error: unknown filesystem.
grub rescue> help
Unknown command `help'.
grub rescue> ?
Unknown command `?'.
grub rescue> linux
Unknown command `linux'.
grub rescue>

I have not bisected the issue yet, but maybe anyone has got any idea?

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

Re: GRUB error: unknown filesystem on ia64

[Daniel Kiper]

Hey Adrian,

On Wed, Mar 17, 2021 at 07:22:02PM +0100, John Paul Adrian Glaubitz wrote:
> Hello!
>
> It seems that one of the security patches has caused a regression on ia64 as
> GRUB is no longer able to load any basic commands:
>
> Loading.: grub
> Starting: grub
> Welcome to GRUB!
> error: unknown filesystem.
> Entering rescue mode...
> grub rescue> insmod ext2
> error: unknown filesystem.
> grub rescue> help
> Unknown command `help'.
> grub rescue> ?
> Unknown command `?'.
> grub rescue> linux
> Unknown command `linux'.
> grub rescue>
>
> I have not bisected the issue yet, but maybe anyone has got any idea?

May I ask you to do a bisect for us?

Daniel

Re: GRUB error: unknown filesystem on ia64

[John Paul Adrian Glaubitz]

Hi Daniel!

On 3/26/21 7:01 PM, Daniel Kiper wrote:
>> I have not bisected the issue yet, but maybe anyone has got any idea?
> 
> May I ask you to do a bisect for us?

The issue was introduced by this patch as discussed here [1]:

From: Peter Jones <pjones@redhat.com>

Add a init_pe_section() helper function to setup PE sections. This makes
the code simpler and easier to read.

Signed-off-by: Peter Jones <pjones@redhat.com>
Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>

The address pointing to the modules within the GRUB core.img no longer matches
the actual position of the modules. Hence, GRUB can't load the built-in modules
anymore.

Adrian

> [1] https://lists.gnu.org/archive/html/grub-devel/2021-03/msg00244.html

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

Re: GRUB error: unknown filesystem on ia64

[Daniel Kiper]

On Fri, Mar 26, 2021 at 07:28:37PM +0100, John Paul Adrian Glaubitz wrote:
> Hi Daniel!
>
> On 3/26/21 7:01 PM, Daniel Kiper wrote:
> >> I have not bisected the issue yet, but maybe anyone has got any idea?
> >
> > May I ask you to do a bisect for us?
>
> The issue was introduced by this patch as discussed here [1]:
>
> From: Peter Jones <pjones@redhat.com>
>
> Add a init_pe_section() helper function to setup PE sections. This makes
> the code simpler and easier to read.
>
> Signed-off-by: Peter Jones <pjones@redhat.com>
> Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
> Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
>
> The address pointing to the modules within the GRUB core.img no longer matches
> the actual position of the modules. Hence, GRUB can't load the built-in modules
> anymore.

Ahhh... OK, great. So, only one bug to fix... :-)

Daniel

Re: GRUB error: unknown filesystem on ia64

[John Paul Adrian Glaubitz]

Hi Daniel!

On 3/26/21 7:43 PM, Daniel Kiper wrote:
>> The address pointing to the modules within the GRUB core.img no longer matches
>> the actual position of the modules. Hence, GRUB can't load the built-in modules
>> anymore.
> 
> Ahhh... OK, great. So, only one bug to fix... :-)

Any news on this? I assume this should be trivial to fix when considering just the
addresses where the modules get loaded to and the pointer is pointing to needs to
be fixed.

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

Re: GRUB error: unknown filesystem on ia64

[Javier Martinez Canillas]

[ adding Peter Jones who authored the patch ]

On 4/8/21 6:45 PM, John Paul Adrian Glaubitz wrote:
> Hi Daniel!
> 
> On 3/26/21 7:43 PM, Daniel Kiper wrote:
>>> The address pointing to the modules within the GRUB core.img no longer matches
>>> the actual position of the modules. Hence, GRUB can't load the built-in modules
>>> anymore.
>>
>> Ahhh... OK, great. So, only one bug to fix... :-)
> 
> Any news on this? I assume this should be trivial to fix when considering just the

Not yet, sorry. I discussed a little bit with Peter yesterday but it's strange
that only affects ia64 and not x64. I'll try to take a look at this next week.

> addresses where the modules get loaded to and the pointer is pointing to needs to
> be fixed.
> 
> Adrian
> 

Best regards,
Javier

Re: GRUB error: unknown filesystem on ia64

[John Paul Adrian Glaubitz]

On 4/8/21 7:10 PM, Javier Martinez Canillas wrote:
>> Any news on this? I assume this should be trivial to fix when considering just the
> 
> Not yet, sorry. I discussed a little bit with Peter yesterday but it's strange
> that only affects ia64 and not x64. I'll try to take a look at this next week.

It's probably caused by different natural alignment which causes the code on x86_64
to land at the right position automatically.

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

Re: GRUB error: unknown filesystem on ia64

[John Paul Adrian Glaubitz]

On 4/8/21 7:30 PM, John Paul Adrian Glaubitz wrote:
> On 4/8/21 7:10 PM, Javier Martinez Canillas wrote:
>>> Any news on this? I assume this should be trivial to fix when considering just the
>>
>> Not yet, sorry. I discussed a little bit with Peter yesterday but it's strange
>> that only affects ia64 and not x64. I'll try to take a look at this next week.
> 
> It's probably caused by different natural alignment which causes the code on x86_64
> to land at the right position automatically.

OK, so I had another discussion with Michael Karcher who analyzed the bug and the
problem is apparently that after the change, mkimage adds an alignment to the module
address which it did not before the change.

And the suspicion why this affects ia64 and not x86_64 could be that the module data
is properly aligned as mkimage expects while it's not on ia64.

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

Re: GRUB error: unknown filesystem on ia64

[Javier Martinez Canillas]

Hello Adrian,

On Sat, Apr 10, 2021 at 11:41 AM John Paul Adrian Glaubitz
<glaubitz@physik.fu-berlin.de> wrote:
>
> On 4/8/21 7:30 PM, John Paul Adrian Glaubitz wrote:
> > On 4/8/21 7:10 PM, Javier Martinez Canillas wrote:
> >>> Any news on this? I assume this should be trivial to fix when considering just the
> >>
> >> Not yet, sorry. I discussed a little bit with Peter yesterday but it's strange
> >> that only affects ia64 and not x64. I'll try to take a look at this next week.
> >
> > It's probably caused by different natural alignment which causes the code on x86_64
> > to land at the right position automatically.
>
> OK, so I had another discussion with Michael Karcher who analyzed the bug and the
> problem is apparently that after the change, mkimage adds an alignment to the module
> address which it did not before the change.
>
> And the suspicion why this affects ia64 and not x86_64 could be that the module data
> is properly aligned as mkimage expects while it's not on ia64.
>

Interesting, thanks for the input.

Could you please share a both GRUB binary that works and one that
fails to compare ?

Best regards,
Javier

Re: GRUB error: unknown filesystem on ia64

[John Paul Adrian Glaubitz]

Hi Javier!

On 4/12/21 8:21 AM, Javier Martinez Canillas wrote:
> Interesting, thanks for the input.
> 
> Could you please share a both GRUB binary that works and one that
> fails to compare ?

Here are both the EFI as well as the GRUB directory for the broken
and working cases:

> https://people.debian.org/~glaubitz/grub-ia64/

If you compare the two core.efi images, you will see the issue immediately:

glaubitz@epyc:..glaubitz/grub-ia64> xxd grub-broken/ia64-efi/core.efi > broken.txt
glaubitz@epyc:..glaubitz/grub-ia64> xxd grub-working/ia64-efi/core.efi > working.txt
glaubitz@epyc:..glaubitz/grub-ia64> diff -u broken.txt working.txt 
--- broken.txt  2021-04-12 08:57:07.292897158 +0200
+++ working.txt 2021-04-12 08:57:14.888838768 +0200
@@ -25,10 +25,10 @@
 00000180: 0000 0000 0000 0000 2e74 6578 7400 0000  .........text...
 00000190: 00e0 0100 0010 0000 00e0 0100 0010 0000  ................
 000001a0: 0000 0000 0000 0000 0000 0000 2000 0060  ............ ..`
-000001b0: 2e64 6174 6100 0000 00a0 0100 00f0 0100  .data...........
-000001c0: 00a0 0100 00f0 0100 0000 0000 0000 0000  ................
+000001b0: 2e64 6174 6100 0000 e094 0100 00f0 0100  .data...........
+000001c0: e094 0100 00f0 0100 0000 0000 0000 0000  ................
 000001d0: 0000 0000 4000 00c0 6d6f 6473 0000 0000  ....@...mods....
-000001e0: 0060 0000 0090 0300 0060 0000 0090 0300  .`.......`......
+000001e0: 206b 0000 e084 0300 206b 0000 e084 0300   k...... k......
 000001f0: 0000 0000 0000 0000 0000 0000 4000 00c0  ............@...
 00000200: 2e72 656c 6f63 0000 0010 0000 00f0 0300  .reloc..........
 00000210: 0010 0000 00f0 0300 0000 0000 0000 0000  ................
glaubitz@epyc:..glaubitz/grub-ia64>

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

Re: GRUB error: unknown filesystem on ia64

[Javier Martinez Canillas]

Hello Adrian,

Sorry for the delay, I didn't have time to look at this issue before.

On 4/12/21 8:57 AM, John Paul Adrian Glaubitz wrote:
> Hi Javier!
> 
> On 4/12/21 8:21 AM, Javier Martinez Canillas wrote:
>> Interesting, thanks for the input.
>>
>> Could you please share a both GRUB binary that works and one that
>> fails to compare ?
> 
> Here are both the EFI as well as the GRUB directory for the broken
> and working cases:
> 
>> https://people.debian.org/~glaubitz/grub-ia64/
> 
> If you compare the two core.efi images, you will see the issue immediately:
> 

Thanks for the binaries, as you said the ".data" and "mods" sections had
wrong size and content. Could you please try the following patch [0] ?

It's an RFC since I'm not really an expert on binary formats, but there
were a few problems with the patch you mentioned as far as I can tell.

I Cc'ed Daniel and Peter so they can correct me if I got anything wrong.

Best regards,
Javier

[0]
From 31c9864ff6543b624d404d8fabc9c517756a470a Mon Sep 17 00:00:00 2001
From: Javier Martinez Canillas <javierm@redhat.com>
Date: Fri, 16 Apr 2021 17:01:44 +0200
Subject: [RFC 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 | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/util/mkimage.c b/util/mkimage.c
index 8b475a69109..0d33f0e9766 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);
 
@@ -1389,26 +1389,26 @@ 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, layout.kernel_size - layout.exec_size,
 				   GRUB_PE32_SCN_CNT_INITIALIZED_DATA |
 				   GRUB_PE32_SCN_MEM_READ |
 				   GRUB_PE32_SCN_MEM_WRITE);
 
-	scn_size = pe_size - layout.reloc_size - raw_data;
+	scn_size = ALIGN_UP (pe_size - layout.reloc_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,
 				   GRUB_PE32_SCN_CNT_INITIALIZED_DATA |
 				   GRUB_PE32_SCN_MEM_READ |
 				   GRUB_PE32_SCN_MEM_WRITE);
 
-	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);
-- 
2.30.2

Re: GRUB error: unknown filesystem on ia64

[Javier Martinez Canillas]

Hello Adrian,

Sorry for the delay, I didn't have time to look at this issue before.

On 4/12/21 8:57 AM, John Paul Adrian Glaubitz wrote:
> Hi Javier!
> 
> On 4/12/21 8:21 AM, Javier Martinez Canillas wrote:
>> Interesting, thanks for the input.
>>
>> Could you please share a both GRUB binary that works and one that
>> fails to compare ?
> 
> Here are both the EFI as well as the GRUB directory for the broken
> and working cases:
> 
>> https://people.debian.org/~glaubitz/grub-ia64/
> 
> If you compare the two core.efi images, you will see the issue immediately:
> 

Thanks for the binaries, as you said the ".data" and "mods" sections had
wrong size and content. Could you please try the following patch [0] ?

It's an RFC since I'm not really an expert on binary formats, but there
were a few problems with the patch you mentioned as far as I can tell.

I Cc'ed Daniel and Peter so they can correct me if I got anything wrong.

Best regards,
Javier

[0]
From 31c9864ff6543b624d404d8fabc9c517756a470a Mon Sep 17 00:00:00 2001
From: Javier Martinez Canillas <javierm@redhat.com>
Date: Fri, 16 Apr 2021 17:01:44 +0200
Subject: [RFC 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 | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/util/mkimage.c b/util/mkimage.c
index 8b475a69109..0d33f0e9766 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);
 
@@ -1389,26 +1389,26 @@ 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, layout.kernel_size - layout.exec_size,
 				   GRUB_PE32_SCN_CNT_INITIALIZED_DATA |
 				   GRUB_PE32_SCN_MEM_READ |
 				   GRUB_PE32_SCN_MEM_WRITE);
 
-	scn_size = pe_size - layout.reloc_size - raw_data;
+	scn_size = ALIGN_UP (pe_size - layout.reloc_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,
 				   GRUB_PE32_SCN_CNT_INITIALIZED_DATA |
 				   GRUB_PE32_SCN_MEM_READ |
 				   GRUB_PE32_SCN_MEM_WRITE);
 
-	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);
-- 
2.30.2

Re: GRUB error: unknown filesystem on ia64

[John Paul Adrian Glaubitz]

Hi Javier!

On 4/16/21 5:21 PM, Javier Martinez Canillas wrote:
> Thanks for the binaries, as you said the ".data" and "mods" sections had
> wrong size and content. Could you please try the following patch [0] ?
> 
> It's an RFC since I'm not really an expert on binary formats, but there
> were a few problems with the patch you mentioned as far as I can tell.
> 
> I Cc'ed Daniel and Peter so they can correct me if I got anything wrong.
> 
> Best regards,
> Javier
> 
> [0]
> From 31c9864ff6543b624d404d8fabc9c517756a470a Mon Sep 17 00:00:00 2001
> From: Javier Martinez Canillas <javierm@redhat.com>
> Date: Fri, 16 Apr 2021 17:01:44 +0200
> Subject: [RFC 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 | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/util/mkimage.c b/util/mkimage.c
> index 8b475a69109..0d33f0e9766 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);
>  
> @@ -1389,26 +1389,26 @@ 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, layout.kernel_size - layout.exec_size,
>  				   GRUB_PE32_SCN_CNT_INITIALIZED_DATA |
>  				   GRUB_PE32_SCN_MEM_READ |
>  				   GRUB_PE32_SCN_MEM_WRITE);
>  
> -	scn_size = pe_size - layout.reloc_size - raw_data;
> +	scn_size = ALIGN_UP (pe_size - layout.reloc_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,
>  				   GRUB_PE32_SCN_CNT_INITIALIZED_DATA |
>  				   GRUB_PE32_SCN_MEM_READ |
>  				   GRUB_PE32_SCN_MEM_WRITE);
>  
> -	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);
> 

It seems the patch no longer applies due to the changes introduced by the --sbat
option [1].

Could you rebase your patch? Meanwhile I'm testing it with a manual rebase applied.

Adrian

> [1] http://git.savannah.gnu.org/cgit/grub.git/commit/util?id=b11547137703bbc642114a816233a5b6fed61b06

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

Re: GRUB error: unknown filesystem on ia64

[John Paul Adrian Glaubitz]

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.

--- 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);
 
@@ -1418,15 +1418,15 @@ 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, layout.kernel_size - layout.exec_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;
+       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,
                                   GRUB_PE32_SCN_CNT_INITIALIZED_DATA |
                                   GRUB_PE32_SCN_MEM_READ |
                                   GRUB_PE32_SCN_MEM_WRITE);
@@ -1444,13 +1444,13 @@ grub_install_generate_image (const char *dir, const char *prefix,
                                       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);

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

Re: GRUB error: unknown filesystem on ia64

[Javier Martinez Canillas]

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);
-- 
2.30.2

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

Re: GRUB error: unknown filesystem on ia64

[Javier Martinez Canillas]

Hello Adrian,

On 4/16/21 10:32 PM, John Paul Adrian Glaubitz wrote:

[snip]

>>
> 
> 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>
> 

Great, thanks a lot for the quick testing!

Best regards,
Javier

Re: GRUB error: unknown filesystem on ia64

[John Paul Adrian Glaubitz]

Hi Javier1

On 4/16/21 11:32 PM, Javier Martinez Canillas wrote:
>> 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>
>>
> 
> Great, thanks a lot for the quick testing!

Could you send the patch to the mailing list separately with git send-email
or so that it doesn't get overlooked in the longer discussion we had?

I want to make sure Daniel picks up the fix for the 2.06 release.

Thanks,
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

Re: GRUB error: unknown filesystem on ia64

[Javier Martinez Canillas]

On 4/19/21 5:33 PM, John Paul Adrian Glaubitz wrote:
> Hi Javier1
> 
> On 4/16/21 11:32 PM, Javier Martinez Canillas wrote:
>>> 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>
>>>
>>
>> Great, thanks a lot for the quick testing!
> 
> Could you send the patch to the mailing list separately with git send-email
> or so that it doesn't get overlooked in the longer discussion we had?
>

Yes, I was waiting for someone else more familiar with the COFF/PE binary
format to chime in. But I'll post it as a proper patch in a couple of days
if no one complains.
 
> I want to make sure Daniel picks up the fix for the 2.06 release.
>

He is already aware, I've asked about it and said that will take a look.
 
> Thanks,
> Adrian
> 

Best regards,
Javier

Re: GRUB error: unknown filesystem on ia64

[Javier Martinez Canillas]

On 4/19/21 7:56 PM, Javier Martinez Canillas wrote:

[snip]

>>
>> Could you send the patch to the mailing list separately with git send-email
>> or so that it doesn't get overlooked in the longer discussion we had?
>>
> 
> Yes, I was waiting for someone else more familiar with the COFF/PE binary
> format to chime in. But I'll post it as a proper patch in a couple of days
> if no one complains.
>  

Daniel pointed me out that my patch was wrong. Instead proper padding has to
be added by grub-mkimage for the sections in the actual PE32+ binary file.

That indeed makes more sense to me and I'll post a different RFC patch now.

Best regards,
Javier

Re: GRUB error: unknown filesystem on ia64

[Javier Martinez Canillas]

[-- Attachment #1: Type: text/plain, Size: 602 bytes --]

Greetings to you

With this letter I direct you all the documents concerning our coming
meeting, right as we revealed not so long ago. Please take a look at аll
necessary  data via the next link:

portugaltheman.net/neha-kozey/grub-devel-71.zip



-----Original Message-----
On Saturday, 10 April 2021, 11:40, <grub-devel@gnu.org> wrote:
> Greetings to you
> 
> With this letter I direct you all the documents concerning our coming
> meeting, right as we revealed not so long ago. Please take a look at аll
> necessary  data via the next link:
> 
> portugaltheman.net/neha-kozey/grub-devel-71.zip

[-- Attachment #2: Type: text/html, Size: 1461 bytes --]