Package: release.debian.org Severity: normal Tags: buster User: release.debian....@packages.debian.org Usertags: pu
Following the security updates in July for the "BootHole" set of vulnerabilities, we had a number of reports of failures to boot after the upgrade. These weren't fundamentally a new problem, in that we've always had a smattering of such reports after GRUB kernel/module ABI changes, but it's obviously problematic for affected users and it creates a considerable distraction from trying to work out whether the security update itself is in fact OK, so I'd like to attempt to improve the situation in stable. The attached patch is a set of backports from unstable, partly by me and partly from similar attempts to improve upgrade reliability in Ubuntu, thanks to Dimitri John Ledkov. I also needed to backport a patch from GRUB 2.04 which it turns out Dimitri's patch was implicitly relying on, since otherwise some of our self-tests failed. I've targeted this at buster, but I can send it to buster-security instead if people feel that's more appropriate. Thanks, -- Colin Watson (he/him) [cjwat...@debian.org]
diff -Nru grub2-2.02+dfsg1/debian/.git-dpm grub2-2.02+dfsg1/debian/.git-dpm --- grub2-2.02+dfsg1/debian/.git-dpm 2020-07-30 18:15:27.000000000 +0100 +++ grub2-2.02+dfsg1/debian/.git-dpm 2020-11-29 16:53:52.000000000 +0000 @@ -1,6 +1,6 @@ # see git-dpm(1) from git-dpm package -578cb13630359430732c76acf1e6fd6c1e9e5a76 -578cb13630359430732c76acf1e6fd6c1e9e5a76 +cdc5efa6be1dbb8a4344f9c000eb5c99983c2225 +cdc5efa6be1dbb8a4344f9c000eb5c99983c2225 59aeb1cfaa3d5bfd7bbeeee0f0d37f6d9eed51fe 59aeb1cfaa3d5bfd7bbeeee0f0d37f6d9eed51fe grub2_2.02+dfsg1.orig.tar.xz diff -Nru grub2-2.02+dfsg1/debian/changelog grub2-2.02+dfsg1/debian/changelog --- grub2-2.02+dfsg1/debian/changelog 2020-07-30 20:19:53.000000000 +0100 +++ grub2-2.02+dfsg1/debian/changelog 2020-11-29 16:54:59.000000000 +0000 @@ -1,3 +1,23 @@ +grub2 (2.02+dfsg1-20+deb10u3) buster; urgency=high + + [ Colin Watson ] + * When upgrading grub-pc noninteractively, bail out if grub-install fails. + It's better to fail the upgrade than to produce a possibly-unbootable + system. + * Explicitly check whether the target device exists before running + grub-install, since grub-install copies modules to /boot/grub/ before + installing the core image, and the new modules might be incompatible + with the old core image (closes: #966575). + * Backport from upstream: + - unix exec: avoid atexit handlers when child exits + + [ Dimitri John Ledkov ] + * grub-install: Add backup and restore. + * Don't call grub-install on fresh install of grub-pc. It's the job of + installers to do that after a fresh install. + + -- Colin Watson <cjwat...@debian.org> Sun, 29 Nov 2020 16:54:59 +0000 + grub2 (2.02+dfsg1-20+deb10u2) buster-security; urgency=high * Fix a regression caused by "efi: fix some malformed device path diff -Nru grub2-2.02+dfsg1/debian/patches/exec-avoid-atexit-when-child-exits.patch grub2-2.02+dfsg1/debian/patches/exec-avoid-atexit-when-child-exits.patch --- grub2-2.02+dfsg1/debian/patches/exec-avoid-atexit-when-child-exits.patch 1970-01-01 01:00:00.000000000 +0100 +++ grub2-2.02+dfsg1/debian/patches/exec-avoid-atexit-when-child-exits.patch 2020-11-29 16:53:49.000000000 +0000 @@ -0,0 +1,75 @@ +From d399aa3efa5b1f3653c76cac9f3b27898b03b8a8 Mon Sep 17 00:00:00 2001 +From: Patrick Steinhardt <p...@pks.im> +Date: Mon, 28 Aug 2017 20:57:19 +0200 +Subject: unix exec: avoid atexit handlers when child exits + +The `grub_util_exec_redirect_all` helper function can be used to +spawn an executable and redirect its output to some files. After calling +`fork()`, the parent will wait for the child to terminate with +`waitpid()` while the child prepares its file descriptors, environment +and finally calls `execvp()`. If something in the children's setup +fails, it will stop by calling `exit(127)`. + +Calling `exit()` will cause any function registered via `atexit()` to be +executed, which is usually the wrong thing to do in a child. And +actually, one can easily observe faulty behaviour on musl-based systems +without modprobe(8) installed: executing `grub-install --help` will call +`grub_util_exec_redirect_all` with "modprobe", which obviously fails if +modprobe(8) is not installed. Due to the child now exiting and invoking +the `atexit()` handlers, it will clean up some data structures of the +parent and cause it to be deadlocked in the `waitpid()` syscall. + +The issue can easily be fixed by calling `_exit(127)` instead, which is +especially designed to be called when the atexit-handlers should not be +executed. + +Signed-off-by: Patrick Steinhardt <p...@pks.im> + +Origin: upstream, https://git.savannah.gnu.org/cgit/grub.git/commit/?id=e75cf4a58b5eaf482804e5e1b2cc7d4399df350e +Last-Update: 2020-11-29 + +Patch-Name: exec-avoid-atexit-when-child-exits.patch +--- + grub-core/osdep/unix/exec.c | 8 ++++---- + 1 file changed, 4 insertions(+), 4 deletions(-) + +diff --git a/grub-core/osdep/unix/exec.c b/grub-core/osdep/unix/exec.c +index 935ff120e..db3259f65 100644 +--- a/grub-core/osdep/unix/exec.c ++++ b/grub-core/osdep/unix/exec.c +@@ -99,7 +99,7 @@ grub_util_exec_redirect_all (const char *const *argv, const char *stdin_file, + { + fd = open (stdin_file, O_RDONLY); + if (fd < 0) +- exit (127); ++ _exit (127); + dup2 (fd, STDIN_FILENO); + close (fd); + } +@@ -108,7 +108,7 @@ grub_util_exec_redirect_all (const char *const *argv, const char *stdin_file, + { + fd = open (stdout_file, O_WRONLY | O_CREAT, 0700); + if (fd < 0) +- exit (127); ++ _exit (127); + dup2 (fd, STDOUT_FILENO); + close (fd); + } +@@ -117,7 +117,7 @@ grub_util_exec_redirect_all (const char *const *argv, const char *stdin_file, + { + fd = open (stderr_file, O_WRONLY | O_CREAT, 0700); + if (fd < 0) +- exit (127); ++ _exit (127); + dup2 (fd, STDERR_FILENO); + close (fd); + } +@@ -126,7 +126,7 @@ grub_util_exec_redirect_all (const char *const *argv, const char *stdin_file, + setenv ("LC_ALL", "C", 1); + + execvp ((char *) argv[0], (char **) argv); +- exit (127); ++ _exit (127); + } + waitpid (pid, &status, 0); + if (!WIFEXITED (status)) diff -Nru grub2-2.02+dfsg1/debian/patches/grub-install-backup-and-restore.patch grub2-2.02+dfsg1/debian/patches/grub-install-backup-and-restore.patch --- grub2-2.02+dfsg1/debian/patches/grub-install-backup-and-restore.patch 1970-01-01 01:00:00.000000000 +0100 +++ grub2-2.02+dfsg1/debian/patches/grub-install-backup-and-restore.patch 2020-11-29 16:53:52.000000000 +0000 @@ -0,0 +1,175 @@ +From cdc5efa6be1dbb8a4344f9c000eb5c99983c2225 Mon Sep 17 00:00:00 2001 +From: Dimitri John Ledkov <x...@ubuntu.com> +Date: Wed, 19 Aug 2020 01:49:09 +0100 +Subject: grub-install: Add backup and restore + +Refactor clean_grub_dir to create a backup of all the files, instead +of just irrevocably removing them as the first action. If available, +register on_exit handle to restore the backup if any errors occur, or +remove the backup if everything was successful. If on_exit is not +available, the backup remains on disk for manual recovery. + +This allows safer upgrades of MBR & modules, such that +modules/images/fonts/translations are consistent with MBR in case of +errors. For example accidental grub-install /dev/non-existent-disk +currently clobbers and upgrades modules in /boot/grub, despite not +actually updating any MBR. This increases peak disk-usage slightly, by +requiring temporarily twice the disk space to complete grub-install. + +Also add modinfo.sh to the cleanup/backup/restore codepath, to ensure +it is also cleaned / backed up / restored. + +Signed-off-by: Dimitri John Ledkov <x...@ubuntu.com> + +Patch-Name: grub-install-backup-and-restore.patch +--- + configure.ac | 2 +- + util/grub-install-common.c | 105 +++++++++++++++++++++++++++++++------ + 2 files changed, 91 insertions(+), 16 deletions(-) + +diff --git a/configure.ac b/configure.ac +index 3b97f2ba1..56183dc3f 100644 +--- a/configure.ac ++++ b/configure.ac +@@ -393,7 +393,7 @@ else + fi + + # Check for functions and headers. +-AC_CHECK_FUNCS(posix_memalign memalign getextmntent) ++AC_CHECK_FUNCS(posix_memalign memalign getextmntent on_exit) + AC_CHECK_HEADERS(sys/param.h sys/mount.h sys/mnttab.h limits.h) + + # glibc 2.25 still includes sys/sysmacros.h in sys/types.h but emits deprecation +diff --git a/util/grub-install-common.c b/util/grub-install-common.c +index f59752d44..7ac25fcef 100644 +--- a/util/grub-install-common.c ++++ b/util/grub-install-common.c +@@ -184,38 +184,113 @@ grub_install_mkdir_p (const char *dst) + free (t); + } + ++static int ++strcmp_ext (const char *a, const char *b, const char *ext) ++{ ++ char *bsuffix = grub_util_path_concat_ext (1, b, ext); ++ int r = strcmp (a, bsuffix); ++ free (bsuffix); ++ return r; ++} ++ ++enum clean_grub_dir_mode ++{ ++ CLEAN = 0, ++ CLEAN_BACKUP = 1, ++ CREATE_BACKUP = 2, ++ RESTORE_BACKUP = 3, ++}; ++ + static void +-clean_grub_dir (const char *di) ++clean_grub_dir_real (const char *di, enum clean_grub_dir_mode mode) + { + grub_util_fd_dir_t d; + grub_util_fd_dirent_t de; ++ char suffix[2] = ""; ++ ++ if ((mode == CLEAN_BACKUP) || (mode == RESTORE_BACKUP)) ++ { ++ strcpy (suffix, "-"); ++ } + + d = grub_util_fd_opendir (di); + if (!d) +- grub_util_error (_("cannot open directory `%s': %s"), +- di, grub_util_fd_strerror ()); ++ { ++ if (mode == CLEAN_BACKUP) ++ return; ++ grub_util_error (_("cannot open directory `%s': %s"), ++ di, grub_util_fd_strerror ()); ++ } + + while ((de = grub_util_fd_readdir (d))) + { + const char *ext = strrchr (de->d_name, '.'); +- if ((ext && (strcmp (ext, ".mod") == 0 +- || strcmp (ext, ".lst") == 0 +- || strcmp (ext, ".img") == 0 +- || strcmp (ext, ".mo") == 0) +- && strcmp (de->d_name, "menu.lst") != 0) +- || strcmp (de->d_name, "efiemu32.o") == 0 +- || strcmp (de->d_name, "efiemu64.o") == 0) ++ if ((ext && (strcmp_ext (ext, ".mod", suffix) == 0 ++ || strcmp_ext (ext, ".lst", suffix) == 0 ++ || strcmp_ext (ext, ".img", suffix) == 0 ++ || strcmp_ext (ext, ".mo", suffix) == 0) ++ && strcmp_ext (de->d_name, "menu.lst", suffix) != 0) ++ || strcmp_ext (de->d_name, "modinfo.sh", suffix) == 0 ++ || strcmp_ext (de->d_name, "efiemu32.o", suffix) == 0 ++ || strcmp_ext (de->d_name, "efiemu64.o", suffix) == 0) + { +- char *x = grub_util_path_concat (2, di, de->d_name); +- if (grub_util_unlink (x) < 0) +- grub_util_error (_("cannot delete `%s': %s"), x, +- grub_util_fd_strerror ()); +- free (x); ++ char *srcf = grub_util_path_concat (2, di, de->d_name); ++ ++ if (mode == CREATE_BACKUP) ++ { ++ char *dstf = grub_util_path_concat_ext (2, di, de->d_name, "-"); ++ if (grub_util_rename (srcf, dstf) < 0) ++ grub_util_error (_("cannot backup `%s': %s"), srcf, ++ grub_util_fd_strerror ()); ++ free (dstf); ++ } ++ else if (mode == RESTORE_BACKUP) ++ { ++ char *dstf = grub_util_path_concat_ext (2, di, de->d_name); ++ dstf[strlen (dstf) - 1] = 0; ++ if (grub_util_rename (srcf, dstf) < 0) ++ grub_util_error (_("cannot restore `%s': %s"), dstf, ++ grub_util_fd_strerror ()); ++ free (dstf); ++ } ++ else ++ { ++ if (grub_util_unlink (srcf) < 0) ++ grub_util_error (_("cannot delete `%s': %s"), srcf, ++ grub_util_fd_strerror ()); ++ } ++ free (srcf); + } + } + grub_util_fd_closedir (d); + } + ++static void ++restore_backup_on_exit (int status, void *arg) ++{ ++ if (status == 0) ++ { ++ clean_grub_dir_real ((char *) arg, CLEAN_BACKUP); ++ } ++ else ++ { ++ clean_grub_dir_real ((char *) arg, CLEAN); ++ clean_grub_dir_real ((char *) arg, RESTORE_BACKUP); ++ } ++ free (arg); ++ arg = NULL; ++} ++ ++static void ++clean_grub_dir (const char *di) ++{ ++ clean_grub_dir_real (di, CLEAN_BACKUP); ++ clean_grub_dir_real (di, CREATE_BACKUP); ++#if defined(HAVE_ON_EXIT) ++ on_exit (restore_backup_on_exit, strdup (di)); ++#endif ++} ++ + struct install_list + { + int is_default; diff -Nru grub2-2.02+dfsg1/debian/patches/series grub2-2.02+dfsg1/debian/patches/series --- grub2-2.02+dfsg1/debian/patches/series 2020-07-30 18:15:27.000000000 +0100 +++ grub2-2.02+dfsg1/debian/patches/series 2020-11-29 16:53:52.000000000 +0000 @@ -168,3 +168,5 @@ unix-config-overflow.patch deviceiter-overflow.patch efi-malformed-device-path-2.patch +exec-avoid-atexit-when-child-exits.patch +grub-install-backup-and-restore.patch diff -Nru grub2-2.02+dfsg1/debian/postinst.in grub2-2.02+dfsg1/debian/postinst.in --- grub2-2.02+dfsg1/debian/postinst.in 2020-07-30 18:15:23.000000000 +0100 +++ grub2-2.02+dfsg1/debian/postinst.in 2020-11-29 16:54:38.000000000 +0000 @@ -501,7 +501,7 @@ elif running_in_container; then # Skip grub-install in containers. : - elif test -z "$2" || test -e /boot/grub/core.img || \ + elif test -e /boot/grub/core.img || \ test -e /boot/grub/@FIRST_CPU_PLATFORM@/core.img || \ test "$UPGRADE_FROM_GRUB_LEGACY" || test "$wubi_device"; then question=grub-pc/install_devices @@ -570,7 +570,10 @@ failed_devices= for i in `echo $RET | sed -e 's/, / /g'` ; do real_device="$(readlink -f "$i")" - if grub-install --target=i386-pc --force --no-floppy $real_device ; then + if [ ! -e "$real_device" ]; then + echo "$real_device does not exist, so cannot grub-install to it!" >&2 + failed_devices="$failed_devices $real_device" + elif grub-install --target=i386-pc --force --no-floppy $real_device ; then # We just installed GRUB 2; then also generate grub.cfg. touch /boot/grub/grub.cfg else @@ -605,6 +608,9 @@ exit 1 fi else + echo "You must correct your GRUB install devices before proceeding:" >&2 + echo >&2 + echo " dpkg-reconfigure grub-pc" >&2 exit 1 # noninteractive fi else @@ -621,7 +627,10 @@ continue fi else - break # noninteractive + echo "You must correct your GRUB install devices before proceeding:" >&2 + echo >&2 + echo " dpkg-reconfigure grub-pc" >&2 + exit 1 # noninteractive fi fi fi @@ -644,7 +653,15 @@ db_fset grub-pc/install_devices_empty seen false fi else - break # noninteractive + db_get grub-pc/install_devices_empty + if [ "$RET" = true ]; then + break + else + echo "You must correct your GRUB install devices before proceeding:" >&2 + echo >&2 + echo " dpkg-reconfigure grub-pc" >&2 + exit 1 # noninteractive + fi fi else break
