[lib][uefi] correctness, resource lifetime, and PE parsing fixes - #532
[lib][uefi] correctness, resource lifetime, and PE parsing fixes#532zhangxp1998 wants to merge 14 commits into
Conversation
There was a problem hiding this comment.
Pull request overview
This PR tightens correctness in LK’s lib/uefi implementation by making UEFI service tables safer by default, ensuring loader failures abort execution, and aligning variable/block I/O behavior with expected UEFI semantics (especially around “unsupported” operations and variable sizing/deletion rules).
Changes:
- Ensure unimplemented boot/runtime service table slots remain null, and wire the existing
set_timerstub into the boot services table. - Make synchronous block writes/flushes return
EFI_STATUS_UNSUPPORTEDinstead of falsely reporting success. - Fix UEFI variable semantics: delete variables on zero-size set, improve
GetVariablesize/error handling, and make relocation failure abort image execution.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| lib/uefi/variable_mem.cpp | Implements zero-size SetVariable-as-delete in the in-memory variable store. |
| lib/uefi/uefi.cpp | Stops using byte-pattern-filled service tables; aborts image execution if relocation fails. |
| lib/uefi/runtime_service_provider.cpp | Improves GetVariable handling (size reporting, null checks, attributes) and clarifies SetVariable volatility support. |
| lib/uefi/boot_service_provider.cpp | Wires set_timer into the boot services table. |
| lib/uefi/blockio_protocols.cpp | Makes synchronous writes/flushes “honestly unsupported” rather than returning success. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| EfiStatus SetVariable(const uint16_t* VariableName, const EfiGuid* VendorGuid, | ||
| uint32_t Attributes, size_t DataSize, const void* Data) { | ||
| if (!VariableName || VariableName[0] == 0) { | ||
| return EFI_STATUS_INVALID_PARAMETER; | ||
| } |
There was a problem hiding this comment.
Both addressed. set_variable now rejects a null VendorGuid with EFI_STATUS_INVALID_PARAMETER before the store can do a name-only match, and rejects DataSize > 0 with Data == nullptr before anything is copied. The checks live in the runtime-service wrapper (the EFI-facing surface), mirroring what get_variable already did for its parameters.
7a820a1 to
eeeacb1
Compare
zhangxp1998
changed the title
| if (relocate_image(image_base) != 0) { | ||
| printf("Failed to relocate image\n"); | ||
| return ERR_BAD_STATE; |
There was a problem hiding this comment.
Good catch — the guard was toothless for that case. relocate_image() now returns -1 on EFI_IMAGE_REL_BASED_ARM_MOV32A instead of logging and breaking, so an image carrying an unsupported ARM MOV32 relocation is rejected before it can reach its entry point with the fixup unapplied. Every other unrecognized type already fell through to default: return -1, so all unsupported relocations now abort the load consistently and the relocate_image() != 0 check actually fires. (Implementing the ARM MOVW/MOVT encoding isn't worth it here — the images we load are aarch64/x86 and use DIR64/HIGHLOW.)
| if (*DataSize < 1) { | ||
| *DataSize = 1; | ||
| return EFI_STATUS_BUFFER_TOO_SMALL; |
There was a problem hiding this comment.
Fixed. The attributes are now published as soon as the variable is recognized, before the buffer-size check, so a size probe that returns EFI_STATUS_BUFFER_TOO_SMALL reports the same Attributes as a successful read — matching the stored-variable path, which fills them via efi_get_variable before the size comparison. SecureBoot/SetupMode callers now observe consistent attributes regardless of buffer size.
| free_pages(slot.addr, slot.size / PAGE_SIZE); | ||
| slot.addr = nullptr; |
There was a problem hiding this comment.
Both addressed. #530 is now merged, so I rebased this branch on top of it: free_pages() maps the physical address back through paddr_to_kvaddr() before handing it to pmm_free_kpages(), so the boot buffers are genuinely returned to the PMM. release_boot_buffers() now also checks the result and only clears slot.addr on EFI_STATUS_SUCCESS, so a failed free can no longer downgrade a tracked buffer into an untracked leak. Verified end-to-end: two back-to-back uefi_load runs leave the PMM free_count unchanged (129684 → 129684 → 129684).
| printf("%s: no slot left to track %s, device reference will not be " | ||
| "released\n", | ||
| __FUNCTION__, name); | ||
| return dev; |
There was a problem hiding this comment.
Fixed by refusing the open instead of leaking. When every tracking slot is in use, open_tracked_bdev() now bio_close()s the freshly-opened device and returns nullptr (the caller surfaces it as a failed open) rather than returning an untracked reference. Repeatedly requesting a block protocol can therefore no longer leak references past teardown — the count is bounded and every tracked open is closed when the run ends. (Kept the fixed-capacity tracker; failing a 17th concurrent open is acceptable here and simpler than a growable store.)
The boot and runtime service tables were filled with an incrementing byte pattern before the implemented entries were installed, so calling any unimplemented service jumped to a garbage address. Drop the fill: the tables are value-initialized, unimplemented slots stay null, which applications can test for, and a stray call faults at address zero instead of a random location. Also install the existing set_timer stub, which was never wired into the table.
write_blocks and flush_blocks logged a message and returned EFI_STATUS_SUCCESS without touching the device, so applications believed their writes had been persisted. Return EFI_STATUS_UNSUPPORTED instead, matching the async write path.
bb6ed0c to
9555034
Compare
| @@ -42,27 +42,38 @@ EfiStatus GetVariable(const uint16_t *VariableName, const EfiGuid *VendorGuid, | |||
| } | |||
| buffer[i] = 0; | |||
| if (strncmp(buffer, kSecureBoot, sizeof(kSecureBoot)) == 0 || strcmp(buffer, "SetupMode") == 0) { | |||
There was a problem hiding this comment.
Fixed. The SecureBoot/SetupMode branch now also requires VendorGuid == EFI_GLOBAL_VARIABLE_GUID (a memcmp against the constant in variable_mem.h) before synthesizing. A query for either name under any other GUID now falls through to the variable store instead of getting the synthesized value and attributes.
| const size_t nt_headers_end = static_cast<size_t>(dos_header->e_lfanew) + | ||
| sizeof(IMAGE_FILE_HEADER) + | ||
| file_header->SizeOfOptionalHeader; |
There was a problem hiding this comment.
Handled in relocate_image() before it indexes the directory. It now bails out (treating the image as having no relocations) unless the optional header both declares and physically contains that entry:
constexpr size_t kRelocDirEnd =
offsetof(IMAGE_OPTIONAL_HEADER64, DataDirectory) +
(IMAGE_DIRECTORY_ENTRY_BASERELOC + 1) * sizeof(IMAGE_DATA_DIRECTORY);
if (optional_header->NumberOfRvaAndSizes <= IMAGE_DIRECTORY_ENTRY_BASERELOC ||
pe_header->FileHeader.SizeOfOptionalHeader < kRelocDirEnd) {
printf("No base relocation directory present\n");
return 0;
}
So a short optional header (the 112-byte minimum) can no longer make relocation read section-table bytes as DataDirectory[5].
| // Only drop the pointer once the pages are actually returned; otherwise a | ||
| // failed free would turn a tracked buffer into an untracked leak. | ||
| if (free_pages(slot.addr, slot.size / PAGE_SIZE) != EFI_STATUS_SUCCESS) { | ||
| printf("release_boot_buffers: failed to free %p (%zu bytes)\n", slot.addr, | ||
| slot.size); | ||
| continue; |
There was a problem hiding this comment.
Good point — keeping the pointer was the wrong half to fix. Since release_boot_buffers() runs immediately before reset_heap() (which tears down the boot aspace via vmm_free_aspace), any retained slot.addr is unmapped on the next run. release_boot_buffers() now always clears the slot, so get_boot_buffer() re-allocates and re-maps a fresh buffer next time; a free_pages() failure is logged (physical pages leak) rather than left reusable. With #530 merged the common path frees cleanly, so this is just the defensive tail. This supersedes the earlier "only clear on success" change, which is what opened the unsafe-reuse window you spotted.
| // Every tracking slot is in use. Returning this reference would leak it at | ||
| // teardown, so close it and fail the open instead. | ||
| printf("%s: no free tracking slot for %s, refusing to leak the reference\n", | ||
| __FUNCTION__, name); | ||
| bio_close(dev); | ||
| return nullptr; |
There was a problem hiding this comment.
Done. open_tracked_bdev() now returns an EfiStatus and yields the device through an out-param: EFI_STATUS_NOT_FOUND when bio_open() fails, EFI_STATUS_OUT_OF_RESOURCES when the tracker is full (device closed), EFI_STATUS_SUCCESS otherwise. All three callers (open_block_device, open_async_block_device, open_efi_erase_block_protocol) propagate that status and only print "no such block device" on NOT_FOUND, so tracker exhaustion now surfaces as out-of-resources instead of a bogus not-found.
9555034 to
9f5aa6e
Compare
| /* setting a variable with zero size deletes it */ | ||
| if (data_len == 0) { | ||
| if (var) { | ||
| list_delete(&var->node); | ||
| free(var->Data); | ||
| free(var); | ||
| } | ||
| return; |
There was a problem hiding this comment.
Fixed. efi_set_variable() now treats DataSize == 0 as a delete only when EFI_VARIABLE_APPEND_WRITE is clear; a zero-length append is a no-op that leaves the existing value intact. Added the missing attribute constants to variable_mem.h (EFI_VARIABLE_APPEND_WRITE = 0x40, plus the two authenticated-write bits 0x10/0x20).
| if (optional_header->NumberOfRvaAndSizes <= IMAGE_DIRECTORY_ENTRY_BASERELOC || | ||
| pe_header->FileHeader.SizeOfOptionalHeader < kRelocDirEnd) { | ||
| printf("No base relocation directory present\n"); | ||
| return 0; |
There was a problem hiding this comment.
Good catch. relocate_image() now computes the load adjustment up front and, when there is no usable relocation directory (absent or empty), only returns success if the image can actually run there: it returns an error whenever the load address differs from ImageBase and the image is marked IMAGE_FILE_RELOCS_STRIPPED (it declared it must load at its preferred base).
I deliberately did not error on the strip-clear case: an image with no relocations and RELOCS_STRIPPED clear is position-independent and safe off base. That is exactly what the bundled helloworld_aa64.efi is (ImageBase 0x140000000, loaded at 0x53200000, empty .reloc, Characteristics 0x22) — erroring unconditionally would reject it and other valid PIE images. The RELOCS_STRIPPED flag is the spec-defined signal that an image requires its ImageBase, so keying off it catches the unsafe case (stripped + relocated) without breaking PIE loads. Verified: helloworld still loads off base and runs. Added the IMAGE_FILE_RELOCS_STRIPPED constant to pe.h.
| for (auto &slot : tracked_bdevs) { | ||
| if (slot == nullptr) { | ||
| slot = dev; | ||
| *out_dev = dev; | ||
| return EFI_STATUS_SUCCESS; |
There was a problem hiding this comment.
Addressed by removing the fixed cumulative cap (the second option you suggested). The tracker is now a growable singly-linked list instead of a 16-entry array, so the number of opens across an image run is no longer bounded; close_tracked_bdevs() walks the list at teardown, bio_close()-ing and freeing every node. Repeated OpenProtocol/CloseProtocol cycles can therefore no longer exhaust it and make the 17th open fail. A malloc failure still surfaces as EFI_STATUS_OUT_OF_RESOURCES, which is now a genuine out-of-memory rather than an artificial cap.
relocate_image reports failure for malformed or unsupported relocation entries, but load_sections_and_execute ignored the result and jumped to the entry point of a partially relocated image. Fail the load instead.
Report the required size in *DataSize when returning EFI_STATUS_BUFFER_TOO_SMALL for a too-small buffer, return EFI_STATUS_INVALID_PARAMETER instead of copying through a null Data pointer, and honor *DataSize for the synthesized SecureBoot/SetupMode answers instead of writing a byte unconditionally. Also report the attributes of the synthesized variables.
Setting a variable with a zero DataSize is defined to delete it. The in-memory store kept an empty variable around instead, and get_variable then reported success with zero bytes rather than not-found. Also validate parameters before touching the store: reject a null VendorGuid, which would otherwise match a same-named variable under any GUID in the store, and reject a non-zero DataSize with a null Data pointer, which would be copied from and crash. And fix the set_variable comment and error message, which claimed non-volatile variables were the supported kind while the code accepts only volatile ones.
get_boot_buffer allocated a fresh buffer on every call and nothing ever released the buffers, so a UEFI app that returned to the shell (for example a fastboot session that never boots) leaked up to 259 MiB of contiguous pages per run, and repeated calls for the same type leaked even within one run. Keep one cached buffer per type, hand the same buffer back on repeated requests, and release them all when the image run tears down.
open_block_device, open_async_block_device, and open_efi_erase_block_protocol called bio_open and nothing ever called bio_close, leaking a device reference per protocol open on every run. Route the opens through a small tracked-device table and close every tracked device when the image run tears down. Also fail cleanly with EFI_STATUS_NOT_FOUND when the device does not exist instead of dereferencing a null bdev pointer, and check the async interface allocation like the synchronous path already does.
load_pe_file parsed the DOS header, NT headers, and section table out of a 4 KiB buffer while only checking e_lfanew against the file header size. A truncated or malformed image could place the optional header or the section table beyond the bytes the reader returned, parsing uninitialized heap memory, or beyond the buffer entirely. Check every region against the number of bytes actually read, and reject images whose subsystem is not an EFI application instead of only warning.
lib/cksum already provides the CRC-32 this service is defined to compute (the same polynomial is used for the EFI system table pointer checksum), so wire it up instead of returning EFI_STATUS_UNSUPPORTED. The output parameter is renamed so it no longer shadows the crc32 function it now calls.
9f5aa6e to
03a3e10
Compare
| if (data == nullptr || crc_out == nullptr || len == 0) { | ||
| return EFI_STATUS_INVALID_PARAMETER; | ||
| } | ||
| *crc_out = crc32(0, static_cast<const unsigned char *>(data), len); |
There was a problem hiding this comment.
Fixed. calculate_crc32 now rejects only null pointers, returns 0 for a non-null empty buffer, and feeds larger buffers through crc32() in UINT_MAX-sized chunks using the running crc, so lengths above UINT_MAX are no longer truncated. (fd361ad)
| const auto Adjust = | ||
| reinterpret_cast<size_t>(image - optional_header->ImageBase); |
There was a problem hiding this comment.
Fixed in 3865331. The load adjustment is now computed from integer addresses (reinterpret_cast<size_t>(image) - ImageBase) instead of forming the pointer image - ImageBase, which pointed far outside the allocation whenever ImageBase was nonzero.
| const auto reloc_directory = | ||
| optional_header->DataDirectory[IMAGE_DIRECTORY_ENTRY_BASERELOC]; | ||
| if (reloc_directory.Size == 0) { | ||
| printf("Relocation section empty\n"); | ||
| have_reloc_dir | ||
| ? optional_header->DataDirectory[IMAGE_DIRECTORY_ENTRY_BASERELOC] | ||
| : IMAGE_DATA_DIRECTORY{}; |
There was a problem hiding this comment.
Fixed in 3865331. relocate_image() now takes the allocation size and validates every attacker-controlled offset before dereferencing: the relocation directory must lie within the image, each block must contain its header and fit inside the directory (zero, short, and oversized SizeOfBlock are rejected), and the write extent of each fixup, sized per relocation type, must fall inside the image. Out-of-range directories, blocks, and fixups are now rejected instead of being read or written. Verified: arm64 Clang/LLD build, uefi_load returns 0 twice with stable free_count, ut all 38/38.
| /* A zero-size write deletes the variable, but only when this is not an | ||
| * append. With EFI_VARIABLE_APPEND_WRITE set, appending zero bytes is a | ||
| * no-op that must leave any existing value unchanged. */ | ||
| if (data_len == 0) { | ||
| if ((attribute & EFI_VARIABLE_APPEND_WRITE) == 0 && var) { |
There was a problem hiding this comment.
Fixed in a4a8d5e. A non-empty EFI_VARIABLE_APPEND_WRITE now concatenates onto the existing value instead of falling through to the replace path, and the existing value is left intact on allocation failure. The append bit is stripped from the stored attributes since it is a per-call modifier rather than stored state.
| // On success *out_dev holds the opened device. Returns EFI_STATUS_NOT_FOUND | ||
| // when the device does not exist and EFI_STATUS_OUT_OF_RESOURCES when the | ||
| // tracking table is full (so callers can distinguish the two). |
There was a problem hiding this comment.
Fixed the stale contract comment: with the growable list there is no fixed table, so EFI_STATUS_OUT_OF_RESOURCES now documents a failed tracking-node allocation, and open_tracked_bdev/close_tracked_bdev are described accordingly.
While here I also closed the underlying gap that made the cumulative-open tracking necessary in the first place. close_protocol(EFI_BLOCK_IO_PROTOCOL_GUID) previously returned success without releasing the reference, so a tracked open was only ever closed at teardown. It now balances the reference: close_tracked_bdev() unlinks a single tracked node matching the device (block I/O handles are the bdev name) and bio_close()s it. An app that pairs OpenProtocol/CloseProtocol no longer accumulates references for the whole run; teardown still sweeps anything left open, and unlinking first means it cannot double-close a device that was already released.
Verified: arm64 Clang/LLD build, two uefi_load runs return 0 with stable free_count (129684), ut all 38/38.
| setup_heap(); | ||
| DEFER { reset_heap(); }; | ||
| DEFER { release_boot_buffers(); }; | ||
| DEFER { close_tracked_bdevs(); }; |
There was a problem hiding this comment.
You are right that this is a real teardown/async race, and I have deliberately left it out of this PR. close_tracked_bdevs() only balances the bio reference count; it does not wait for asynchronous BlockIo2 work. read_blocks_async either queues a driver callback or spawns a detached async_bio thread, and nothing joins those before the image returns and teardown runs bio_close.
Doing this correctly means tracking in-flight asynchronous operations before releasing the reference -- an outstanding-op count on the tracked device that teardown drains (or an explicit cancel path) -- which is a larger change to the async path than the reference-lifetime fixes this PR is about. I would rather land it as a focused follow-up than attach a partial guard here. The synchronous BlockIo path completes its I/O before returning and is unaffected; the pending-async-at-exit window is a known limitation I will address separately.
close_protocol() returned success for EFI_BLOCK_IO_PROTOCOL_GUID without releasing the bio reference taken by open_block_device(), so a tracked open was only ever closed by the teardown sweep in close_tracked_bdevs(). That made the tracker count cumulative opens across the whole image run. Balance each CloseProtocol against one open: close_tracked_bdev() unlinks a single tracked node matching the device (block I/O handles are the bdev name) and bio_close()s it. Teardown still reclaims any opens the app never closed, and because the node is unlinked first the teardown sweep no longer double-closes a device that was already released, so there is no refcount underflow. Also correct the open_tracked_bdev() contract comment: with the growable list there is no fixed table, so EFI_STATUS_OUT_OF_RESOURCES now means a tracking-node allocation failed.
CalculateCrc32 rejected a non-null zero-length buffer, but an empty buffer has a well-defined CRC of 0. It also passed a size_t length straight into lib/cksum's crc32(), whose length parameter is unsigned int, so a buffer larger than UINT_MAX was silently truncated while the call still reported success. Reject only null pointers, return 0 for an empty buffer, and feed larger buffers through crc32() in UINT_MAX-sized chunks using the running crc.
relocate_image() trusted the relocation directory, which is attacker-controlled: the directory RVA/size, each block's SizeOfBlock, and every fixup RVA were used to walk and write memory without checking them against the loaded image, so a malformed PE could read or write out of bounds. Pass the allocation size into relocate_image() and validate before every dereference: the directory must lie within the image, each block must contain its header and fit inside the directory, and each fixup's write extent (sized per relocation type) must fall inside the image. Also compute the load adjustment from integer addresses instead of subtracting ImageBase from the image pointer, which formed a pointer outside the allocation (undefined behavior) whenever ImageBase was nonzero.
efi_set_variable() only handled the zero-length append case. A non-empty write with EFI_VARIABLE_APPEND_WRITE set fell through to the replace path, freed the existing value, and stored only the new data while SetVariable reported success -- silent data loss. Concatenate onto the existing value for a non-empty append (leaving it intact on allocation failure), and record the effective attributes without the append bit, which is a per-call modifier rather than stored state.
| // cumulative number of OpenProtocol calls. The underlying bio reference is | ||
| // taken on the first open and released by close_tracked_bdev() when the last | ||
| // open closes, or by close_tracked_bdevs() at teardown. | ||
| struct TrackedBdev { |
There was a problem hiding this comment.
have you considered using https://github.com/littlekernel/lk/blob/master/top/include/lk/list.h
instead of implementing your own linked list?
There was a problem hiding this comment.
Yes, agreed. I switched the tracker to LK's list implementation in 089be41: TrackedBdev now embeds a list_node, and lookup, insertion, removal, and teardown use list_for_every_entry, list_add_tail, list_delete, and list_remove_head_type. The one-entry-per-distinct-bdev/open-count behavior is unchanged.
| } | ||
| } // namespace | ||
|
|
||
| EfiStatus open_tracked_bdev(const char *name, bdev_t **out_dev) { |
There was a problem hiding this comment.
why do we want to reference track the opened devices ourselves? why not just open a new bdev for each open_protocol(), or reference count the blockio_protocol struct instead?
Also, bio_open / bio_close already does its own reference counting & bookkeeping https://github.com/zhangxp1998/lk/blob/5203dcb1b2468ce60b9dc031d49c305fe3c39133/lib/bio/bio.c#L357
so I think we shouldn't ref-count it again here.
There was a problem hiding this comment.
OpenProtocol() can be called multiple times for the same handle/protocol, so there can be several outstanding UEFI opens referring to the same bdev. The extra open_count is meant to track those logical UEFI opens, not to duplicate BIO's device-lifetime refcount.
The tracker deliberately takes one bio_open() reference per distinct bdev and multiplexes repeated UEFI opens onto it. This avoids adding another cleanup-list node and another BIO reference for every repeated OpenProtocol() call. CloseProtocol() drops one logical open; the single BIO reference is released when the last logical open closes. If the application returns without balancing all of its opens, the per-run teardown still has one entry for the bdev and can release that reference, so an uncooperative or buggy UEFI application cannot leak device references across runs.
BIO's own reference count keeps a bdev alive, but BIO does not know which references belong to the current UEFI image run or when that run ends, so it cannot provide that cleanup by itself. Taking a fresh BIO reference for every OpenProtocol() would still require an unbounded per-open cleanup list (or an equivalent count) to recover missing CloseProtocol() calls at teardown.
Reference-counting a shared Block I/O protocol object could also model this, but the current UEFI shim allocates transient protocol interfaces from the per-run heap and CloseProtocol() receives the handle/GUID rather than the returned interface pointer. Doing that cleanly would require a broader handle/protocol registry. I would prefer to keep this small per-bdev ownership tracker here.
open_tracked_bdev() took a fresh bio reference and appended a new node on every OpenProtocol, so an app that repeatedly opened a device grew the tracker without bound. Track each distinct device once with a count of outstanding opens: reuse the existing node (bumping the count) when the device is already open, take the bio reference only on the first open, and release it once the last open closes or at teardown. The list is now bounded by the number of bdevs rather than the cumulative number of opens.
5203dcb to
089be41
Compare
Correctness and resource-lifetime fixes in lib/uefi.
Service tables and loader behavior:
set_timerstub is now actually installed in the table (it was never wired up).write_blocks/flush_blocksreturnedEFI_STATUS_SUCCESSwithout touching the device, so applications believed their writes had been persisted. They now returnEFI_STATUS_UNSUPPORTED, matching the async path (write_blocks_ex).load_sections_and_execute()ignoredrelocate_image()'s result and jumped to the entry point of a partially relocated image.e_lfanewagainst the file header size; a truncated or malformed image could place the optional header or the section table beyond the bytes the reader returned (parsing uninitialized heap) or beyond the 4 KiB buffer entirely. Every region is now checked against the bytes actually read, and a non-EFI-application subsystem is rejected instead of only warned about.Variable services:
EFI_STATUS_BUFFER_TOO_SMALL, returnEFI_STATUS_INVALID_PARAMETERinstead of copying through a nullDatapointer, honor*DataSizefor the synthesized SecureBoot/SetupMode answers instead of writing a byte unconditionally, and report attributes for them.VendorGuid(the store would otherwise match a same-named variable under any GUID) and a non-zeroDataSizewith nullData; per spec, a zeroDataSizenow deletes the variable instead of leaving an empty one behind. Also fixes the inverted comment/error message (the code accepts only volatile variables).Resource lifetime:
get_boot_bufferallocated a fresh buffer on every call and nothing ever released them, so an app that returned to the shell (e.g. a fastboot session that never boots) leaked up to 259 MiB of contiguous pages per run. One buffer per type is now cached, repeated requests get the same buffer, and everything is released at run teardown.open_block_device,open_async_block_device, andopen_efi_erase_block_protocolcalledbio_openwith no matchingbio_close, leaking a device reference per protocol open per run. Opens are now tracked and closed at run teardown, and a missing device fails cleanly withEFI_STATUS_NOT_FOUNDinstead of dereferencing a null bdev.EFI_STATUS_UNSUPPORTED.Validation (qemu-virt-arm64-test, Clang/LLD, WERROR=1):
helloworld_aa64.efiloads and runs (return code 0),uefi_set_var/uefi_list_varbehave as before,ut allpasses 36/36, no faults in the loguefi_loadruns both succeed, PMMfree_countidentical before/after,ut all36/36