Fix the fallback mechanism when menu entries fail to boot#195
Closed
chewi wants to merge 2 commits intorhboot:fedora-43from
Closed
Fix the fallback mechanism when menu entries fail to boot#195chewi wants to merge 2 commits intorhboot:fedora-43from
chewi wants to merge 2 commits intorhboot:fedora-43from
Conversation
2 tasks
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 <jlecuirot@microsoft.com>
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 <jlecuirot@microsoft.com>
93eedb6 to
fb595c3
Compare
Contributor
Author
|
Rebased on fedora-43. Upstream has merged the first change now, but you don't have it yet. |
Contributor
Contributor
Author
|
Done. Hope I got that right. |
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
@martinezjavier's c3fcfe5 commit accidentally broke the fallback mechanism, although it also exposed another bug in upstream GRUB.
We cannot rely on
grub_errnoingrub_menu_execute_entry()becausegrub_script_execute_new_scope()callsgrub_print_error(), which always resetsgrub_errnoback toGRUB_ERR_NONE. It may also get reset bygrub_wait_after_message().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. A new test case is included.See the commit messages for further details.
I can send the first change upstream, although it's unlikely to cause issues without your change, hence why it went unnoticed for so long. I gather you are starting to upstream some of your changes, so perhaps I can leave it with you?