Re: [PATCH v2 1/2] grub-install: Add missing points of no return for IEEE1275 on i386/powerpc
On Mon, Aug 29, 2022 at 04:36:24PM +0200, Ismael Luceno wrote:
> Signed-off-by: Ismael Luceno
> ---
> util/grub-install.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/util/grub-install.c b/util/grub-install.c
> index 7b04bd3c534b..527b85e27aa7 100644
> --- a/util/grub-install.c
> +++ b/util/grub-install.c
> @@ -1835,6 +1835,7 @@ main (int argc, char *argv[])
> {
> if (write_to_disk (ins_dev, imgfile))
> grub_util_error ("%s", _("failed to copy Grub to the PReP
> partition"));
> + grub_set_install_backup_ponr ();
This looks good to me.
> }
> else
> {
> @@ -1859,6 +1860,7 @@ main (int argc, char *argv[])
> partno = grub_dev->disk->partition
> ? grub_dev->disk->partition->number + 1 : 0;
> dev = grub_util_get_os_disk (grub_devices[0]);
> + grub_set_install_backup_ponr ();
This should be moved way up, given failure may occur at
grub_make_system_path_relative_to_its_root or even earlier.
But in the first place the attempt may have been wrong, given the image
is installed as file in the platform directory, there we should not be
botherd to set grub_set_install_backup_ponr as file should be able to be
backup and restored thus is covered. On the other hand, the
grub_set_install_backup_ponr looked like a workaround to me to deal with
raw images embedded in partition that has no good way to restore it.
That is for i386-ieee1275, /boot/grub2/i386-ieee1275/grub should be
added to the list of backup and restore to get it to work in failure
recovery path.
Thanks,
Michael
> grub_install_register_ieee1275 (0, dev,
> partno, relpath);
> }
> --
> 2.37.1
>
>
> ___
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH v2 2/2] grub-install: Ensure a functional /dev/nvram
On Mon, Aug 29, 2022 at 04:36:25PM +0200, Ismael Luceno wrote:
> This enables an early failure; for i386-ieee1275 and powerpc-ieee1275 on
> Linux, without /dev/nvram the system may be left in an unbootable state.
>
> Signed-off-by: Ismael Luceno
> ---
> util/grub-install.c | 23 +++
> 1 file changed, 23 insertions(+)
>
> diff --git a/util/grub-install.c b/util/grub-install.c
> index 527b85e27aa7..c84a2fb0526f 100644
> --- a/util/grub-install.c
> +++ b/util/grub-install.c
> @@ -827,6 +827,16 @@ fill_core_services (const char *core_services)
>free (sysv_plist);
> }
>
> +static void
> +try_open (const char *path)
> +{
> + FILE *f;
> + f = grub_util_fopen (path, "r+");
> + if (!f)
> +grub_util_error (_("Unable to open %s: %s"), path, strerror(errno));
> + fclose (f);
> +}
> +
> int
> main (int argc, char *argv[])
> {
> @@ -1026,6 +1036,19 @@ main (int argc, char *argv[])
>break;
> }
>
> + switch (platform)
> +{
> +case GRUB_INSTALL_PLATFORM_I386_IEEE1275:
> +case GRUB_INSTALL_PLATFORM_POWERPC_IEEE1275:
> +#ifdef __linux__
> + /* On Linux, ensure /dev/nvram is _functional_. */
> + try_open ("/dev/nvram");
The update_nvram variable should be tested in case user has skipped it.
Thanks,
Michael
> +#endif
> + break;
> +default:
> + break;
> +}
> +
>/* Find the EFI System Partition. */
>
>if (is_efi)
> --
> 2.37.1
>
>
> ___
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
[PATCH v3 1/2] grub-install: Ensure a functional /dev/nvram
This enables an early failure; for i386-ieee1275 and powerpc-ieee1275 on
Linux, without /dev/nvram the system may be left in an unbootable state.
Signed-off-by: Ismael Luceno
---
util/grub-install.c | 24
1 file changed, 24 insertions(+)
diff --git a/util/grub-install.c b/util/grub-install.c
index 7b04bd3c534b..da50bd58b73b 100644
--- a/util/grub-install.c
+++ b/util/grub-install.c
@@ -827,6 +827,16 @@ fill_core_services (const char *core_services)
free (sysv_plist);
}
+static void
+try_open (const char *path)
+{
+ FILE *f;
+ f = grub_util_fopen (path, "r+");
+ if (!f)
+grub_util_error (_("Unable to open %s: %s"), path, strerror(errno));
+ fclose (f);
+}
+
int
main (int argc, char *argv[])
{
@@ -1026,6 +1036,20 @@ main (int argc, char *argv[])
break;
}
+ switch (platform)
+{
+case GRUB_INSTALL_PLATFORM_I386_IEEE1275:
+case GRUB_INSTALL_PLATFORM_POWERPC_IEEE1275:
+#ifdef __linux__
+ /* On Linux, ensure /dev/nvram is _functional_. */
+ if (update_nvram)
+try_open ("/dev/nvram");
+#endif
+ break;
+default:
+ break;
+}
+
/* Find the EFI System Partition. */
if (is_efi)
--
2.37.3
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
[PATCH v3 0/2] Fix installation issues on ppc64le
If the nvram device is non-functional, e.g. because the nvram module isn't loaded and it's file been removed from the filesystem, thus can't be loaded, the installation will be attempted but the system will be left in an unbootable state. The boot process shows: Welcome to GRUB! error: ../../grub-core/kern/dl.c:380:symbol `grub_disk_get_size' not found. Entering rescue mode... grub rescue> Introduce a point of no return after embedding grub in the PReP partition, and make sure /dev/nvram is functional or fail early. Ismael Luceno (2): grub-install: Ensure a functional /dev/nvram grub-install: Add point of no return for IEEE1275 on powerpc util/grub-install.c | 25 + 1 file changed, 25 insertions(+) Changes since v2: * Removed point of no return for i386 * Check for update_nvram before opening /dev/nvarm Changes since v1: * Check /dev/nvram early * Removed loading of the nvram module * Added points of no return -- 2.37.3 ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
[PATCH v3 2/2] grub-install: Add point of no return for IEEE1275 on powerpc
Signed-off-by: Ismael Luceno
---
util/grub-install.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/util/grub-install.c b/util/grub-install.c
index da50bd58b73b..45f549c25867 100644
--- a/util/grub-install.c
+++ b/util/grub-install.c
@@ -1859,6 +1859,7 @@ main (int argc, char *argv[])
{
if (write_to_disk (ins_dev, imgfile))
grub_util_error ("%s", _("failed to copy Grub to the PReP
partition"));
+ grub_set_install_backup_ponr ();
}
else
{
--
2.37.3
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH v3 5/8] linux/arm: account for COFF headers appearing at unexpected offsets
On Mon, 5 Sept 2022 at 00:12, Ard Biesheuvel wrote:
>
> On Fri, 2 Sept 2022 at 12:02, Heinrich Schuchardt
> wrote:
> >
> > On 8/18/22 16:51, Ard Biesheuvel wrote:
> > > The way we load the Linux and PE/COFF image headers depends on a fixed
> > > placement of the COFF header at offset 0x40 into the file. This is a
> > > reasonable default, given that this is where Linux emits it today.
> > > However, in order to comply with the PE/COFF spec, which allows this
> > > header to appear anywhere in the file, let's ensure that we read the
> > > header from where it actually appears in the file if it is not located
> > > at offset 0x40.
> > >
> > > Signed-off-by: Ard Biesheuvel
> > > ---
> > > grub-core/loader/arm64/linux.c | 15 +++
> > > 1 file changed, 15 insertions(+)
> > >
> > > diff --git a/grub-core/loader/arm64/linux.c
> > > b/grub-core/loader/arm64/linux.c
> > > index 7c0f17cf933d..56ba8d0a6ea3 100644
> > > --- a/grub-core/loader/arm64/linux.c
> > > +++ b/grub-core/loader/arm64/linux.c
> > > @@ -63,6 +63,21 @@ grub_arch_efi_linux_load_image_header (grub_file_t
> > > file,
> > > grub_dprintf ("linux", "UEFI stub kernel:\n");
> > > grub_dprintf ("linux", "PE/COFF header @ %08x\n", lh->hdr_offset);
> > >
> > > + /*
> > > + * The PE/COFF spec permits the COFF header to appear anywhere in the
> > > file, so
> > > + * we need to double check whether it was where we expected it, and if
> > > not, we
> > > + * must load it from the correct offset into the coff_image_header
> > > field of
> > > + * struct linux_arch_kernel_header.
> > > + */
> > > + if ((grub_uint8_t *) lh + lh->hdr_offset != (grub_uint8_t *)
> > > &lh->coff_image_header)
> > > +{
> > > + grub_file_seek (file, lh->hdr_offset);
> > > +
> > > + if (grub_file_read (file, &lh->coff_image_header, sizeof(struct
> > > grub_coff_image_header))
> > > + != sizeof(struct grub_coff_image_header))
> > > + return grub_error(GRUB_ERR_FILE_READ_ERROR, "failed to read COFF
> > > image header");
> > > +}
> >
> > Why do we use multiple reads? We need the whole binary for booting.
>
> We need multiple reads because the PE/COFF header describes the
> mapping from file offsets to memory offsets - even if Linux kernels
> generally use a 1:1 mapping between the location of a PE/COFF section
> in the file and in memory, the PE/COFF spec does not require this at
> all.
>
> So reading the whole file into memory might require multiple
> memcpy/memmove calls to rearrange the copied file in memory so it
> matches the section layout as the header describes it.
>
> Therefore, reading just the header is a reasonable thing to do here.
> And note that the second read only happens when the header layout
> deviates from the expected default.
>
> > Reading the complete file into memory once should be good enough. But
> > that seems to be more a question of overall design.
> >
> > In grub_efi_modules_addr() the same logic is needed.
>
> Not really. grub_efi_modules_addr() is only used to load GRUB's own
> PE/COFF image, and its layout is known a priori. So having this logic
> there is essentially dead code.
>
> > It is not ARM
> > specific. Please, move it to a common code module.
> >
>
> That makes sense - I'll move it to an appropriate common source file.
>
Actually, this is not as straight-forward as it may seem: struct
linux_arch_kernel_header is used in this function, which is specific
to linux so it does not belong in the generic EFI part of the code.
I am going to leave this as-is for the time being.
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [Regression] efi: Don't display a uefi-firmware entry if it's not supported
On Wed, 31 Aug 2022 16:14:42 +0100
Dimitri John Ledkov wrote:
> On Tue, 30 Aug 2022 at 21:22, Robbie Harwood wrote:
> >
> > Philip Müller writes:
> >
> > >> Hello Robbie, hello Daniel,
> > >>
> > >> with the commit 26031d3b101648352e4e427f04bf69d320088e77
> > >> 30_uefi-firmware will always call `fwsetup --is-supported' to check
> > >> if the system supports EFI or not. However most installed grub
> > >> versions on MBR don't support the '--is-supported' flag and crash the
> > >> menu creation routine.
> > >>
> > >> Arch Linux is tracking the issue here:
> > >> https://bugs.archlinux.org/task/75701
> > >>
> > >> What would be the best approach with older installations of grub via
> > >> grub-install not having to reinstall grub to MBR as some users don't
> > >> even know on how to install grub properly as that job a graphical
> > >> installer does.
> > >
> > > Hi all,
> >
> > Adding grub-devel to CC.
> >
> > > I looked into the '30_uefi-firmware' changes a little more and also
> > > commented my findings at the Arch-Bugtracker:
> > > https://bugs.archlinux.org/task/75701#comment210684
> > >
> > > Before Menu changes we simply had this:
> > >
> > > ### BEGIN /etc/grub.d/30_uefi-firmware ###
> > > menuentry 'UEFI Firmware Settings' $menuentry_id_option 'uefi-firmware' {
> > >fwsetup
> > > }
> > > ### END /etc/grub.d/30_uefi-firmware ###
> > >
> > > After Menu changes we went to that:
> > >
> > > ### BEGIN /etc/grub.d/30_uefi-firmware ###
> > > fwsetup --is-supported
> > > if [ "$grub_platform" = "efi" -a "$?" = 0 ]; then
> > >menuentry 'UEFI Firmware Settings' $menuentry_id_option
> > > 'uefi-firmware' {
> > > fwsetup
> > >}
> > > fi
>
> above code looks buggy to me. As far as I understand fwsetup only
> works on EFI grub_platform, and thus originally the menu entry and
> fwsetup call was scoped under efi platform. thus there should be two
> if conditions, not one:
>
> if [ "$grub_platform" = "efi" ]; then
> fwsetup --is-supported
> if [ "$?" = 0 ]; then
> menuetry 'UEFI Firmware Settings' $menuentry_id_option 'uefi-firmware' {
> fwsetup
> }
> fi
> fi
I agree there's a bug, but I don't think its what is mentioned. The bug
is that $? is not set to 1 when a command does not exist. So in
i386-pc, where there is no fwsetup, executing "fwsetup --is-supported"
will fail because there is no command fwsetup. And $? should be set to
1 in this case, which would allow the existing code to work as expected.
Even if we fix the bug I mention, I support the above suggested change
because its easier to read.
>
> This way the code continues to work correctly on bios platforms
> (skipped completely), and on EFI like platforms fwsetup menu entry is
> only shown if fwsetup --is-supported executes successfully.
>
> the optimisation of having two conditionals evaluated together, whilst
> efficient, is wrong given cross grub platform compatibilities desires.
>
> This fwsetup has never before been executed on bios platform, and we
> should not be introducing this.
And it never will be because its never built for the BIOS platform.
GRUB will error with "error: [16] can't find command `fwsetup'.".
Glenn
>
> > > ### END /etc/grub.d/30_uefi-firmware ###
> > >
> > > So 0eb684e8bfb0a9d2d42017a354740be25947babe I get and
> > > 26031d3b101648352e4e427f04bf69d320088e77 tries to improve things,
> > > however doesn't count in what will happen if 'fwsetup' is not supporting
> > > the flag. It may either crash due to
> > > 1e79bbfbda24a08cb856ff30f8b1bec460779b91 or start UEFI firmware instead.
> >
> > Why doesn't grub on the MBR get updated when you install a new grub
> > package? That seems like the real issue here - what am I missing?
> >
> > Regardless, if we can't count on fwsetup being updated, then I think we
> > need to go back to the original version of my patch which doesn't have
> > --is-supported.
> >
> > > Simply don't break user-space when older grub got installed to MBR.
> > > Somehow this idea needs some more thinking and a solution for that
> > > case too.
> >
> > Sure, what do you suggest?
> >
> > Be well,
> > --Robbie
> > ___
> > Grub-devel mailing list
> > Grub-devel@gnu.org
> > https://lists.gnu.org/mailman/listinfo/grub-devel
>
>
>
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
