[lib][uefi] Fix memory leak in load_pe_file by zhangxp1998 · Pull Request #497 · littlekernel/lk

zhangxp1998 · 2026-03-02T22:13:24Z

Fixes memory leak in load_pe_file and updates UEFI/GBL interface layer

zhangxp1998 · 2026-03-02T22:19:13Z

or some reason github won't let me re-open #468 , so had to create a new one

Copilot

Pull request overview

This PR aims to fix a memory leak in the PE loader (load_pe_file) and adjusts parts of the UEFI interface layer while extending relocation handling.

Changes:

Add missing cleanup/error handling in load_pe_file (notably freeing the PE header buffer).
Update PE loading/section reading to use LK ERR_* codes and add read-size validation.
Modify UEFI system table firmware_vendor typing and extend relocation support with a LoongArch64 relocation type.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 13 comments.

Show a summary per file

File	Description
`lib/uefi/uefi.cpp`	Fixes PE-header allocation leak, adds read/error checks, adjusts return codes, and tweaks system table wiring.
`lib/uefi/relocation.cpp`	Removes Thumb MOV32T helpers and adds a LoongArch64 relocation case.
`lib/uefi/pe.h`	Adds a comment in the DOS header and introduces a new relocation type constant.
`lib/uefi/include/uefi/system_table.h`	Changes `firmware_vendor` to `const char16_t*`.
`lib/uefi/boot_service_provider.cpp`	Minor formatting and changes a protocol response to `EFI_STATUS_UNSUPPORTED`.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-03-02T22:19:33Z

lib/uefi/uefi.cpp

+    bytes_read = reader->read(image_base + section.VirtualAddress,
+                             section.PointerToRawData, section.SizeOfRawData);
+    if (bytes_read != section.SizeOfRawData) {
+      printf("Failed to read section %s %zu\n", section.Name, bytes_read);


The error path prints bytes_read with %zu, but bytes_read is ssize_t. This is undefined/incorrect formatting on 32-bit/64-bit mixes and will print garbage for negative values. Use %zd (or cast to an appropriate unsigned type after validating non-negative).

Suggested change

printf("Failed to read section %s %zu\n", section.Name, bytes_read);

printf("Failed to read section %s %zd\n", section.Name, bytes_read);

lib/uefi/uefi.cpp

Copilot · 2026-03-02T22:19:35Z

lib/uefi/relocation.cpp

+        *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);
+        *Fixup32 = (*Fixup32 & ~0x3ffc00) | (((Value >> 52) & 0xfff) << 10);


This relocation case advances Fixup (lines 90/93/96) but continues to write through the original Fixup32 pointer, so the second/third/fourth instruction patches will overwrite the first word repeatedly. Recompute Fixup32 after advancing, or avoid mutating Fixup and patch via Fixup32[i].

Suggested change

*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);

*Fixup32 = (*Fixup32 & ~0x3ffc00) | (((Value >> 52) & 0xfff) << 10);

*(Fixup32 + 0) =

(*(Fixup32 + 0) & ~0x1ffffe0) | (((Value >> 12) & 0xfffff) << 5);

*(Fixup32 + 1) =

(*(Fixup32 + 1) & ~0x3ffc00) | ((Value & 0xfff) << 10);

*(Fixup32 + 2) =

(*(Fixup32 + 2) & ~0x1ffffe0) | (((Value >> 32) & 0xfffff) << 5);

*(Fixup32 + 3) =

(*(Fixup32 + 3) & ~0x3ffc00) | (((Value >> 52) & 0xfff) << 10);

Copilot · 2026-03-02T22:19:35Z

lib/uefi/include/uefi/system_table.h

 typedef struct EfiSystemTable {
  EfiTableHeader header;
-  EfiChar16* firmware_vendor;
+  const char16_t* firmware_vendor;


EfiSystemTable previously used the project’s fixed-width EfiChar16 type (see lib/uefi/include/uefi/types.h comments about ABI stability). Switching the public struct field to built-in char16_t undermines that ABI guarantee and also creates type inconsistency with the rest of the UEFI headers. Prefer const EfiChar16* firmware_vendor (and update call sites accordingly).

Suggested change

const char16_t* firmware_vendor;

const EfiChar16* firmware_vendor;

Copilot · 2026-03-02T22:19:36Z

lib/uefi/pe.h

+  // ASCII "PE\x0\x0"
  constexpr bool CheckMagic() const { return LE32(e_magic) == 0x5A4D; }


The new comment says ASCII "PE\x0\x0" but e_magic/CheckMagic() are validating the DOS "MZ" signature (0x5A4D). This comment is misleading; update it to describe the MZ/DOS header magic, and (if needed) comment the PE\0\0 signature check where kPEHeader is validated.

Copilot · 2026-03-02T22:19:36Z

lib/uefi/uefi.cpp

+  if (bytes_read != section_header[0].SizeOfRawData) {
+    printf("Failed to read section %s\n", section_header[0].Name);
+    return ERR_IO;


IMAGE_SECTION_HEADER::Name is an 8-byte field and is not guaranteed to be null-terminated. Printing it with %s can read past the field. Use a length-limited format (e.g., %.8s) when logging section names.

Copilot · 2026-03-02T22:19:36Z

lib/uefi/uefi.cpp

+    if (bytes_read != section.SizeOfRawData) {
+      printf("Failed to read section %s %zu\n", section.Name, bytes_read);


section.Name is not guaranteed to be null-terminated (8-byte PE section name). Logging it with %s can read past the field. Use a length-limited format (e.g., %.8s) for section names in this message as well.

Copilot · 2026-03-02T22:19:37Z

lib/uefi/uefi.cpp

+  if (bytes_read != section_header[0].SizeOfRawData) {
+    printf("Failed to read section %s\n", section_header[0].Name);


bytes_read is compared against section_header[0].SizeOfRawData, but the read length here is section_header[0].PointerToRawData (header size up to the first section). This will incorrectly report an I/O error (or miss one) whenever those two fields differ. Compare against the requested length (PointerToRawData) and consider adjusting the error message since this read is for headers, not a section payload.

Suggested change

if (bytes_read != section_header[0].SizeOfRawData) {

printf("Failed to read section %s\n", section_header[0].Name);

if (bytes_read != static_cast<ssize_t>(section_header[0].PointerToRawData)) {

printf("Failed to read PE headers before first section\n");

Copilot AI review requested due to automatic review settings March 2, 2026 22:13

Copilot started reviewing on behalf of zhangxp1998 March 2, 2026 22:14 View session

zhangxp1998 closed this Mar 2, 2026

zhangxp1998 reopened this Mar 2, 2026

Copilot AI reviewed Mar 2, 2026

View reviewed changes

zhangxp1998 force-pushed the master branch 3 times, most recently from 67ce547 to 75a3e2d Compare March 3, 2026 00:06

littlekernel deleted a comment from Copilot AI Mar 3, 2026

zhangxp1998 force-pushed the master branch 4 times, most recently from 98b3114 to c0b1dff Compare March 3, 2026 18:21

zhangxp1998 closed this Mar 3, 2026

zhangxp1998 reopened this Mar 3, 2026

zhangxp1998 force-pushed the master branch 3 times, most recently from 76f1fea to 6200c18 Compare March 3, 2026 22:12

[lib][uefi] Fix error in relocation

ae96764

zhangxp1998 force-pushed the master branch from 6200c18 to ae96764 Compare March 3, 2026 22:14

zhangxp1998 merged commit 8296581 into littlekernel:master Mar 3, 2026
120 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[lib][uefi] Fix memory leak in load_pe_file#497

[lib][uefi] Fix memory leak in load_pe_file#497
zhangxp1998 merged 1 commit intolittlekernel:masterfrom
zhangxp1998:master

zhangxp1998 commented Mar 2, 2026

Uh oh!

zhangxp1998 commented Mar 2, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Mar 2, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI Mar 2, 2026

Uh oh!

Copilot AI Mar 2, 2026

Uh oh!

Copilot AI Mar 2, 2026

Uh oh!

Copilot AI Mar 2, 2026

Uh oh!

Copilot AI Mar 2, 2026

Uh oh!

Copilot AI Mar 2, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

	printf("Failed to read section %s %zu\n", section.Name, bytes_read);
	printf("Failed to read section %s %zd\n", section.Name, bytes_read);

-        *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);
-        *Fixup32 = (*Fixup32 & ~0x3ffc00) | (((Value >> 52) & 0xfff) << 10);
+        *(Fixup32 + 0) =
+            (*(Fixup32 + 0) & ~0x1ffffe0) | (((Value >> 12) & 0xfffff) << 5);
+        *(Fixup32 + 1) =
+            (*(Fixup32 + 1) & ~0x3ffc00) | ((Value & 0xfff) << 10);
+        *(Fixup32 + 2) =
+            (*(Fixup32 + 2) & ~0x1ffffe0) | (((Value >> 32) & 0xfffff) << 5);
+        *(Fixup32 + 3) =
+            (*(Fixup32 + 3) & ~0x3ffc00) | (((Value >> 52) & 0xfff) << 10);

	const char16_t* firmware_vendor;
	const EfiChar16* firmware_vendor;

		// ASCII "PE\x0\x0"
		constexpr bool CheckMagic() const { return LE32(e_magic) == 0x5A4D; }

		if (bytes_read != section.SizeOfRawData) {
		printf("Failed to read section %s %zu\n", section.Name, bytes_read);

		if (bytes_read != section_header[0].SizeOfRawData) {
		printf("Failed to read section %s\n", section_header[0].Name);

Conversation

zhangxp1998 commented Mar 2, 2026

Uh oh!

zhangxp1998 commented Mar 2, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Mar 2, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI Mar 2, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 2, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 2, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 2, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 2, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 2, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants