Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Freshly compiled Shim-15.7 (x86_64) cannot load grubx64.efi #624

Open
Jurij-Ivastsuk opened this issue Dec 20, 2023 · 11 comments
Open

Freshly compiled Shim-15.7 (x86_64) cannot load grubx64.efi #624

Jurij-Ivastsuk opened this issue Dec 20, 2023 · 11 comments

Comments

@Jurij-Ivastsuk
Copy link

Freshly compiled Shim-15.7 (x86_64) cannot load grubx64.efi, only after "falling back to default loader" grubx64.efi can be loaded.
Does anyone know the cause? How can this problem be solved?

IMG_5120

Thanks in advance!
Jurij

@Jurij-Ivastsuk
Copy link
Author

Jurij-Ivastsuk commented Dec 21, 2023

Here is some additional information:
I have checked shim.c and the following is the result:
in function: shim_init(void)
in function: EFI_STATUS set_second_stage (EFI_HANDLE image_handle)
second_stage is set to "\grubx64.efi"
but
in function: efi_status = init_grub(image_handle);
EFI_STATUS init_grub(EFI_HANDLE image_handle)
second_stage - no longer has a value !!! The value "\grubx64.efi" has been lost...
second_stage is defined in: load-options.c
CHAR16 *second_stage;
Compiled with gcc-10 (10.2.0-19), g++-10 (10.2.0-19), gnu-efi (3.0.9-2)

Therefore, I believe that this is the cause of this problem. But why does second_stage lose its value? The value "\grubx64.efi" is set earlier, before the execution of init_grub()...

@Jurij-Ivastsuk
Copy link
Author

The solution was found. Please have a look here: https://github.com/rhboot/shim/pull/625

@AndrewSav
Copy link

AndrewSav commented Dec 26, 2023

@Jurij-Ivastsuk did you manage to understand what is replacing second_stage or why it is being replaced? From the comment on your PR it looks like an interaction with some external entity is assumed which is writing the value of this variable and the shim does not write this value itself. I'm curious what that external entity would be?

@Jurij-Ivastsuk
Copy link
Author

@AndrewSav My PR 625 was rejected, and that is correct. The first attempt was pretty quick but not a good solution for everyone. But pointing out the weakness of this solution helped me move forward. I think I have actually localized the problem. I still need to do a lot of testing to be able to say that this would actually be a solution for everyone.

@Jurij-Ivastsuk
Copy link
Author

@AndrewSav We have localized the problem. The overwriting of the value for second_stage takes place in the function parse_load_options() in the file load-options.c Because the function split_load_options() returns a non-printable ascii character, the originally set value \grubx64.efi is overwritten by this character. You can view the problem description and the solution here:
[https://github.com//pull/626]
I hope that this time my pull request will not be rejected.

@Jurij-Ivastsuk
Copy link
Author

After some research I found a similar solution. It starts at exactly the same point as I did and checks whether the value of this variable represents a valid path before overwriting the variable "second_stage":
https://github.com/rhboot/shim/commit/36adff84a8251f0593e2c524268116c05688170b

Various checks are implemented there, but the check whether the first character contains a non-printable ASCII character is omitted. If you are looking for a general solution to this problem, perhaps a combination of both solutions: mine and the one just mentioned would be a better solution that takes several eventualities into account. But in my opinion, one should make a fundamental consideration whether the function parse_load_options() has any justification at all.

@mikebeaton
Copy link
Contributor

mikebeaton commented Jan 3, 2024

It's usually not good practice to post comments twice - as you're ofc aware, your issue is discussed at both places you've posted, and probably other people reading this who are more or less up to speed on the issue are aware of that too. (So basically, people who care much get your message twice - and have to scroll through it twice in future, if scrolling through both threads - and only people who don't care much see it maybe once or never!) Thx. (Feel free to just delete one, if on reflection you feel this comment is justified - and I'll delete this! :-) )

@Jurij-Ivastsuk
Copy link
Author

@mikebeaton Thanks for the tip. According to my consideration, there are different readers: some who only participate in discussions and others like the reviewers who need more basic information to judge whether the proposed solution is good or not. To inform both groups I posted the information on both channels. Only in the latter case the text is identical.

@AndrewSav
Copy link

It does not annoy me personally, if it is posted twice. I can skip if I have read it before. I agree that the audiences might not cross over in parts. As long as it is not every single message duplicated it's fine IMO.

@ghost
Copy link

ghost commented Feb 1, 2024

Of course it got rejected. Because people are completely stupid.

@Jurij-Ivastsuk
Copy link
Author

@PC-Doctor666 Hello, sorry, but your comments are unfortunately not very constructive.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants