Steve McIntyre <st...@einval.com> (2014-12-01): > Control: severity 767037 serious > Control: tag 767037 +patch > > [ Raising severity to serious as I've heard more and more reports of > the problems here recently. ] > > Hi folks, > > i have two patches attached here, one for grub and one for > grub-installer. They implement the logic I described below and are > reasonably self-contained and non-intrusive. Please check them over > and give feedback, I'd like to get these in ASAP.
Some comments inline + there seems to be little to no consistency as far
as “{U,}EFI removable path” is concerned. Maybe make that uniform across
the patches to reduce user bewilderment?
Did you send the templates for review through debian-l10n-english?
> I saw almost zero response to my previous mail... :-/
>
> For this to work sensibly, we will need both patches applying.
>
> On Thu, Nov 20, 2014 at 01:49:55AM +0000, Steve McIntyre wrote:
> >
> >Hi folks,
> >
> >All of these bugs look to be the same issue: dealing with broken UEFI
> >implementations that don't find/boot a *correctly* installed grub-efi
> >loader in \EFI\debian\grubx64.efi for some reason. There's multiple
> >parts to fixing this for Debian, I think, and I'll spell them out
> >here. Please comment and tell me I'm wrong if you think I am!
> >
> > 1. Add extra support in the grub-efi*(?) packages. When we
> > install/update a grub-efi boot image (grub*.efi), add the option
> > of *also* installing that image to the removable media path:
> > \EFI\boot\boot$ARCH.efi. This code should *not* be enabled by
> > default (as correctly-functioning UEFI implementations will not
> > need it), but we should add a debconf variable to enable it. Thus,
> > this can be configured elsewhere or even pre-seeded at
> > installation time if desired. As newer, future versions of grub
> > are installed, we should ensure that the copy of grub in the
> > removable media path is updated in sync.
> >
> > (Why do we need to update the removable media path copy? To ensure
> > that the loader portion there and the rest of the grub modules,
> > config etc. remain in sync. Otherwise, badness...)
> >
> > 2. Add support (question, template, etc.) in grub-installer to set
> > the new debconf variable. This should be at low priority so most
> > people won't see it, but it's available in expert mode or (again)
> > for pre-seed use.
> >
> > 3. Add an extra path through the grub-installer code for *rescue*
> > mode: "Did you install a UEFI system but your Debian system did
> > not boot?" which can set the new debconf variable and then call
> > the normal "reinstall grub" code. We'll need to make sure we warn
> > people that this will over-write any existing UEFI bootloaders on
> > their system, so if they still want to use their old Windows
> > install dual-boot etc. they need to make sure it's configured for
> > booting via Grub.
> >
> > Ideally, it would be lovely if we can somehow guess/determine
> > automatically that there is a Debian UEFI installation on the
> > system and offer this new rescue option automatically only where
> > it makes sense. Not sure how hard/easy that would be right now,
> > before investigating the code further.
> >
> > 4. Add documentation to the installation guide to take people through
> > this process: "If you have a broken UEFI implementation on your
> > computer, then here's how to recognise it and here's what to do to
> > work around it......"
> >
> >Go on, what have I missed / misunderstood / got wrong?
> >
> >--
> >Steve McIntyre, Cambridge, UK.
> >st...@einval.com
> > Armed with "Valor": "Centurion" represents quality of Discipline,
> > Honor, Integrity and Loyalty. Now you don't have to be a Caesar to
> > concord the digital world while feeling safe and proud.
> >
> >
> >--
> >To UNSUBSCRIBE, email to debian-boot-requ...@lists.debian.org
> >with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org
> >Archive: https://lists.debian.org/20141120014955.ga28...@einval.com
> >
> >
> --
> Steve McIntyre, Cambridge, UK. st...@einval.com
> "...In the UNIX world, people tend to interpret `non-technical user'
> as meaning someone who's only ever written one device driver." -- Daniel Pead
> >From e384e597914b6e1b1dcbf96ef6782cf9bcc2313b Mon Sep 17 00:00:00 2001
> From: Steve McIntyre <st...@einval.com>
> Date: Mon, 1 Dec 2014 11:37:06 +0000
> Subject: [PATCH] Add support for forcing EFI installation to the removable
> media path
>
> Add an extra option to grub-install "--force-extra-removable". On EFI
> platforms, this will cause an extra copy of the grub-efi image to be
> written to the appropriate removable media patch
> /boot/efi/EFI/BOOT/BOOT$ARCH.EFI as well. This will help with broken
> UEFI implementations where the firmware does not work when configured
> with new boot paths.
>
> Also added new debconf logic to add this extra option to grub-install
> calls when grub2/force_efi_extra_removable is set true. This allows
> other programs like d-i / grub-installer to configure this for general
> use.
> ---
> debian/changelog | 4 +
> debian/config.in | 1 +
> debian/patches/grub-install-extra-removable.patch | 115
> ++++++++++++++++++++++
> debian/patches/series | 1 +
> debian/postinst.in | 6 +-
> debian/templates.in | 11 +++
> 6 files changed, 137 insertions(+), 1 deletion(-)
> create mode 100644 debian/patches/grub-install-extra-removable.patch
>
> diff --git a/debian/changelog b/debian/changelog
> index 09034d9..0ee7e89 100644
> --- a/debian/changelog
> +++ b/debian/changelog
> @@ -9,6 +9,10 @@ grub2 (2.02~beta2-17) UNRELEASED; urgency=medium
> [ Ian Campbell ]
> * Correct syntax error in grub-xen-host bootstrap configuration file.
>
> + [ Steve McIntyre ]
> + * Add support for forcing an extra copy of grub-efi to the removable
> + media path /boot/efi/EFI/BOOT/BOOT$ARCH.EFI (#767037)
> +
> -- Colin Watson <cjwat...@debian.org> Tue, 18 Nov 2014 14:55:27 +0000
>
> grub2 (2.02~beta2-16) unstable; urgency=medium
> diff --git a/debian/config.in b/debian/config.in
> index fcc0dd4..d2afbcb 100644
> --- a/debian/config.in
> +++ b/debian/config.in
> @@ -73,4 +73,5 @@ esac
>
> db_input ${priority} grub2/linux_cmdline || true
> db_input medium grub2/linux_cmdline_default || true
> +db_input low grub2/force_efi_extra_removable || true
> db_go
> diff --git a/debian/patches/grub-install-extra-removable.patch
> b/debian/patches/grub-install-extra-removable.patch
> new file mode 100644
> index 0000000..c32634a
> --- /dev/null
> +++ b/debian/patches/grub-install-extra-removable.patch
> @@ -0,0 +1,115 @@
> +Index: grub2-2.02~beta2/util/grub-install.c
> +===================================================================
> +--- grub2-2.02~beta2.orig/util/grub-install.c
> ++++ grub2-2.02~beta2/util/grub-install.c
> +@@ -56,6 +56,7 @@
> +
> + static char *target;
> + static int removable = 0;
> ++static int force_extra_removable = 0;
> + static int recheck = 0;
> + static int update_nvram = 1;
> + static char *install_device = NULL;
> +@@ -114,7 +115,8 @@ enum
> + OPTION_LABEL_BGCOLOR,
> + OPTION_PRODUCT_VERSION,
> + OPTION_UEFI_SECURE_BOOT,
> +- OPTION_NO_UEFI_SECURE_BOOT
> ++ OPTION_NO_UEFI_SECURE_BOOT,
> ++ OPTION_FORCE_EXTRA_REMOVABLE
> + };
> +
> + static int fs_probe = 1;
> +@@ -217,6 +219,10 @@ argp_parser (int key, char *arg, struct
> + removable = 1;
> + return 0;
> +
> ++ case OPTION_FORCE_EXTRA_REMOVABLE:
> ++ force_extra_removable = 1;
> ++ return 0;
> ++
> + case OPTION_ALLOW_FLOPPY:
> + allow_floppy = 1;
> + return 0;
> +@@ -323,6 +329,9 @@ static struct argp_option options[] = {
> + N_("do not install an image usable with UEFI Secure Boot, even if the "
> + "system was currently started using it. "
> + "This option is only available on EFI."), 2},
> ++ {"force-extra-removable", OPTION_FORCE_EXTRA_REMOVABLE, 0, 0,
> ++ N_("force installation to the removable media path also. "
> ++ "This option is only available on EFI."), 2},
> + {0, 0, 0, 0, 0, 0}
> + };
> +
> +@@ -829,6 +838,27 @@ fill_core_services (const char *core_ser
> + free (sysv_plist);
> + }
> +
> ++static void
> ++also_install_removable(const char *src, const char *base_efidir, const char
> *efi_suffix_upper)
> ++{
> ++ char *efi_file = NULL;
> ++ char *dst = NULL;
> ++ char *dir = NULL;
> ++
> ++ if (!efi_suffix_upper)
> ++ grub_util_error ("%s", _("You've found a bug"));
> ++ efi_file = xasprintf ("BOOT%s.EFI", efi_suffix_upper);
> ++
> ++ dir = grub_util_path_concat (3, base_efidir, "EFI", "BOOT");
> ++ grub_install_mkdir_p (dir);
> ++
> ++ dst = grub_util_path_concat (2, dir, efi_file);
> ++ grub_install_copy_file (src, dst, 1);
> ++ free (dst);
> ++ free (efi_file);
> ++ free (dir);
> ++}
> ++
> + int
> + main (int argc, char *argv[])
> + {
> +@@ -846,6 +876,7 @@ main (int argc, char *argv[])
> + char *relative_grubdir;
> + char **efidir_device_names = NULL;
> + grub_device_t efidir_grub_dev = NULL;
> ++ char *base_efidir = NULL;
> + char *efidir_grub_devname;
> + int efidir_is_mac = 0;
> + int is_prep = 0;
> +@@ -878,6 +909,9 @@ main (int argc, char *argv[])
> + bootloader_id = xstrdup ("grub");
> + }
> +
> ++ if (removable && force_extra_removable)
> ++ grub_util_error (_("Invalid to use both --removable and
> --force_extra_removable"));
> ++
> + if (!grub_install_source_directory)
> + {
> + if (!target)
> +@@ -1087,6 +1121,8 @@ main (int argc, char *argv[])
> + if (!efidir_is_mac && grub_strcmp (fs->name, "fat") != 0)
> + grub_util_error (_("%s doesn't look like an EFI partition.\n"), efidir);
> +
> ++ base_efidir = xstrdup(efidir);
^^^^^^^
Needs a matching free()?
> ++
> + /* The EFI specification requires that an EFI System Partition must
> + contain an "EFI" subdirectory, and that OS loaders are stored in
> + subdirectories below EFI. Vendors are expected to pick names that do
> +@@ -1949,9 +1985,15 @@ main (int argc, char *argv[])
> + fprintf (config_dst_f, "configfile $prefix/grub.cfg\n");
> + fclose (config_dst_f);
> + free (config_dst);
> ++ if (force_extra_removable)
> ++ also_install_removable(efi_signed, base_efidir, efi_suffix_upper);
> + }
> + else
> +- grub_install_copy_file (imgfile, dst, 1);
> ++ {
> ++ grub_install_copy_file (imgfile, dst, 1);
> ++ if (force_extra_removable)
> ++ also_install_removable(imgfile, base_efidir, efi_suffix_upper);
> ++ }
> + free (dst);
> + }
> + if (!removable && update_nvram)
> diff --git a/debian/patches/series b/debian/patches/series
> index 2fdba0c..0107ec5 100644
> --- a/debian/patches/series
> +++ b/debian/patches/series
> @@ -61,3 +61,4 @@ ieee1275-clear-reset.patch
> ppc64el-disable-vsx.patch
> grub-install-pvxen-paths.patch
> gettext-print-typo.patch
> +grub-install-extra-removable.patch
> diff --git a/debian/postinst.in b/debian/postinst.in
> index 30e6947..cf99e2a 100644
> --- a/debian/postinst.in
> +++ b/debian/postinst.in
> @@ -696,7 +696,11 @@ case "$1" in
> grub-efi-arm) target=arm-efi ;;
> grub-efi-arm64) target=arm64-efi ;;
> esac
> - grub-install --target="$target"
> + db_get grub2/force_efi_extra_removable
> + if [ "$RET" = true ]; then
> + FORCE_EXTRA_REMOVABLE="--force-extra-removable"
> + fi
> + grub-install --target="$target" "$FORCE_EXTRA_REMOVABLE"
> fi
>
> # /boot/grub/ has more chances of being accessible by GRUB
> diff --git a/debian/templates.in b/debian/templates.in
> index c8326f6..7068602 100644
> --- a/debian/templates.in
> +++ b/debian/templates.in
> @@ -12,6 +12,17 @@ _Description: Linux default command line:
> The following string will be used as Linux parameters for the default menu
> entry but not for the recovery mode.
>
> +Template: grub2/force_efi_extra_removable
> +Type: boolean
> +Default: false
> +_Description: Force extra installation to the EFI removable path?
> + Some UEFI-based systems are buggy and do not handle new bootloaders
> correctly.
> + If you force extra installation of GRUB to the EFI removable path, it should
> + make sure that this system will boot Debian correctly despite such a
> problem.
> + However, this may remove the ability to boot any other operating systems
> that
> + also depend on this path. If so, uou will need to ensure that GRUB is
^^^
you
> + configured successfully to be able boot any other OS installations
> correctly.
> +
> # still unused
> Template: grub2/kfreebsd_cmdline
> Type: string
> --
> 2.1.3
>
> >From 75c67230ec3464dfa9ef9e15cf05101cf814afd9 Mon Sep 17 00:00:00 2001
> From: Steve McIntyre <st...@einval.com>
> Date: Mon, 1 Dec 2014 02:53:14 +0000
> Subject: [PATCH] Add support for using the UEFI removable media path
>
> Either during installation (low priority or preseeding), or as an
> extra rescue-mode option to help people fix their systems post-install
> once they realise they need to. (#767037)
> ---
> debian/changelog | 10 ++++
> debian/grub-installer.templates | 43 +++++++++++++++
> grub-installer | 14 +++++
> rescue.d/81grub-efi-force-removable | 91
> +++++++++++++++++++++++++++++++
> rescue.d/81grub-efi-force-removable.tst | 3 +
> 5 files changed, 161 insertions(+)
> create mode 100644 rescue.d/81grub-efi-force-removable
> create mode 100644 rescue.d/81grub-efi-force-removable.tst
>
> diff --git a/debian/changelog b/debian/changelog
> index 6d94005..20e48cc 100644
> --- a/debian/changelog
> +++ b/debian/changelog
> @@ -1,3 +1,13 @@
> +grub-installer (1.102) unstable; urgency=medium
> +
> + [ Steve McIntyre ]
> + * Add extra support for forcing installation to the UEFI
> + removable media path, either during installation (low priority or
> + preseeding), or as an extra rescue-mode option to help people fix
> + their systems post-install once they realise they need to. (#767037)
> +
> + -- Steve McIntyre <93...@debian.org> Mon, 01 Dec 2014 02:49:36 +0000
> +
> grub-installer (1.101) unstable; urgency=medium
>
> [ Steve McIntyre ]
> diff --git a/debian/grub-installer.templates b/debian/grub-installer.templates
> index e439ad0..a6af2ec 100644
> --- a/debian/grub-installer.templates
> +++ b/debian/grub-installer.templates
> @@ -209,6 +209,21 @@ Type: text
> # :sl1:
> _Description: Updating /etc/kernel-img.conf...
>
> +Template: grub-installer/progress/step_force_efi
> +Type: text
> +# :sl1:
> +_Description: Checking whether to force usage of the removable media path
> +
> +Template: grub-installer/progress/step_mount_filesystems
> +Type: text
> +# :sl1:
> +_Description: Mounting filesystems
> +
> +Template: grub-installer/progress/step_update_debconf_efi_removable
> +Type: text
> +# :sl1:
> +_Description: Configuring grub-efi for future usage of the removable media
> path
> +
> Template: debian-installer/grub-installer/title
> Type: text
> # Main menu item
> @@ -242,3 +257,31 @@ _Description: Failed to mount /target/proc
> Check /var/log/syslog or see virtual console 4 for the details.
> .
> Warning: Your system may be unbootable!
> +
> +Template: rescue/menu/grub-efi-force-removable
> +Type: text
> +# Rescue menu item
> +# :sl2:
> +_Description: Force GRUB installation to the UEFI removable media path
> +
> +Template: grub-installer/force-efi-extra-removable
> +Type: boolean
> +Default: false
> +# :sl1:
> +_Description: Force GRUB installation to the UEFI removable media path?
> + It seems that this computer is configured to boot via UEFI, but maybe
> + that configuration will not work for booting from the hard
> + drive. Some UEFI firmware implementations do not meet the UEFI
> + specification (i.e. they are buggy!) and do not support proper
> + configuration of boot options from system hard drives.
> + .
> + A workaround for this problem is to install an extra copy of the UEFI
> + version of the GRUB boot loader to a fallback location, the
> + "removable media path". Almost all UEFI systems, no matter how buggy,
> + will boot GRUB that way.
> + .
> + Warning: If the installer failed to detect another operating system
> + that is present on your computer that also depends on this fallback,
> + installing GRUB there will make that operating system temporarily
> + unbootable. GRUB can be manually configured later to boot it if
> + necessary.
> diff --git a/grub-installer b/grub-installer
> index 4c12998..ef81dbf 100755
> --- a/grub-installer
> +++ b/grub-installer
> @@ -785,6 +785,20 @@ if [ -z "$frdisk" ]; then
> fi
> fi
>
> + # Should we force a copy of grub-efi to be installed
> + # to the removable media path too? Ask at low
> + # priority, or can also be pre-seeded of course
> + db_input low grub-installer/force-efi-extra-removable || [ $?
> -eq 30 ]
> + db_go || exit 10
> + db_get grub-installer/force-efi-extra-removable
> + if [ "$RET" = true ]; then
> + grub_install_params="$grub_install_params
> --force-extra-removable"
> + # Make sure this happens on upgrades too
> + $chroot $ROOT 'debconf-set-selections' <<EOF
> +grub2/force_efi_extra_removable boolean true
> +EOF
> + fi
> +
> if [ "$ARCH" = "powerpc/chrp_pegasos" ] ; then
> # nvram is broken here
> grub_install_params="$grub_install_params --no-nvram"
> diff --git a/rescue.d/81grub-efi-force-removable
> b/rescue.d/81grub-efi-force-removable
> new file mode 100644
> index 0000000..b35b724
> --- /dev/null
> +++ b/rescue.d/81grub-efi-force-removable
> @@ -0,0 +1,91 @@
> +#! /bin/sh -e
> +
> +. /usr/share/debconf/confmodule
> +
> +. /usr/share/grub-installer/functions.sh
> +
> +log () {
> + logger -t grub-installer "grub-efi-force-removable $@"
> +}
> +
> +error () {
> + log "error: $@"
> +}
> +
> +die () {
> + local template="$1"
> + shift
> +
> + error "$@"
> + db_input critical "$template" || [ $? -eq 30 ]
> + db_go || true
> + exit 1
> +}
> +
> +mountvirtfs () {
> + fstype="$1"
> + path="$2"
> + if grep -q "[[:space:]]$fstype\$" /proc/filesystems && \
> + ! grep -q "^[^ ]\+ \+$path " /proc/mounts; then
> + mkdir -p "$path" || \
> + die grub-installer/mounterr "Error creating $path"
> + mount -t "$fstype" "$fstype" "$path" || \
> + die grub-installer/mounterr "Error mounting $path"
> + trap "umount $path" HUP INT QUIT KILL PIPE TERM EXIT
> + fi
> +}
> +
> +db_progress START 0 3 grub-installer/progress/title
> +db_progress INFO grub-installer/progress/step_force_efi
> +
> +# Should we also install grub-efi to the removable media path?
> +# Ask the user
> +log "Prompting user about removable media path"
> +db_input high grub-installer/force-efi-extra-removable
> +if ! db_go; then
> + # back up to menu
> + db_progress STOP
> + exit 10
> +fi
> +db_get grub-installer/force-efi-extra-removable
> +if [ "$RET" != true ]; then
> + db_progress STOP
> + exit 0
> +fi
> +
> +db_progress STEP 1
> +db_progress INFO grub-installer/progress/step_mount_filesystems
> +
> +log "Mounting filesystems"
> +# If we're installing grub-efi, it wants /sys mounted in the
> +# target. Maybe /proc too?
> +mountvirtfs proc /target/proc
> +mountvirtfs sysfs /target/sys
> +chroot /target mount /boot/efi || true
> +
> +db_progress STEP 1
> +db_progress INFO grub-installer/progress/step_install_loader
> +# Do the installation now
> +log "Running grub-install"
> +if ! chroot /target grub-install --force-extra-removable; then
in-target?
> + db_input critical grub-installer/grub-install-failed || true
> + db_go || true
> + db_progress STOP
> + exit 1
> +fi
> +
> +db_progress STEP 1
> +db_progress INFO grub-installer/progress/step_update_debconf_efi_removable
> +# And add the debconf flag so the installed system will also do this in
> future
> +log "Running debconf-set-selections in the chroot"
> +chroot /target 'debconf-set-selections' <<EOF
> +grub2/force_efi_extra_removable boolean true
> +EOF
> +
> +db_progress STEP 1
> +db_progress STOP
> +
> +# And do some cleanup
> +umount /target/boot/efi
> +umount /target/sys
> +umount /target/proc
> diff --git a/rescue.d/81grub-efi-force-removable.tst
> b/rescue.d/81grub-efi-force-removable.tst
> new file mode 100644
> index 0000000..9b78191
> --- /dev/null
> +++ b/rescue.d/81grub-efi-force-removable.tst
> @@ -0,0 +1,3 @@
> +#! /bin/sh -e
> +[ -f /target/boot/grub/grub.cfg ] && ( grep -q /boot/efi /target/etc/fstab )
> +
Mraw,
KiBi.
signature.asc
Description: Digital signature
