Skip to content

patina-v21.0.2

Latest

Choose a tag to compare

@github-actions github-actions released this 22 Apr 15:30
· 6 commits to refs/heads/main since this release

What's Changed

  • patina\_internal\_cpu: Remove redundant architecture cfg gates @makubacki (#1471)
    Change Details
      ## Description

    The aarch64 and x64 modules are already gated by target_arch at the parent module level, so the inner #[cfg(all(not(test), target_arch = "aarch64/x86_64"))] checks simplify to just #[cfg(not(test))].

    • Impacts functionality?
    • Impacts security?
    • Breaking change?
    • Includes tests?
    • Includes documentation?

    How This Was Tested

    • cargo make all

    Integration Instructions

    • N/A


  • Add AI code review customization files [Rebase \& FF] @makubacki (#1460)
    Change Details
      ## Description

    Closes #1453

    Adds project-level AI customization files to support AI-assisted code review.

    These files encode Patina's domain-specific conventions so that AI reviewers can flag violations locally, prior to human review.

    The files are layered:

    1. Always-on instructions (.github/copilot-instructions.md): Covers global conventions across the codebase.

    2. File-based instructions (.github/instructions/): Currently, provides component-specific instructions in components.instructions.md(applyTo: components/**) to supplement the global instructions. These load only when editing matching files to minimize token cost.

    3. Review prompt file (.github/prompts/review.prompt.md): A reusable /review slash command that defines a structured review checklist. This is to provide a convenient way to reliably perform the same set of review steps. Delegates convention details to the always-on instructions to avoid duplication.

    4. Reviewer agent (.github/agents/reviewer.agent.md): A read-only review persona restricted to search and read tool sets (no edit or execute tools). Includes a handoff to the default agent for implementing fixes after review.

    References:

    How This Was Tested

    • Use the "Patina Code Reviewer" locally

    Integration Instructions

    • N/A

    This PR is in draft as it serves as a starting open to changes from others before publishing for broader review.




  • Safety \& UB Updates [Rebase \& FF] @makubacki (#1470)
    Change Details
      ## Description

    Fixes some safety and undefined behavior (UB) issues.

    Reported by Claude with a UB/safety focused prompt. Manually filtered out less severe issues, false positives, and made fixes.


    boot_services: Fix UB in create_event_ex() Option transmute

    create_event_ex() transmuted Option to extern fn, dropping
    the Option wrapper. Passing None would produce a null function pointer
    typed as fn(...) which is instant UB since fn pointers are guaranteed
    non-null.

    Align create_event_ex() with create_event() by preserving the Option
    wrapper through the transmute chain. Update create_event_ex_unchecked()
    to accept Option matching create_event_unchecked().


    patina_dxe_core: Fix TplGuard Deref lifetime unsoundness

    TplGuard's Deref and DerefMut returned references with the mutex's
    lifetime ('a) instead of the guard's lifetime. This allowed extracting
    references that outlive the guard, accessing mutex-protected data
    without the lock held.

    Tie the returned reference lifetime to &self (the guard) instead of
    'a (the mutex), matching the correct pattern used in sdk/patina's
    TplMutexGuard.


    patina_macro: Reject records without #[string_pool] at compile time

    The SmbiosRecordStructure trait requires string_pool_mut() to return
    &mut Vec. Records without a #[string_pool] field previously
    used a static mut Vec which was mutable aliasing UB.

    Replace with a compile-time error requiring all records to declare a
    #[string_pool] field for trait conformance, even if never populated.
    This makes the invalid state unrepresentable rather than deferring to
    runtime.


    patina_sdk: Replace unreachable_unchecked() with unreachable in Service Deref

    The downcast failure path used unreachable_unchecked() which is UB if
    reached. While all intended construction paths guarantee correct types,
    the public From<&'static dyn Any> impl accepts any dyn Any, creating a
    reachable path to UB. Replace with unreachable! which provides a safe
    panic with a diagnostic message if the invariant is ever violated.


    boot_services: Use NonNull::dangling for ZST protocol references

    The ZST protocol path returned &'static mut T from a shared static (),
    creating aliasing &mut references across concurrent callers.

    Use NonNull::dangling() instead, which gives each caller a unique
    aligned pointer without shared state.


    runtime: Use raw pointers for linked list manipulation to avoid aliasing

    When the runtime image/event lists are empty, the previous code created
    two &mut references to the same image_head/event_head field on the same
    line.

    Follow the advice in https://rust-unofficial.github.io/too-many-lists/fifth-stacked-borrows.html#managing-stacked-borrows
    and use pointers with addr_of_mut! throughout the list code.


    fv: Use checked arithmetic for FV read address calculation

    saturating_add() silently produces usize::MAX on overflow instead of
    returning an error. A malformed FV with large LBA values could trigger
    this, leading to a wild pointer in from_raw_parts().

    Switch to checked_add and propagate InvalidParameter on overflow.


    fw_fs: Use read_unaligned() for FV header parsing from byte buffer

    The byte buffer has alignment 1 but fv::Header contains u32/u64 fields
    requiring higher alignment. Creating a reference via &*(ptr as *const
    fv::Header) is UB on unaligned data.

    Use ptr::read_unaligned() to safely copy the header out of the buffer,
    matching the approach used in patina_ffs.


    hob_list: Guard against zero-length or overflowing HOB entries

    A malformed HOB with length 0 causes an infinite loop, and a very
    large length can wrap the pointer on 32-bit targets. Break out of
    the parsing loop if the length is too small to contain a HOB header
    or if the pointer addition overflows.


    patina_mm: Use read_unaligned() for communicate buffer header parsing

    The communicate buffer header fields were read with ptr::read() which
    requires aligned pointers. The buffer comes from firmware memory that
    may not be aligned to BinaryGuid or usize requirements depending on
    the platform. Switch to ptr::read_unaligned() to be correct regardless
    of alignment.


    • Impacts functionality?
    • Impacts security?
    • Breaking change?
    • Includes tests?
    • Includes documentation?

    How This Was Tested

    • cargo make all

    Integration Instructions

    • N/A


  • Project convention updates [Rebase \& FF] @makubacki (#1469)
    Change Details
      ## Description

    A few miscellaneous changes to update code not complying with the project instructions in #1460.


    patina_stacktrace: Replace mod.rs with named module files

    Move x64/mod.rs to x64.rs and aarch64/mod.rs to aarch64.rs to follow
    the project convention of using named module files instead of mod.rs.


    patina_mm: Replace mod.rs in integration test with a named module file

    Rename mod.rs to tests_root.rs and update the test entry point to use
    a #[path] attribute redirect, eliminating mod.rs while preserving the
    existing module hierarchy and crate-level import paths.


    Replace unwrap() with expect() in a few locations

    We prefer expect() in production code to provide more context in the
    event of an error.

    Add descriptive messages to unwrap() calls in:

    • rbt.rs: RBT rebalancing where sibling/parent nodes are guaranteed
      to exist by the algorithm invariants
    • x64/runtime_function.rs: PE .pdata chunk reads validated by
      preceding size checks
    • aarch64/runtime_function.rs: same pattern for AArch64 .pdata parsing

    components: Enforce missing_docs lint in all component crates

    Add #![deny(missing_docs)] to component crate lib.rs files to enforce
    documentation requirements on public items.


    • Impacts functionality?
    • Impacts security?
    • Breaking change?
    • Includes tests?
    • Includes documentation?

    How This Was Tested

    • cargo make all

    Integration Instructions

    • N/A


  • Update rand crate dev dependency @cfernald (#1464)
    Change Details
      ## Description

    The rand crate is being rejected by cargo-deny because some of it's interfaces expose UB through safe functions. This commit updates to a fixed version to resolve the deny.

    • Impacts functionality?
    • Impacts security?
    • Breaking change?
    • Includes tests?
    • Includes documentation?

    How This Was Tested

    • Unit tests

    Integration Instructions

    N/A

      </blockquote>
      <hr>
    </details>
    
  • [Rebase\&FF] pecoff: add `get_section` functionality @Javagedes (#1409)
    Change Details
      ## Description

    This commit pulls the functionality for getting a sub-slice that represents a particular PE/COFF section from an unloaded image out of load_resource_section and makes it a re-usable function. It also removes an unnecessary allocation when comparing section names.

    NOTE: The git diff does not do this change justice due to each line's "depth" being adjusted. There is no change in logic, except for the second commit, which removes the allocation.

    • Impacts functionality?
    • Impacts security?
    • Breaking change?
    • Includes tests?
    • Includes documentation?

    How This Was Tested

    CI continues to work. Boot on Q35.

    Integration Instructions

    N/A




  • patina\_smbios: Use zerocopy::IntoBytes for SMBIOS record serialization @kat-perez (#1451)
    Change Details
      ## Description

    Replace the primitive type whitelist in the SmbiosRecord derive macro with zerocopy::IntoBytes::as_bytes(). The macro no longer validates field types at expansion time — instead, any type that implements IntoBytes is accepted, with type checking deferred to the compiler.

    This means new SMBIOS field types (enums, bitfields, etc.) can be added by deriving IntoBytes without modifying the macro.

    Re-export zerocopy from patina_smbios so consuming crates don't need a direct dependency.

    • Impacts functionality?
    • Impacts security?
    • Breaking change?
    • Includes tests?
    • Includes documentation?

    How This Was Tested

    • cargo make all passes
    • Updated macro proc tests to verify new behavior
    • Existing SMBIOS record serialization tests pass unchanged

    Integration Instructions

    SMBIOS record fields must implement zerocopy::IntoBytes. All primitive types already do. Custom types need #[derive(IntoBytes)] with an appropriate #[repr(...)].




🐛 Bug Fixes

  • patina: Use correct FPDT record types for MODULE\_DB\_END\_ID @cfernald (#1463)
    Change Details
      ## Description

    The patina implementation uses QWORD String event type for MODULE_DB_STOP_END_ID instead of MODULE_DB_END_ID, which is causing asserts when parsing the FDPT as this in unexpected. This commit fixes this to be consistent with the EDKII implementation.

    • Impacts functionality?
    • Impacts security?
    • Breaking change?
    • Includes tests?
    • Includes documentation?

    How This Was Tested

    Tested on Q35 by parsing from shell with dp

    Integration Instructions

    N/A

      </blockquote>
      <hr>
    </details>
    
  • patina\_debugger: Fix bug in writing memory across page boundary @cfernald (#1450)
    Change Details
      ## Description

    The current memory write logic walks through the memory range one page at a time to ensure that each page is accessible. However, the current logic still uses the original address when writing the memory. This means that if you write a buffer that spans a page boundary, it will overwrite the first part of the buffer. This commit fixes this bug and adds a test to check this scenario.

    • Impacts functionality?
    • Impacts security?
    • Breaking change?
    • Includes tests?
    • Includes documentation?

    How This Was Tested

    Tested with unit tests and live debugging on Q35

    Integration Instructions

    N/A

      </blockquote>
      <hr>
    </details>
    
  • patina\_internal\_collections: Fix UB memory read in Node fields @garybeihl (#1152)
    Change Details
      ## Description Fixes an undefined behavior issue where `Cell::set()` reads uninitialized memory during linked list creation in `Storage::resize()`.

    Root Cause

    • Cell::set() internally uses mem::replace(), which reads the old value before writing the new one.
    • When Storage::resize() allocates new nodes and calls build_linked_list(), the Cell fields contain uninitialized memory.
    • Reading uninitialized memory is undefined behavior, even if immediately overwritten. Unwanted compiler "optimizations" could follow.

    Impact

    • Any package using patina_internal_collections.
    • Potential memory corruption and non-deterministic errors
    • Detected by Miri testing (issue #560)

    Fix

    • Initialize all Cell fields using ptr::write() before build_linked_list()
    • Use addr_of_mut!() to read field pointers without creating references to uninitialized data

    Introduced by

    • PR #1050 (Nov 13, 2025) Replace AtomicPtr with Cell in patina_internal_collections
    • AtomicPtr::store() writes without reading the old value, but Cell::set() uses mem::replace() which reads before writing

    Related to #560

    • Impacts functionality?
    • Impacts security?
    • Breaking change?
    • Includes tests?
    • Includes documentation?

    How This Was Tested

    • Tested with: cargo +nightly-2025-09-19 miri test -p patina_dxe_core.
    • 7 tests now pass (previously 0/469 due to this UB issue).

    Integration Instructions

    N/A




🔐 Security Impacting

  • patina\_internal\_collections: Fix UB memory read in Node fields @garybeihl (#1152)
    Change Details
      ## Description Fixes an undefined behavior issue where `Cell::set()` reads uninitialized memory during linked list creation in `Storage::resize()`.

    Root Cause

    • Cell::set() internally uses mem::replace(), which reads the old value before writing the new one.
    • When Storage::resize() allocates new nodes and calls build_linked_list(), the Cell fields contain uninitialized memory.
    • Reading uninitialized memory is undefined behavior, even if immediately overwritten. Unwanted compiler "optimizations" could follow.

    Impact

    • Any package using patina_internal_collections.
    • Potential memory corruption and non-deterministic errors
    • Detected by Miri testing (issue #560)

    Fix

    • Initialize all Cell fields using ptr::write() before build_linked_list()
    • Use addr_of_mut!() to read field pointers without creating references to uninitialized data

    Introduced by

    • PR #1050 (Nov 13, 2025) Replace AtomicPtr with Cell in patina_internal_collections
    • AtomicPtr::store() writes without reading the old value, but Cell::set() uses mem::replace() which reads before writing

    Related to #560

    • Impacts functionality?
    • Impacts security?
    • Breaking change?
    • Includes tests?
    • Includes documentation?

    How This Was Tested

    • Tested with: cargo +nightly-2025-09-19 miri test -p patina_dxe_core.
    • 7 tests now pass (previously 0/469 due to this UB issue).

    Integration Instructions

    N/A




📖 Documentation Updates

  • Add Safe MMIO Docs and Example [Rebase \& FF] @makubacki (#1462)
    Change Details
      ## Description

    Resolves #1461

    docs: Add MMIO documentation and example

    Adds a new "Hardware Access" section to the developer documentation
    to host guides and conventions for accessing hardware in Patina.

    Right now, this includes a new page on Memory-Mapped I/O (MMIO) that
    explains why MMIO must be accessed through raw pointers in Rust, and
    why we're using the safe-mmio crate to do so.


    sdk/patina: Replace raw volatile MMIO with safe-mmio in PL011 UART

    Replace raw pointer volatile reads/writes for the PL011 UART code with
    safe-mmio abstractions.

    ARM PrimeCell UART (PL011) register properties taken from:

    https://developer.arm.com/documentation/ddi0183/g/programmers-model/summary-of-registers


    Re-export safe-mmio as patina::mmio

    Since the patina crate serves as the underlying SDK, this change
    allows consumers to use patina::mmio instead of depending on
    safe-mmio directly for the reasons stated in the mmio.md file
    update.


    • Impacts functionality?
    • Impacts security?
    • Breaking change?
    • Includes tests?
    • Includes documentation?

    How This Was Tested

    • cargo make all
    • mdbook build ./docs
    • QEMU SBSA boot to EFI shell with serial output

    Integration Instructions

    • N/A


  • RFC Amendment: Update major branch release notes guidance @makubacki (#1456)
    Change Details
      ## Description

    Provides guidance for preserving release notes from the major branch when it is merged into main.

    • Impacts functionality?
    • Impacts security?
    • Breaking change?
    • Includes tests?
    • Includes documentation?

    How This Was Tested

    • cargo make all
    • mdbook build ./docs

    Integration Instructions

    • N/A


  • [RFC] Rust supervisor proposal @kuqin12 (#1170)
    Change Details
      ## Description

    This proposal intends to provide a supervised Standalone MM solution with Rust.

    • Impacts functionality?
    • Impacts security?
    • Breaking change?
    • Includes tests?
    • Includes documentation?

    How This Was Tested

    RFC only.

    Integration Instructions

    N/A

      </blockquote>
      <hr>
    </details>
    

Full Changelog: patina-v21.0.1...v21.0.2