All of lore.kernel.org
 help / color / mirror / Atom feed
From: Javier Martinez Canillas <fmartine@redhat.com>
To: John Paul Adrian Glaubitz <glaubitz@physik.fu-berlin.de>,
	The development of GNU GRUB <grub-devel@gnu.org>
Cc: Daniel Kiper <daniel.kiper@oracle.com>, Peter Jones <pjones@redhat.com>
Subject: Re: GRUB error: unknown filesystem on ia64
Date: Fri, 16 Apr 2021 22:14:02 +0200	[thread overview]
Message-ID: <35141d37-a745-eaae-5c1d-d23ed2d3ae09@redhat.com> (raw)
In-Reply-To: <2b2d1558-ad3b-afe8-3e96-c150533ae59f@physik.fu-berlin.de>

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



  reply	other threads:[~2021-04-16 20:14 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-03-17 18:22 GRUB error: unknown filesystem on ia64 John Paul Adrian Glaubitz
2021-03-26 18:01 ` Daniel Kiper
2021-03-26 18:28   ` John Paul Adrian Glaubitz
2021-03-26 18:43     ` Daniel Kiper
2021-04-08 16:45       ` John Paul Adrian Glaubitz
2021-04-08 17:10         ` Javier Martinez Canillas
2021-04-08 17:30           ` John Paul Adrian Glaubitz
2021-04-10  9:40             ` John Paul Adrian Glaubitz
2021-04-12  6:21               ` Javier Martinez Canillas
2021-04-12  6:57                 ` John Paul Adrian Glaubitz
2021-04-16 15:21                   ` Javier Martinez Canillas
2021-04-16 16:43                     ` John Paul Adrian Glaubitz
2021-04-16 16:55                       ` John Paul Adrian Glaubitz
2021-04-16 20:14                         ` Javier Martinez Canillas [this message]
2021-04-16 20:32                           ` John Paul Adrian Glaubitz
2021-04-16 21:32                             ` Javier Martinez Canillas
2021-04-19 15:33                               ` John Paul Adrian Glaubitz
2021-04-19 17:56                                 ` Javier Martinez Canillas
2021-04-21 17:54                                   ` Javier Martinez Canillas
2021-04-16 15:21                   ` Javier Martinez Canillas
2021-06-16 14:22 Javier Martinez Canillas

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=35141d37-a745-eaae-5c1d-d23ed2d3ae09@redhat.com \
    --to=fmartine@redhat.com \
    --cc=daniel.kiper@oracle.com \
    --cc=glaubitz@physik.fu-berlin.de \
    --cc=grub-devel@gnu.org \
    --cc=pjones@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.