From 7727f381455a6283def7668b33199dff2555713a 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 014132703d..3d26a3fe41 100644 --- a/grub-core/script/execute.c +++ b/grub-core/script/execute.c @@ -952,7 +952,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 c97b78d77f..9c62110424 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 From fb595c30f261f6f1a8680672c26d223e384cde02 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 97687013c9..a2703dabb1 100644 --- a/grub-core/normal/menu.c +++ b/grub-core/normal/menu.c @@ -377,14 +377,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);