loader-protocol: add workaround for EDK2 2025.02 page fault on FreePages #750
Conversation
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
Filed https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1105209 |
vathpela
reviewed
May 13, 2025
vathpela
approved these changes
May 13, 2025
4 tasks
|
Issue raised on EDK2 repo: tianocore/edk2#11074 |
EDK2 since version 2025.02 introduced the EFI memory attribute protocol: tianocore/edk2@efaa102 This is used by shim to unset the writable bit and set the executable bit on images when they are loaded. EDK2 also (at least in the Debian/Ubuntu production builds) overwrites pages with a fixed 0xaf pattern when BS->FreePages() is called: https://github.com/tianocore/edk2/blob/399a40e5cba2ed70197ac61c8da9cf9805fb918e/MdePkg/Library/BaseDebugLibSerialPort/DebugLib.c#L256 https://github.com/tianocore/edk2/blob/399a40e5cba2ed70197ac61c8da9cf9805fb918e/MdePkg/Library/UefiDebugLibConOut/DebugLib.c#L256 https://github.com/tianocore/edk2/blob/399a40e5cba2ed70197ac61c8da9cf9805fb918e/MdePkg/Library/UefiDebugLibStdErr/DebugLib.c#L256 These two properties mix together as well as one may image: systemd-stub@0x72c64000 v999-bluca FSOpen: Open '\loader\addons' Success FSOpen: Open '\loader\addons\1.addon.efi' Success InstallProtocolInterface: 6E6BAEB8-7108-4179-949D-A3493415EC97 7D368D18 InstallProtocolInterface: 5B1B31A1-9562-11D2-8E3F-00A0C969723B 7D368D18 InstallProtocolInterface: BC62157E-3E33-4FEC-9920-2D3B36D750DF 7D376D18 !!!! X64 Exception Type - 0E(#PF - Page-Fault) CPU Apic ID - 00000000 !!!! ExceptionData - 0000000000000003 I:0 R:0 U:0 W:1 P:1 PK:0 SS:0 SGX:0 RIP - 000000007EF0931A, CS - 0000000000000038, RFLAGS - 0000000000010206 RAX - 00000000000000AF, RCX - 0000000000005000, RDX - 000000007D352000 RBX - 000000007D352000, RSP - 000000007EEEAAF0, RBP - 000000007EEEAB80 RSI - 000000007EF10F20, RDI - 000000007D353000 R8 - 00000000000000AF, R9 - 000000007D357FFF, R10 - 0000000072C7C2F7 R11 - 0000000001CBD9D0, R12 - 000000007D357FFF, R13 - 000000007D358000 R14 - 000000007D35B300, R15 - 000000007D357FFF DS - 0000000000000030, ES - 0000000000000030, FS - 0000000000000030 GS - 0000000000000030, SS - 0000000000000030 CR0 - 0000000080010033, CR2 - 000000007D353000, CR3 - 000000007EC01000 CR4 - 0000000000000668, CR8 - 0000000000000000 DR0 - 0000000000000000, DR1 - 0000000000000000, DR2 - 0000000000000000 DR3 - 0000000000000000, DR6 - 00000000FFFF0FF0, DR7 - 0000000000000400 GDTR - 000000007E9E1000 0000000000000047, LDTR - 0000000000000000 IDTR - 000000007E407018 0000000000000FFF, TR - 0000000000000000 FXSAVE_STATE - 000000007EEEA750 !!!! Find image based on IP(0x7EF0931A) /home/bluca/git/edk2/Build/OvmfX64/DEBUG_GCC5/X64/MdeModulePkg/Core/Dxe/DxeMain/DEBUG/DxeCore.dll (ImageBase=000000007EEEC000, EntryPoint=000000007EF026FD) !!!! This issue can be reproduced by simply calling BS->UnloadImage() on any image that was loaded by shim's BS->LoadImage(). The systemd stub used in UKIs does so while loading and parsing addons. While this is arguably a bug in EDK2 (it should check that pages are writable before attempting to write them) and in the Debian/Ubuntu build (these debug modules should be swapped out for the BaseDebugLibNull module where this is a no-op: https://github.com/tianocore/edk2/blob/399a40e5cba2ed70197ac61c8da9cf9805fb918e/MdePkg/Library/BaseDebugLibNull/DebugLib.c#L137 ), this is how things work currently in at least two distribution stable releases, so add a workaround and set W+ X- before calling BS->FreePages(). Signed-off-by: Luca Boccassi <[email protected]>
Merged
Ensure we don't leave the image protocol installed if there is a failure (e.g.: security violation) in the return variable that the user gave us Follow-up for 774f226 Signed-off-by: Luca Boccassi <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
EDK2 since version 2025.02 introduced the EFI memory attribute protocol:
tianocore/edk2@efaa102
This is used by shim to unset the writable bit and set the executable bit on images when they are loaded.
EDK2 also (at least in the Debian/Ubuntu production builds) overwrites pages with a fixed 0xaf pattern when BS->FreePages() is called:
https://github.com/tianocore/edk2/blob/399a40e5cba2ed70197ac61c8da9cf9805fb918e/MdePkg/Library/BaseDebugLibSerialPort/DebugLib.c#L256 https://github.com/tianocore/edk2/blob/399a40e5cba2ed70197ac61c8da9cf9805fb918e/MdePkg/Library/UefiDebugLibConOut/DebugLib.c#L256 https://github.com/tianocore/edk2/blob/399a40e5cba2ed70197ac61c8da9cf9805fb918e/MdePkg/Library/UefiDebugLibStdErr/DebugLib.c#L256
These two properties mix together as well as one may image:
This issue can be reproduced by simply calling BS->UnloadImage() on any image that was loaded by shim's BS->LoadImage(). The systemd stub used in UKIs does so while loading and parsing addons.
While this is arguably a bug in EDK2 (it should check that pages are writable before attempting to write them) and in the Debian/Ubuntu build (these debug modules should be swapped out for the BaseDebugLibNull module where this is a no-op: https://github.com/tianocore/edk2/blob/399a40e5cba2ed70197ac61c8da9cf9805fb918e/MdePkg/Library/BaseDebugLibNull/DebugLib.c#L137 ), this is how things work currently in at least two distribution stable releases, so add a workaround and set W+ X- before calling BS->FreePages().