From adffe94b10fe09ea1940424c53dc80957b65aa9b Mon Sep 17 00:00:00 2001 From: James Le Cuirot Date: Thu, 24 Oct 2024 18:09:25 +0100 Subject: [PATCH] sys-boot/grub: Fix fallback mechanism broken by Red Hat's patches This fix has been submitted to Red Hat. It will hopefully be merged soon. Signed-off-by: James Le Cuirot --- .../user-patches/sys-boot/grub/README.md | 5 + .../grub-2.12-01-execute-return-code.patch | 112 ++++++++++++++++++ 2 files changed, 117 insertions(+) create mode 100644 sdk_container/src/third_party/coreos-overlay/coreos/user-patches/sys-boot/grub/grub-2.12-01-execute-return-code.patch diff --git a/sdk_container/src/third_party/coreos-overlay/coreos/user-patches/sys-boot/grub/README.md b/sdk_container/src/third_party/coreos-overlay/coreos/user-patches/sys-boot/grub/README.md index 1c91274a09f..14d706c9663 100644 --- a/sdk_container/src/third_party/coreos-overlay/coreos/user-patches/sys-boot/grub/README.md +++ b/sdk_container/src/third_party/coreos-overlay/coreos/user-patches/sys-boot/grub/README.md @@ -23,6 +23,11 @@ functionality, and the other is for extracting the verity root hash from the initrd. Gentoo's upstream ebuild is used, but Gentoo's patches are discarded because they conflict and are not relevant to Flatcar. +Finally, another patch is applied to fix the fallback mechanism, which was +accidentally broken by Red Hat's patches. This has been submitted to Red Hat in +[rhboot/grub2#195](https://github.com/rhboot/grub2/pull/195). It will hopefully +be merged soon. + ## How to import the Red Hat patches Red Hat maintains a fork of GRUB on GitHub with branches for each Fedora release. Generate a diff between the latest upstream release and the latest Fedora branch. diff --git a/sdk_container/src/third_party/coreos-overlay/coreos/user-patches/sys-boot/grub/grub-2.12-01-execute-return-code.patch b/sdk_container/src/third_party/coreos-overlay/coreos/user-patches/sys-boot/grub/grub-2.12-01-execute-return-code.patch new file mode 100644 index 00000000000..fe8bfef6ddf --- /dev/null +++ b/sdk_container/src/third_party/coreos-overlay/coreos/user-patches/sys-boot/grub/grub-2.12-01-execute-return-code.patch @@ -0,0 +1,112 @@ +From 442b676110c35caedb57408c4a27a6ef74f4c7db Mon Sep 17 00:00:00 2001 +From: James Le Cuirot +Date: Thu, 24 Oct 2024 14:42:46 +0100 +Subject: [PATCH 1/2] script/execute: Don't let trailing blank lines determine + the return code + +grub_script_execute_sourcecode() parses and executes code one line at a +time, updating the return code each time because only the last line +determines the final status. However, trailing new lines were also +executed, masking any failure on the previous line. Fix this by only +trying to execute the command when there is actually one present. + +This has presumably never been noticed because this code is not used by +regular functions, only in special cases like eval and menu entries. The +latter generally don't return at all, having booted an OS. When failing +to boot, upstream GRUB triggers the fallback mechanism regardless of the +return code. + +We noticed the problem while using Red Hat's patches, which change this +behaviour to take account of the return code. In that case, a failure +takes you back to the menu rather than triggering a fallback. + +Signed-off-by: James Le Cuirot +--- + grub-core/script/execute.c | 5 ++++- + tests/grub_script_eval.in | 10 +++++++++- + 2 files changed, 13 insertions(+), 2 deletions(-) + +diff --git a/grub-core/script/execute.c b/grub-core/script/execute.c +index c19b4bf70..e369e8318 100644 +--- a/grub-core/script/execute.c ++++ b/grub-core/script/execute.c +@@ -935,7 +935,10 @@ grub_script_execute_sourcecode (const char *source) + break; + } + +- ret = grub_script_execute (parsed_script); ++ /* Don't let trailing blank lines determine the return code. */ ++ if (parsed_script->cmd) ++ ret = grub_script_execute (parsed_script); ++ + grub_script_free (parsed_script); + grub_free (line); + } +diff --git a/tests/grub_script_eval.in b/tests/grub_script_eval.in +index c97b78d77..9c6211042 100644 +--- a/tests/grub_script_eval.in ++++ b/tests/grub_script_eval.in +@@ -3,4 +3,12 @@ + eval echo "Hello world" + valname=tst + eval $valname=hi +-echo $tst +\ No newline at end of file ++echo $tst ++ ++if eval " ++false ++"; then ++ echo should have failed ++else ++ echo failed as expected ++fi +-- +2.46.1 + + +From 93eedb6cebe758db09066e084ddd2fb659ebc4fb Mon Sep 17 00:00:00 2001 +From: James Le Cuirot +Date: Thu, 24 Oct 2024 15:00:26 +0100 +Subject: [PATCH 2/2] normal/menu: Check return code of the script when + executing a menu entry + +Don't rely on grub_errno here because grub_script_execute_new_scope() +calls grub_print_error(), which always resets grub_errno back to +GRUB_ERR_NONE. It may also get reset by grub_wait_after_message(). + +This problem was observed when a "bad signature" error resulted in the +menu being redisplayed rather than the fallback mechanism being +triggered, although another change was also needed to fix it. This only +happens with Red Hat's patches because upstream GRUB triggers the +fallback mechanism regardless of the return code. + +Signed-off-by: James Le Cuirot +--- + grub-core/normal/menu.c | 4 ++-- + 1 file changed, 2 insertions(+), 2 deletions(-) + +diff --git a/grub-core/normal/menu.c b/grub-core/normal/menu.c +index cda10fa8b..fd9b8c6d8 100644 +--- a/grub-core/normal/menu.c ++++ b/grub-core/normal/menu.c +@@ -376,14 +376,14 @@ grub_menu_execute_entry(grub_menu_entry_t entry, int auto_boot) + if (ptr && ptr[0] && ptr[1]) + grub_env_set ("default", ptr + 1); + +- grub_script_execute_new_scope (entry->sourcecode, entry->argc, entry->args); ++ err = grub_script_execute_new_scope (entry->sourcecode, entry->argc, entry->args); + + if (errs_before != grub_err_printed_errors) + grub_wait_after_message (); + + errs_before = grub_err_printed_errors; + +- if (grub_errno == GRUB_ERR_NONE && grub_loader_is_loaded ()) ++ if (err == GRUB_ERR_NONE && grub_loader_is_loaded ()) + /* Implicit execution of boot, only if something is loaded. */ + err = grub_command_execute ("boot", 0, 0); + +-- +2.46.1 +