shim: don't set second_stage to the empty string #640
Merged
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.
When LoadOptions is either L" " or L"shim.efi ", parse_load_options sets second_stage to the empty string. This is unlikely to be what is intended, and typically leads to a non-obvious failure mode.
The failure happens because parse_load_options's call to split_load_options (after eating shim's own filename, if present) returns the empty string. Since init_grub typically passes second_stage to start_image, this causes read_image to concatenate the empty string onto the directory name. This means PathName refers to the directory, not the path to a pe image. Then load_image successfully opens a handle on the directory and reads "data" from it. It only eventually fails when handle_image calls read_header which finds that this data isn't in fact a pe header, reporting "Invalid image".
This scenario has been seen when shim is loaded via rEFInd 0.11.5, which sets LoadOptions to the name of the shim program followed by a space character.
Instead, modify parse_load_options to leave second_stage set to its default value rather than the empty string.