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.

Sign up for GitHub

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

Jump to bottom

Second-stage's loaded image ImageSize is wrong on IA32 #515

Closed
nicholasbishop opened this issue Oct 6, 2022 · 0 comments · Fixed by #516
Closed

Second-stage's loaded image ImageSize is wrong on IA32 #515

nicholasbishop opened this issue Oct 6, 2022 · 0 comments · Fixed by #516

Comments

@nicholasbishop
Copy link
Contributor

In pe.c, it sets the ImageBase and ImageSize in EFI_LOADED_IMAGE for the second stage: https://github.com/rhboot/shim/blob/main/pe.c#L1394

On the IA32 target, this doesn't work properly because the ImageSize field is not correctly aligned. ImageSize is a UINT64. Fields are supposed to be "naturally aligned". Naturally the spec doesn't bother to specify exactly what "naturally aligned" means, but other toolchains such as EDK2 seem to think it means a UINT64 should be 8-byte aligned.

In other words, offsetof(EFI_LOADED_IMAGE, ImageSize) on IA32 is 40 in EDK2, but 36 in shim due to the missing padding.

I think a straightforward fix would be to add -malign-double on IA32 (as EDK2 does), which will align "double, long double, and long long variables on a two-word boundary".
nicholasbishop added a commit to nicholasbishop/shim that referenced this issue Oct 6, 2022
@nicholasbishop
Add -malign-double to IA32 compiler flags
e272afe 
This changes the alignment of UINT64 data to 8 bytes on IA32, which
matches EDK2's understanding of alignment. In particular this change
affects the offset where shim writes `EFI_LOADED_IMAGE.ImageSize`.

Fixes rhboot#515

Signed-off-by: Nicholas Bishop <[email protected]>
@nicholasbishop nicholasbishop mentioned this issue Oct 6, 2022
Merged
vathpela pushed a commit that referenced this issue Nov 14, 2022
@nicholasbishop @vathpela
Add -malign-double to IA32 compiler flags
0cf43ac 
This changes the alignment of UINT64 data to 8 bytes on IA32, which
matches EDK2's understanding of alignment. In particular this change
affects the offset where shim writes `EFI_LOADED_IMAGE.ImageSize`.

Fixes #515

Signed-off-by: Nicholas Bishop <[email protected]>
brianredbeard pushed a commit to brianredbeard/redhat-efi-boot-shim that referenced this issue Feb 22, 2024
@nicholasbishop @brianredbeard
Add -malign-double to IA32 compiler flags
04bb3a6 
This changes the alignment of UINT64 data to 8 bytes on IA32, which
matches EDK2's understanding of alignment. In particular this change
affects the offset where shim writes `EFI_LOADED_IMAGE.ImageSize`.

Fixes rhboot#515

Signed-off-by: Nicholas Bishop <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Add -malign-double to IA32 compiler flags nicholasbishop/shim
1 participant
@nicholasbishop