-
Notifications
You must be signed in to change notification settings - Fork 52
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
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 <[email protected]>
- Loading branch information
Showing
2 changed files
with
117 additions
and
0 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
112 changes: 112 additions & 0 deletions
112
...y/coreos-overlay/coreos/user-patches/sys-boot/grub/grub-2.12-01-execute-return-code.patch
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,112 @@ | ||
From 442b676110c35caedb57408c4a27a6ef74f4c7db Mon Sep 17 00:00:00 2001 | ||
From: James Le Cuirot <[email protected]> | ||
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 <[email protected]> | ||
--- | ||
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 <[email protected]> | ||
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 <[email protected]> | ||
--- | ||
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 | ||
|