Skip to content

opentmk: add invariant runner - #4332

Open
wanghenry-msft wants to merge 32 commits into
microsoft:mainfrom
wanghenry-msft:user/wanghenry/tmk-inv
Open

opentmk: add invariant runner#4332
wanghenry-msft wants to merge 32 commits into
microsoft:mainfrom
wanghenry-msft:user/wanghenry/tmk-inv

Conversation

@wanghenry-msft

@wanghenry-msft wanghenry-msft commented Aug 27, 2026

Copy link
Copy Markdown
Contributor

This adds invariant runner/executor on top of opentmk

Invariant is a fuzzer framework, with opentmk-invariant being the opentmk program that takes in a set of commands (currently only via serial I/O) and executes them determinant on the invariant fuzzer.

@wanghenry-msft
wanghenry-msft requested a review from a team as a code owner August 27, 2026 17:06
Copilot AI lite review requested due to automatic review settings August 27, 2026 17:06
@github-actions github-actions Bot added the unsafe Related to unsafe code label Aug 27, 2026
@github-actions

Copy link
Copy Markdown

⚠️ Unsafe Code Detected

This PR modifies files containing unsafe Rust code. Extra scrutiny is required during review.

For more on why we check whole files, instead of just diffs, check out the Rustonomicon

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR adds an “invariant” execution target on top of OpenTMK, introducing a new opentmk_invariant runtime that receives serialized commands over serial I/O, decodes them (initially via a syzkaller-oriented decoder), and dispatches mapped fuzz functions (I/O port operations + Hyper-V hypercalls).

Changes:

  • Introduces new crates inv_packet (wire protocol) and inv_decoder (syz program decode/exec) plus a new opentmk_invariant UEFI/no_std entrypoint with serial comms, executor loop, and deserializer plumbing.
  • Extends workspace dependencies/members to include the new crates and adjusts anyhow workspace defaults (disabling default features; enabling std where required).
  • Adds unit tests for executor/comms and the syzlang deserializer mapping/dispatch behavior.

Reviewed changes

Copilot reviewed 27 out of 28 changed files in this pull request and generated 8 comments.

Show a summary per file
File Description
xtask/Cargo.toml Enables anyhow std feature explicitly after workspace default-features change.
flowey/flowey_hvlite/Cargo.toml Enables anyhow std feature explicitly after workspace default-features change.
Cargo.toml Adds new workspace members/deps; makes anyhow default-features=false; adds postcard/num-traits workspace deps.
Cargo.lock Locks new crates and dependency changes.
opentmk/opentmk_invariant/Cargo.toml New invariant runtime crate manifest and deps.
opentmk/opentmk_invariant/src/main.rs New invariant entrypoint + UEFI entry + executor loop startup.
opentmk/opentmk_invariant/src/rt.rs UEFI-only panic handler module.
opentmk/opentmk_invariant/src/prelude.rs Prelude to bridge alloc/std for UEFI vs host builds.
opentmk/opentmk_invariant/src/comms/mod.rs Serial handshake + packet framing + blocking read/write implementation.
opentmk/opentmk_invariant/src/comms/test.rs Unit tests for handshake and packet framing/parse.
opentmk/opentmk_invariant/src/executor/mod.rs Core executor, configuration handling, and dispatch loop.
opentmk/opentmk_invariant/src/executor/test.rs Unit tests for executor protocol behavior and response packets.
opentmk/opentmk_invariant/src/deserializer/mod.rs Deserializer trait + module wiring.
opentmk/opentmk_invariant/src/deserializer/syzlang/mod.rs Syzlang deserializer integration with inv_decoder + mapping + dispatch.
opentmk/opentmk_invariant/src/deserializer/syzlang/test.rs Unit tests for call mapping and pointer/copyin behavior.
opentmk/opentmk_invariant/src/functions/mod.rs Function module registry exports.
opentmk/opentmk_invariant/src/functions/registry.rs Name-to-function registry and dispatch helper.
opentmk/opentmk_invariant/src/functions/variable.rs Fuzz function variable model + arg validation helper.
opentmk/opentmk_invariant/src/functions/io_port/mod.rs I/O port read/write fuzz functions with port allowlist.
opentmk/opentmk_invariant/src/functions/hyperv.rs Hyper-V hypercall fuzz function wrapper.
opentmk/opentmk_invariant/src/functions/hvcall_meta.rs Hvcall metadata wire decoding helper.
opentmk/inv_packet/Cargo.toml New wire-protocol crate manifest.
opentmk/inv_packet/src/lib.rs Packet types + magic constants for serial framing/handshake.
opentmk/inv_decoder/Cargo.toml New syzkaller decode/exec crate manifest.
opentmk/inv_decoder/src/wire.rs On-the-wire structs and constants for syz format.
opentmk/inv_decoder/src/safememory.rs Safe memory map abstraction for copyin/copyout.
opentmk/inv_decoder/src/atomicrefqueue.rs Concurrent pop queue used by decoder execution.
opentmk/inv_decoder/src/lib.rs Main decode/execute engine + copyin/copyout helpers + tests.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread opentmk/opentmk_invariant/src/functions/hvcall_meta.rs
Comment thread opentmk/opentmk_invariant/src/comms/mod.rs Outdated
Comment thread opentmk/opentmk_invariant/src/comms/mod.rs
Comment thread opentmk/opentmk_invariant/src/deserializer/syzlang/mod.rs
Comment thread opentmk/opentmk_invariant/src/executor/mod.rs Outdated
Comment thread opentmk/inv_decoder/src/lib.rs
Comment thread opentmk/inv_decoder/src/lib.rs
Comment thread opentmk/opentmk_invariant/src/functions/hyperv.rs
Copilot AI review requested due to automatic review settings August 27, 2026 17:15
Comment thread Cargo.toml Outdated

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 27 out of 28 changed files in this pull request and generated no new comments.

Suppressed comments (6)

Previously missed (5) — in code that hasn't changed since the last review.

opentmk/inv_decoder/src/atomicrefqueue.rs:67

  • AtomicRefQueue::pop_ref_conditional updates start_idx using the slice-relative idx and compares it to the absolute start_idx. When start_idx > 0 this can store a smaller (relative) value and cause rescans (or prevent advancing at all), defeating the optimization and potentially looping over already-checked entries.
                        if update_start_idx && idx > start_idx {
                            // Just use the current idx as the start idx, even though the current is currently being
                            // returned and marked. Prevents the need for checking edge cases e.g. if the current idx is the
                            // last.
                            //

opentmk/opentmk_invariant/src/functions/hyperv.rs:53

  • hvcall allocates vec![0; input_len] directly from the fuzzer-provided input_len. Even though HvTestCtx::hypercall clamps to the hypercall page size internally, this allocation can still be attacker-controlled and cause OOM / watchdog resets.
    let meta = unpack_hvcall_meta(meta.expect_int("meta")?);
    let input = input.expect_int("input")? as usize;
    let input_len = input_len.expect_int("input_len")? as usize;

    // Read in the input page from `input` (only if any input is
    // expected — `void`-input hypercalls send `input_len == 0`).
    let mut hvc_lock = CALLS.lock();
    let (hvc, init) = &mut *hvc_lock;
    if !*init {
        hvc.init(Vtl::Vtl0)
            .map_err(|e| format!("Failed to initialize HvTestCtx: {e}"))?;
        *init = true;
    }

    let mut in_args = vec![0; input_len];
    match mem.try_read_mem(input, &mut in_args) {

opentmk/inv_decoder/src/lib.rs:686

  • read_arg can panic on malformed input (cnt == 0) and also uses todo!() for arg type 3. Since this parser processes untrusted on-wire program data, these should be surfaced as recoverable decode errors (Err) rather than crashing the whole executor.
            // Read out each data word.
            let cnt = arg.size.div_ceil(8);
            if cnt == 0 {
                panic!("data argument with size of zero!?");
            }

            let mut words = Vec::new();
            for _i in 0..cnt {
                words.push(read_inc_input(buf).context("unexpected end of input")?);
            }

            Arg::Data((arg, words))
        }
        // arg_csum
        0x3 => todo!(),
        // Catchall
        _ => bail!("unsupported argument type: {typ}"),

opentmk/inv_decoder/src/lib.rs:307

  • exec_single uses SafeMemoryMap::write_mem for ARG_DATA copyins. write_mem panics on partial/failed writes, so malformed/untrusted addresses can crash the decoder instead of returning a structured error to the caller.
                Arg::Data((a, d)) => {
                    mem.write_mem(
                        (i.wire.addr + COPYIN_OFFSET) as usize,
                        &d.as_bytes()[..a.size as usize],
                    );
                }

Cargo.toml:477

  • Switching the workspace anyhow dependency to default-features = false changes the default for every crate that uses anyhow.workspace = true. Only a couple of crates were updated to explicitly re-enable features = ["std"], but many other std crates (e.g. flowey_cli) still depend on anyhow.workspace = true and will now build against a no-std-configured anyhow, which is likely to cause build breaks or behavior changes (missing std/backtrace integration).
# --- Error handling ---
anyhow = { version = "1.0", default-features = false }
thiserror = { version = "2", default-features = false }

opentmk/opentmk_invariant/src/comms/mod.rs:122

  • read_packet_blocking trusts the on-wire payload size. A large payload_sz can cause huge allocations (via Vec::with_capacity(payload_sz as usize)) and long blocking reads, which is an easy DoS vector if the serial stream is corrupted or hostile.
        //2. Read packet size
        let payload_sz = self.read_u64_blocking();

        //3. Read payload
        let mut buffer: Vec<u8> = Vec::with_capacity(payload_sz as usize);

Copilot AI review requested due to automatic review settings August 27, 2026 17:44

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 27 out of 28 changed files in this pull request and generated 3 comments.

Suppressed comments (6)

Previously missed (3) — in code that hasn't changed since the last review.

opentmk/inv_decoder/src/atomicrefqueue.rs:68

  • idx from enumerate() is relative to self.data[start_idx..], but the code compares it to start_idx (an absolute index) and stores the relative idx back into start_idx. Once start_idx != 0, this prevents start_idx from advancing (and can even move it backwards), causing unnecessary rescans.
                        if update_start_idx && idx > start_idx {
                            // Just use the current idx as the start idx, even though the current is currently being
                            // returned and marked. Prevents the need for checking edge cases e.g. if the current idx is the
                            // last.
                            //

opentmk/opentmk_invariant/src/main.rs:8

  • Doc comment has a few grammar/spelling issues (e.g., "customly" and missing terminal punctuation), which makes the crate-level documentation harder to read/search.
//! Opentmk Invariant is a bare-bones operating system based off of opentmk
//! framework used to accept customly crafted program consisting of an encoded
//! series of functions that would invoke specific functions into this OS

opentmk/opentmk_invariant/src/prelude.rs:5

  • Doc comment grammar: "a extended" should be "an extended", and "rust" should be capitalized as "Rust" in docs.
//! This is a extended prelude crate that imports a number of common rust API entities that
//! would've been imported from the `alloc` crate.

opentmk/opentmk_invariant/src/functions/hvcall_meta.rs:31

  • size_of isn't in scope here, so this assertion will fail to compile (cannot find function size_of in this scope). Prefer fully qualifying the call (or importing it explicitly) to make the check robust.
const _: () = assert!(size_of::<HvcallMeta>() == 8);

opentmk/opentmk_invariant/src/deserializer/syzlang/mod.rs:149

  • src.len() is used to slice self.tc_slice without bounds checking. Since the testcase bytes originate from the serial packet, an oversized payload will panic here and take down the runtime rather than returning a structured error.
        let src = &testcase.testcase_vcpu0.as_slice();
        self.tc_slice[..src.len()].copy_from_slice(src);
        self.mem.0.as_mut_slice().fill(0);

opentmk/opentmk_invariant/src/executor/mod.rs:96

  • Method names on_recieve_ack_packet / on_recieve_error_packet are misspelled (recieve -> receive). Since these are part of the executor's API surface and are used at call sites, it’s better to fix the spelling now before the interface is consumed more widely.
            OpenTMKPacket::Ack(a) => self.on_recieve_ack_packet(&a)?,
            OpenTMKPacket::Error(a) => self.on_recieve_error_packet(&a)?,
        };

Comment thread opentmk/inv_packet/src/lib.rs Outdated
Comment thread opentmk/opentmk_invariant/src/deserializer/syzlang/mod.rs
Comment thread opentmk/inv_decoder/src/safememory.rs
@github-actions

Copy link
Copy Markdown

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

There is a confirmed indexing bug in AtomicRefQueue’s start_idx update logic and the workspace-wide anyhow feature flip is likely to cause widespread unintended build/behavior changes unless re-scoped.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Suppressed comments (4)

Previously missed (4) — in code that hasn't changed since the last review.

opentmk/inv_decoder/src/atomicrefqueue.rs:67

  • start_idx update uses idx from enumerate() (relative to start_idx) but compares it to/stores it as if it were an absolute index. This prevents start_idx from advancing correctly once start_idx != 0, and can also move start_idx backwards, leading to repeated rescans and O(n^2) behavior under contention.
    opentmk/inv_decoder/Cargo.toml:12
  • inv_decoder is #![no_std], so it should explicitly disable anyhow default features to avoid accidentally pulling in std if the workspace dependency is std-enabled (which is the typical configuration for the rest of the workspace).
    opentmk/opentmk_invariant/src/functions/io_port/mod.rs:100
  • Typo in comment/log message: this path uses arch::io::inw, but the text says inh. This makes grepping/debugging I/O port reads harder.
    opentmk/opentmk_invariant/src/main.rs:8
  • Doc comment has a spelling/grammar issue: “customly crafted program” should be “custom-crafted program” (or similar) to read correctly.
  • Files reviewed: 28/29 changed files
  • Comments generated: 1
  • Review effort level: Lite

Comment thread Cargo.toml Outdated
Copilot AI review requested due to automatic review settings September 2, 2026 17:53

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

There is at least one confirmed build-breaking issue (missing size_of import/qualification) plus a concrete indexing bug affecting the new AtomicRefQueue optimization logic.

Review details

Suppressed comments (2)

Previously missed (1) — in code that hasn't changed since the last review.

opentmk/inv_decoder/src/atomicrefqueue.rs:67

  • idx from enumerate() is relative to start_idx, but the code compares it to (and stores it into) the absolute start_idx. This prevents start_idx from ever advancing in the sequential case and can even move it backwards if the condition were changed, leading to unnecessary rescans.

opentmk/opentmk_invariant/src/functions/hvcall_meta.rs:31

  • size_of isn’t in scope in this module, so this compile-time assert will fail to compile. Fully-qualify it (or import core::mem::size_of) so the file builds standalone.
const _: () = assert!(size_of::<HvcallMeta>() == 8);
  • Files reviewed: 28/29 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

Copilot AI review requested due to automatic review settings September 2, 2026 21:05

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

It contains at least one confirmed logic issue (AtomicRefQueue’s start_idx update uses a relative index) plus a misleading size requirement doc comment that should be corrected.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details
  • Files reviewed: 26/27 changed files
  • Comments generated: 2
  • Review effort level: Lite

Comment thread opentmk/inv_decoder/src/atomicrefqueue.rs
Comment thread opentmk/inv_decoder/src/lib.rs Outdated
Copilot AI review requested due to automatic review settings September 2, 2026 21:13

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

A verified compile error (size_of not in scope) and a mismatched/incorrect public doc comment should be fixed before approval.

Review details

Suppressed comments (2)

Previously missed (1) — in code that hasn't changed since the last review.

opentmk/inv_decoder/src/lib.rs:825

  • This doc comment refers to an addr_size parameter that doesn’t exist and also claims 0x1000000 is 4MB (it’s 16MiB). Update it to describe the actual syz_exec_mem/syz_input_buffer sizing requirements.

opentmk/opentmk_invariant/src/functions/hvcall_meta.rs:31

  • size_of is not in scope here (and there’s no local import), so this assertion won’t compile. Fully-qualify it to core::mem::size_of (or add an explicit import).
const _: () = assert!(size_of::<HvcallMeta>() == 8);
  • Files reviewed: 26/27 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

Copilot AI review requested due to automatic review settings September 2, 2026 21:22

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

inv_decoder can return early on dependent-call failure without marking results executed, which can block later scheduling and cause incomplete/incorrect program execution.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Suppressed comments (1)

Previously missed (1) — in code that hasn't changed since the last review.

opentmk/inv_packet/src/lib.rs:39

  • OpenTMKGrammarDeserializer::None is documented as “No encoding — just use raw bytes”, but the executor currently treats None as an invalid configuration (NoDeserializerEnabled). Either support the raw-bytes mode or update this doc comment to match the actual behavior to avoid misleading protocol users.
  • Files reviewed: 26/27 changed files
  • Comments generated: 1
  • Review effort level: Lite

Comment thread opentmk/inv_decoder/src/lib.rs Outdated
Copilot AI review requested due to automatic review settings September 2, 2026 21:35

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

There are confirmed correctness/compile issues in newly added code paths (e.g., missing size_of qualification/import and unchecked u64usize payload sizing in comms) that should be fixed before approval.

Review details

Suppressed comments (3)

Previously missed (2) — in code that hasn't changed since the last review.

opentmk/opentmk_invariant/src/comms/mod.rs:89

  • payload_sz is read as a u64 but then used both as a usize (Vec capacity) and as the loop bound. On 32-bit targets this can truncate and/or make it impossible to allocate/read consistently. Convert to usize with a checked conversion before allocating/looping.
    opentmk/inv_packet/src/lib.rs:40
  • OpenTMKGrammarDeserializer::None is documented as a valid “raw bytes” mode, but the executor currently treats it as an error (NoDeserializerEnabled). Either implement the raw-bytes mode or update this doc comment to reflect that None is not supported.

opentmk/opentmk_invariant/src/functions/hvcall_meta.rs:31

  • size_of is referenced without being in scope; this won’t compile unless size_of has been imported. Prefer fully qualifying it in the const-assert to avoid relying on an implicit import.
const _: () = assert!(size_of::<HvcallMeta>() == 8);
  • Files reviewed: 27/28 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

Copilot AI review requested due to automatic review settings September 2, 2026 21:42

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

There are verified compile-breaking issues in newly added code paths (X86_REGISTRY iteration destructuring and an unqualified size_of use) that must be fixed before approval.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Suppressed comments (1)

opentmk/opentmk_invariant/src/functions/hvcall_meta.rs:31

  • size_of::<HvcallMeta>() is used without being in scope; this file doesn't import it, and size_of is not in the Rust prelude. This will fail to compile (cannot find function size_of in this scope). Prefer fully qualifying it.
const _: () = assert!(size_of::<HvcallMeta>() == 8);
  • Files reviewed: 27/28 changed files
  • Comments generated: 1
  • Review effort level: Lite

Comment thread opentmk/opentmk_invariant/src/executor/mod.rs
@github-actions

github-actions Bot commented Sep 2, 2026

Copy link
Copy Markdown

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

unsafe Related to unsafe code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants