-
Notifications
You must be signed in to change notification settings - Fork 741
[lib][uefi] Fix memory leak in load_pe_file #468
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
Changes from all commits
bc39f4e
6416056
1aed710
f947ab7
f103ce0
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -7,102 +7,6 @@ | |||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| #include "pe.h" | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| namespace { | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| constexpr size_t BIT26 = 1 << 26; | ||||||||||||||||||||||||||||||||||||
| constexpr size_t BIT11 = 1 << 11; | ||||||||||||||||||||||||||||||||||||
| constexpr size_t BIT10 = 1 << 10; | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||||||||||||
| Pass in a pointer to an ARM MOVT or MOVW immediate instruciton and | ||||||||||||||||||||||||||||||||||||
| return the immediate data encoded in the instruction. | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| @param Instruction Pointer to ARM MOVT or MOVW immediate instruction | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| @return Immediate address encoded in the instruction | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| **/ | ||||||||||||||||||||||||||||||||||||
| uint16_t ThumbMovtImmediateAddress(const uint16_t *Instruction) { | ||||||||||||||||||||||||||||||||||||
| uint32_t Movt{}; | ||||||||||||||||||||||||||||||||||||
| uint16_t Address{}; | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| // Thumb2 is two 16-bit instructions working together. Not a single 32-bit | ||||||||||||||||||||||||||||||||||||
| // instruction Example MOVT R0, #0 is 0x0000f2c0 or 0xf2c0 0x0000 | ||||||||||||||||||||||||||||||||||||
| Movt = (*Instruction << 16) | (*(Instruction + 1)); | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| // imm16 = imm4:i:imm3:imm8 | ||||||||||||||||||||||||||||||||||||
| // imm4 -> Bit19:Bit16 | ||||||||||||||||||||||||||||||||||||
| // i -> Bit26 | ||||||||||||||||||||||||||||||||||||
| // imm3 -> Bit14:Bit12 | ||||||||||||||||||||||||||||||||||||
| // imm8 -> Bit7:Bit0 | ||||||||||||||||||||||||||||||||||||
| Address = static_cast<uint16_t>(Movt & 0x000000ff); // imm8 | ||||||||||||||||||||||||||||||||||||
| Address |= static_cast<uint16_t>((Movt >> 4) & 0x0000f700); // imm4 imm3 | ||||||||||||||||||||||||||||||||||||
| Address |= (((Movt & BIT26) != 0) ? BIT11 : 0); // i | ||||||||||||||||||||||||||||||||||||
| return Address; | ||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||||||||||||
| Pass in a pointer to an ARM MOVW/MOVT instruciton pair and | ||||||||||||||||||||||||||||||||||||
| return the immediate data encoded in the two` instruction. | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| @param Instructions Pointer to ARM MOVW/MOVT insturction pair | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| @return Immediate address encoded in the instructions | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| **/ | ||||||||||||||||||||||||||||||||||||
| uint32_t ThumbMovwMovtImmediateAddress(uint16_t *Instructions) { | ||||||||||||||||||||||||||||||||||||
| uint16_t *Word{}; | ||||||||||||||||||||||||||||||||||||
| uint16_t *Top{}; | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| Word = Instructions; // MOVW | ||||||||||||||||||||||||||||||||||||
| Top = Word + 2; // MOVT | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| return (ThumbMovtImmediateAddress(Top) << 16) + | ||||||||||||||||||||||||||||||||||||
| ThumbMovtImmediateAddress(Word); | ||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||||||||||||
| Update an ARM MOVT or MOVW immediate instruction immediate data. | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| @param Instruction Pointer to ARM MOVT or MOVW immediate instruction | ||||||||||||||||||||||||||||||||||||
| @param Address New addres to patch into the instruction | ||||||||||||||||||||||||||||||||||||
| **/ | ||||||||||||||||||||||||||||||||||||
| void ThumbMovtImmediatePatch(uint16_t *Instruction, uint16_t Address) { | ||||||||||||||||||||||||||||||||||||
| uint16_t Patch{}; | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| // First 16-bit chunk of instruciton | ||||||||||||||||||||||||||||||||||||
| Patch = ((Address >> 12) & 0x000f); // imm4 | ||||||||||||||||||||||||||||||||||||
| Patch |= (((Address & BIT11) != 0) ? BIT10 : 0); // i | ||||||||||||||||||||||||||||||||||||
| // Mask out instruction bits and or in address | ||||||||||||||||||||||||||||||||||||
| *(Instruction) = (*Instruction & ~0x040f) | Patch; | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| // Second 16-bit chunk of instruction | ||||||||||||||||||||||||||||||||||||
| Patch = Address & 0x000000ff; // imm8 | ||||||||||||||||||||||||||||||||||||
| Patch |= ((Address << 4) & 0x00007000); // imm3 | ||||||||||||||||||||||||||||||||||||
| // Mask out instruction bits and or in address | ||||||||||||||||||||||||||||||||||||
| Instruction++; | ||||||||||||||||||||||||||||||||||||
| *Instruction = (*Instruction & ~0x70ff) | Patch; | ||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||||||||||||
| Update an ARM MOVW/MOVT immediate instruction instruction pair. | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| @param Instructions Pointer to ARM MOVW/MOVT instruction pair | ||||||||||||||||||||||||||||||||||||
| @param Address New addres to patch into the instructions | ||||||||||||||||||||||||||||||||||||
| **/ | ||||||||||||||||||||||||||||||||||||
| void ThumbMovwMovtImmediatePatch(uint16_t *Instructions, uint32_t Address) { | ||||||||||||||||||||||||||||||||||||
| uint16_t *Word{}; | ||||||||||||||||||||||||||||||||||||
| uint16_t *Top{}; | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| Word = Instructions; // MOVW | ||||||||||||||||||||||||||||||||||||
| Top = Word + 2; // MOVT | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| ThumbMovtImmediatePatch(Word, static_cast<uint16_t>(Address & 0xffff)); | ||||||||||||||||||||||||||||||||||||
| ThumbMovtImmediatePatch(Top, static_cast<uint16_t>(Address >> 16)); | ||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| } // namespace | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| int relocate_image(char *image) { | ||||||||||||||||||||||||||||||||||||
| const auto dos_header = reinterpret_cast<IMAGE_DOS_HEADER *>(image); | ||||||||||||||||||||||||||||||||||||
| const auto pe_header = dos_header->GetPEHeader(); | ||||||||||||||||||||||||||||||||||||
|
|
@@ -141,7 +45,6 @@ int relocate_image(char *image) { | |||||||||||||||||||||||||||||||||||
| auto Fixup16 = reinterpret_cast<uint16_t *>(Fixup); | ||||||||||||||||||||||||||||||||||||
| auto Fixup32 = reinterpret_cast<uint32_t *>(Fixup); | ||||||||||||||||||||||||||||||||||||
| auto Fixup64 = reinterpret_cast<uint64_t *>(Fixup); | ||||||||||||||||||||||||||||||||||||
| uint32_t FixupVal = 0; | ||||||||||||||||||||||||||||||||||||
| switch ((*Reloc) >> 12) { | ||||||||||||||||||||||||||||||||||||
| case EFI_IMAGE_REL_BASED_ABSOLUTE: | ||||||||||||||||||||||||||||||||||||
| break; | ||||||||||||||||||||||||||||||||||||
|
|
@@ -167,18 +70,32 @@ int relocate_image(char *image) { | |||||||||||||||||||||||||||||||||||
| *Fixup64 = *Fixup64 + static_cast<uint64_t>(Adjust); | ||||||||||||||||||||||||||||||||||||
| break; | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| case EFI_IMAGE_REL_BASED_ARM_MOV32T: | ||||||||||||||||||||||||||||||||||||
| FixupVal = ThumbMovwMovtImmediateAddress(Fixup16) + | ||||||||||||||||||||||||||||||||||||
| static_cast<uint32_t>(Adjust); | ||||||||||||||||||||||||||||||||||||
| ThumbMovwMovtImmediatePatch(Fixup16, FixupVal); | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| break; | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| case EFI_IMAGE_REL_BASED_ARM_MOV32A: | ||||||||||||||||||||||||||||||||||||
| printf("Unsupported relocation type: EFI_IMAGE_REL_BASED_ARM_MOV32A\n"); | ||||||||||||||||||||||||||||||||||||
| // break omitted - ARM instruction encoding not implemented | ||||||||||||||||||||||||||||||||||||
| break; | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| case EFI_IMAGE_REL_BASED_LOONGARCH64_MARK_LA: { | ||||||||||||||||||||||||||||||||||||
| // The next four instructions are used to load a 64 bit address, | ||||||||||||||||||||||||||||||||||||
| // relocate all of them | ||||||||||||||||||||||||||||||||||||
| uint64_t Value = | ||||||||||||||||||||||||||||||||||||
| (*Fixup32 & 0x1ffffe0) << 7 | // lu12i.w 20bits from bit5 | ||||||||||||||||||||||||||||||||||||
| (*(Fixup32 + 1) & 0x3ffc00) >> 10; // ori 12bits from bit10 | ||||||||||||||||||||||||||||||||||||
| uint64_t Tmp1 = *(Fixup32 + 2) & 0x1ffffe0; // lu32i.d 20bits from bit5 | ||||||||||||||||||||||||||||||||||||
| uint64_t Tmp2 = *(Fixup32 + 3) & 0x3ffc00; // lu52i.d 12bits from bit10 | ||||||||||||||||||||||||||||||||||||
| Value = Value | (Tmp1 << 27) | (Tmp2 << 42); | ||||||||||||||||||||||||||||||||||||
| Value += Adjust; | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| *Fixup32 = (*Fixup32 & ~0x1ffffe0) | (((Value >> 12) & 0xfffff) << 5); | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| Fixup += sizeof(uint32_t); | ||||||||||||||||||||||||||||||||||||
| *Fixup32 = (*Fixup32 & ~0x3ffc00) | ((Value & 0xfff) << 10); | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| Fixup += sizeof(uint32_t); | ||||||||||||||||||||||||||||||||||||
| *Fixup32 = (*Fixup32 & ~0x1ffffe0) | (((Value >> 32) & 0xfffff) << 5); | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| Fixup += sizeof(uint32_t); | ||||||||||||||||||||||||||||||||||||
|
Comment on lines
+90
to
+96
|
||||||||||||||||||||||||||||||||||||
| Fixup += sizeof(uint32_t); | |
| *Fixup32 = (*Fixup32 & ~0x3ffc00) | ((Value & 0xfff) << 10); | |
| Fixup += sizeof(uint32_t); | |
| *Fixup32 = (*Fixup32 & ~0x1ffffe0) | (((Value >> 32) & 0xfffff) << 5); | |
| Fixup += sizeof(uint32_t); | |
| Fixup += sizeof(uint32_t); | |
| Fixup32 = reinterpret_cast<uint32_t *>(Fixup); | |
| *Fixup32 = (*Fixup32 & ~0x3ffc00) | ((Value & 0xfff) << 10); | |
| Fixup += sizeof(uint32_t); | |
| Fixup32 = reinterpret_cast<uint32_t *>(Fixup); | |
| *Fixup32 = (*Fixup32 & ~0x1ffffe0) | (((Value >> 32) & 0xfffff) << 5); | |
| Fixup += sizeof(uint32_t); | |
| Fixup32 = reinterpret_cast<uint32_t *>(Fixup); |
Copilot
AI
Feb 25, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This case EFI_IMAGE_REL_BASED_LOONGARCH64_MARK_LA block falls through into default because it lacks a break; after the closing brace. As written it will always print "Unsupported relocation type" and return -1 even after performing the relocation.
| } | |
| } | |
| break; |
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -22,29 +22,19 @@ | |||||
| #include <string.h> | ||||||
| #include <uefi/types.h> | ||||||
|
|
||||||
| #include "charset.h" | ||||||
| #include "variable_mem.h" | ||||||
|
|
||||||
| namespace { | ||||||
|
|
||||||
| constexpr auto &&kSecureBoot = "SecureBoot"; | ||||||
|
|
||||||
| EfiStatus GetVariable(const uint16_t *VariableName, const EfiGuid *VendorGuid, | ||||||
| uint32_t *Attributes, size_t *DataSize, void *Data) { | ||||||
| if (!VariableName || !VendorGuid || !DataSize) { | ||||||
| return EFI_STATUS_INVALID_PARAMETER; | ||||||
| } | ||||||
|
|
||||||
| char buffer[512]; | ||||||
| size_t i = 0; | ||||||
| while (VariableName[i] && i < sizeof(buffer)) { | ||||||
| size_t j = 0; | ||||||
| for (j = 0; j < sizeof(buffer) - 1 && VariableName[i + j]; j++) { | ||||||
| buffer[j] = VariableName[i + j]; | ||||||
| } | ||||||
| i += j; | ||||||
| } | ||||||
| buffer[i] = 0; | ||||||
| if (strncmp(buffer, kSecureBoot, sizeof(kSecureBoot)) == 0 || strcmp(buffer, "SetupMode") == 0) { | ||||||
| if (utf16_strcmp(VariableName, u"SecureBoot") == 0 && | ||||||
|
||||||
| if (utf16_strcmp(VariableName, u"SecureBoot") == 0 && | |
| if (utf16_strcmp(VariableName, u"SecureBoot") == 0 || |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The comment says the conversion functions return the number of characters converted "including the null terminator", but the implementation returns the count of non-null characters copied (the terminator is written after the loop and not counted). Update the comment or adjust the return value for consistency.