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

[Jurij-Ivastsuk]

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?

Thanks in advance!
Jurij

[Jurij-Ivastsuk]

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]

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

[AndrewSav]

@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]

@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]

@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]

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]

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]

@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]

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.

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

[Jurij-Ivastsuk]

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

[Metabolix]

I suppose your issue was discussed in #626 and was found to be due to missing build option DISABLE_REMOVABLE_LOAD_OPTIONS=1, so this should be closed?

[Jurij-Ivastsuk]

Yes, I will close this issue.