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

2021-04-27 Thread John Paul Adrian Glaubitz
Hi Javier!

On 4/26/21 7:18 PM, Daniel Kiper wrote:
> (...)
> I think this should be:
>   layout->kernel_size = ALIGN_UP (layout->kernel_size, 
> GRUB_PE32_FILE_ALIGNMENT);
> 
> Here we care about alignment in the PE file not in the memory. So,
> I think we have to use GRUB_PE32_FILE_ALIGNMENT instead. Though
> the result is the same...

Let me know once you have a new version of the patch ready, I'll test it as 
quickly
as possible.

Adrian

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

___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel


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

2021-04-27 Thread Javier Martinez Canillas
Hello Adrian,

On 4/27/21 9:55 AM, John Paul Adrian Glaubitz wrote:
> Hi Javier!
> 
> On 4/26/21 7:18 PM, Daniel Kiper wrote:
>> (...)
>> I think this should be:
>>   layout->kernel_size = ALIGN_UP (layout->kernel_size, 
>> GRUB_PE32_FILE_ALIGNMENT);
>>
>> Here we care about alignment in the PE file not in the memory. So,
>> I think we have to use GRUB_PE32_FILE_ALIGNMENT instead. Though
>> the result is the same...
> 
> Let me know once you have a new version of the patch ready, I'll test it as 
> quickly
> as possible.
>

Thanks! I was busy this morning with other things but I'll post a v3 now.

I'll keep your Tested-by tag since the size calculated should be the same,
but would be good to test it again just to make sure that's the case.
 
> Adrian
> 

Best regards,
-- 
Javier Martinez Canillas
Software Engineer - Desktop Hardware Enablement
Red Hat


___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel


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

2021-04-27 Thread Javier Martinez Canillas
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 
Signed-off-by: Javier Martinez Canillas 
---

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


___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel


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

2021-04-27 Thread 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 
> Signed-off-by: Javier Martinez Canillas 
> ---
> 
> 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 

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

___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel


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

2021-04-27 Thread Javier Martinez Canillas
On 4/27/21 12:48 PM, John Paul Adrian Glaubitz wrote:
> On 4/27/21 12:25 PM, Javier Martinez Canillas wrote:

[snip]

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

Thanks for testing it again! Hopefully this could be the last version
of this patch.

Best regards,
-- 
Javier Martinez Canillas
Software Engineer - Desktop Hardware Enablement
Red Hat


___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel


grub-2:2.06rc1-2 Installation Messages Related to Secure Boot as Displayed under Arch Linux

2021-04-27 Thread HardenedArray via Grub-devel
As a heads up to those working on the Secure Boot aspects of grub, I wanted to 
provide the grub-2:2.06rc1-2 installation messages being shown to Arch Linux 
grub users during a 'pacman -Syu' run that happens to pull in the latest grub 
upgrade.

2/3) upgrading grub [##] 100%
:: Recent versions of grub may fail to boot with secure boot enabled. The
message will look like this:
> error: verification requested but nobody cares:
> (hd0,gpt2)/grub/x86_64-efi/normal.mod
> Entering rescue mode...
Handle installation to UEFI with care and be prepared!
For details see: https://md.archlinux.org/F1JuYj5xQtWyhvH8_ilErg#

Not all Arch grub users are seeing these messages, as grub-2:2.06rc1-2 resides 
only in the TESTING repo.

I do not run Secure Boot, so as expected, grub-2:2.06rc1-2 did not interfere 
with unlocking my LUKS2 encrypted /boot upon reboot.

I am not sure any action is required, I only wanted those involved with Secure 
Boot to be aware of the information being shown to grub end users on Arch Linux.

Cheers___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel


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

2021-04-27 Thread 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 
> > Signed-off-by: Javier Martinez Canillas 
> > ---
> >
> > 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 

Reviewed-by: Daniel Kiper 

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

Daniel

___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel