Fix guest call to halt not dropping allocated trace data leading to memory leak (#1072)

* [trace] Update flatbuffers schema to define an enum guest event to send to the host

Signed-off-by: Doru Blânzeanu <[email protected]>

* [trace] Rename KeyValue to EventKeyValue

Signed-off-by: Doru Blânzeanu <[email protected]>

* [trace] Add Encoder and Decoder traits

Signed-off-by: Doru Blânzeanu <[email protected]>

* [trace] Add EventsEncoder and EventsDecoder structs

- This types help exchange GuestEvent types from guest to host
- The encoder keeps a Flatbuffer Builder and takes a maximum
capacity which when is close to be filled, it calls a function
configured at initialization.
- This is a mechanism to report to the host the serialized
events. This mechanism serializes the events as they are created
so it avoids reallocation of the Flatbuffer Builder buffer

Signed-off-by: Doru Blânzeanu <[email protected]>

* [trace] Update unit tests for the new Encoder/Decoder

Signed-off-by: Doru Blânzeanu <[email protected]>

* [trace-guest] Update tracing logic to use the encoder

Signed-off-by: Doru Blânzeanu <[email protected]>

* [trace] Update trace context to use the new decoder

Signed-off-by: Doru Blânzeanu <[email protected]>

* Add guest trace fuzzying

- The fuzzying test runs a guest function that runs a recursive function
  that creates spans and events.
- The rationale is to stress the logic that creates, serializes,
  de-serializes and emits the trace data to find errors that can crash
  the host or guest

Signed-off-by: Doru Blânzeanu <[email protected]>

* [trace-guest] Add event estimation for buffer preallocation to avoid reallocation by Flatbuffers

Signed-off-by: Doru Blânzeanu <[email protected]>

* Enable miri tests for hyperlight-common

Signed-off-by: Doru Blânzeanu <[email protected]>

---------

Signed-off-by: Doru Blânzeanu <[email protected]>

Author: dblnz

.github/workflows/Fuzzing.yml

@@ -13,7 +13,7 @@ jobs:
   fuzzing:
     uses: ./.github/workflows/dep_fuzzing.yml
     with:
-      targets: '["fuzz_host_print", "fuzz_guest_call", "fuzz_host_call"]' # Pass as a JSON array
+      targets: '["fuzz_host_print", "fuzz_guest_call", "fuzz_host_call", "fuzz_guest_estimate_trace_event", "fuzz_guest_trace"]' # Pass as a JSON array
       max_total_time: 18000 # 5 hours in seconds
     secrets: inherit
 
@@ -26,7 +26,7 @@ jobs:
     steps:
       - name: Checkout code
         uses: actions/checkout@v6
-        
+
       - name: Notify Fuzzing Failure
         run: ./dev/notify-ci-failure.sh --labels="area/fuzzing,area/testing,lifecycle/needs-review,release-blocker"
         env:

.github/workflows/ValidatePullRequest.yml

@@ -63,7 +63,7 @@ jobs:
       - docs-pr
     uses: ./.github/workflows/dep_fuzzing.yml
     with:
-      targets: '["fuzz_host_print", "fuzz_guest_call", "fuzz_host_call"]' # Pass as a JSON array
+      targets: '["fuzz_host_print", "fuzz_guest_call", "fuzz_host_call", "fuzz_guest_estimate_trace_event", "fuzz_guest_trace"]' # Pass as a JSON array
       max_total_time: 300 # 5 minutes in seconds
       docs_only: ${{needs.docs-pr.outputs.docs-only}}
     secrets: inherit

.github/workflows/dep_rust.yml

@@ -138,6 +138,13 @@ jobs:
         env: 
           TARGET_TRIPLE: ${{ inputs.target_triple }}
 
+      - name: Run Miri tests
+        if: ${{ (runner.os == 'Linux' )}}
+        env:
+          CARGO_TERM_COLOR: always
+          TARGET_TRIPLE: ${{ inputs.target_triple }}
+        run: just miri-tests
+
       - name: Run Rust tests
         env:
           CARGO_TERM_COLOR: always

Justfile

@@ -214,6 +214,12 @@ test-doc target=default-target features="":
 ### LINTING ####
 ################
 
+miri-tests:
+    rustup component add miri --toolchain nightly
+    # For now only run miri tests on hyperlight-common with trace_guest feature
+    # We can add more as needed
+    cargo +nightly miri test -p hyperlight-common -F trace_guest
+
 check:
     {{ cargo-cmd }} check  {{ target-triple-flag }}
     {{ cargo-cmd }} check -p hyperlight-host --features crashdump  {{ target-triple-flag }}
@@ -342,12 +348,14 @@ bench features="":
 fuzz_memory_limit := "4096"
 
 # Fuzzes the given target
+# Uses *case* for compatibility to determine if the target is a tracing fuzzer or not
 fuzz fuzz-target:
-    cargo +nightly fuzz run {{ fuzz-target }} --release -- -rss_limit_mb={{ fuzz_memory_limit }}
+    case "{{ fuzz-target }}" in *trace*) just fuzz-trace {{ fuzz-target }} ;; *) cargo +nightly fuzz run {{ fuzz-target }} --release -- -rss_limit_mb={{ fuzz_memory_limit }} ;; esac
 
 # Fuzzes the given target. Stops after `max_time` seconds
+# Uses *case* for compatibility to determine if the target is a tracing fuzzer or not
 fuzz-timed fuzz-target max_time:
-    cargo +nightly fuzz run {{ fuzz-target }} --release -- -rss_limit_mb={{ fuzz_memory_limit }} -max_total_time={{ max_time }}
+    case "{{ fuzz-target }}" in *trace*) just fuzz-trace-timed {{ max_time }} {{ fuzz-target }} ;; *) cargo +nightly fuzz run {{ fuzz-target }} --release -- -rss_limit_mb={{ fuzz_memory_limit }} -max_total_time={{ max_time }} ;; esac
 
 # Builds fuzzers for submission to external fuzzing services
 build-fuzzers: (build-fuzzer "fuzz_guest_call") (build-fuzzer "fuzz_host_call") (build-fuzzer "fuzz_host_print")
@@ -356,6 +364,28 @@ build-fuzzers: (build-fuzzer "fuzz_guest_call") (build-fuzzer "fuzz_host_call")
 build-fuzzer fuzz-target:
     cargo +nightly fuzz build {{ fuzz-target }}
 
+# Fuzzes the guest with tracing enabled
+fuzz-trace fuzz-target="fuzz_guest_trace":
+    # We need to build the trace guest with the trace feature enabled
+    just build-rust-guests release trace_guest
+    just move-rust-guests release
+    RUST_LOG="trace,hyperlight_guest=trace,hyperlight_guest_bin=trace" cargo +nightly fuzz run {{ fuzz-target }} --features trace --release -- -rss_limit_mb={{ fuzz_memory_limit }}
+    # Rebuild the trace guest without the trace feature to avoid affecting other tests
+    just build-rust-guests release
+    just move-rust-guests release
+
+# Fuzzes the guest with tracing enabled. Stops after `max_time` seconds
+fuzz-trace-timed max_time fuzz-target="fuzz_guest_trace":
+    # We need to build the trace guest with the trace feature enabled
+    just build-rust-guests release trace_guest
+    just move-rust-guests release
+    RUST_LOG="trace,hyperlight_guest=trace,hyperlight_guest_bin=trace" cargo +nightly fuzz run {{ fuzz-target }} --features trace --release -- -rss_limit_mb={{ fuzz_memory_limit }} -max_total_time={{ max_time }}
+    # Rebuild the trace guest without the trace feature to avoid affecting other tests
+    just build-rust-guests release
+    just move-rust-guests release
+
+build-trace-fuzzers:
+    cargo +nightly fuzz build fuzz_guest_trace --features trace
 
 ###################
 ### FLATBUFFERS ###

fuzz/Cargo.toml

@@ -14,8 +14,8 @@ hyperlight-host = { workspace = true, default-features = true, features = ["fuzz
 hyperlight-common = {workspace = true}
 
 [[bin]]
-name = "fuzz_host_print"
-path = "fuzz_targets/host_print.rs"
+name = "fuzz_guest_estimate_trace_event"
+path = "fuzz_targets/guest_estimate_trace_event.rs"
 test = false
 doc = false
 bench = false
@@ -27,9 +27,27 @@ test = false
 doc = false
 bench = false
 
+[[bin]]
+name = "fuzz_guest_trace"
+path = "fuzz_targets/guest_trace.rs"
+test = false
+doc = false
+bench = false
+
 [[bin]]
 name = "fuzz_host_call"
 path = "fuzz_targets/host_call.rs"
 test = false
 doc = false
-bench = false
+bench = false
+
+[[bin]]
+name = "fuzz_host_print"
+path = "fuzz_targets/host_print.rs"
+test = false
+doc = false
+bench = false
+
+[features]
+default = []
+trace = ["hyperlight-host/trace_guest", "hyperlight-common/trace_guest"]

fuzz/fuzz_targets/guest_estimate_trace_event.rs

@@ -0,0 +1,170 @@
+/*
+Copyright 2025  The Hyperlight Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+#![no_main]
+
+#[cfg(not(feature = "trace"))]
+compile_error!("feature `trace` must be enabled to fuzz guest trace event estimation");
+
+use hyperlight_common::flatbuffer_wrappers::guest_trace_data::{
+    EventKeyValue, EventsBatchDecoder, EventsBatchEncoder, EventsDecoder, EventsEncoder,
+    GuestEvent, estimate_event,
+};
+use libfuzzer_sys::arbitrary::{Arbitrary, Result as FuzzResult, Unstructured};
+use libfuzzer_sys::fuzz_target;
+
+const MAX_STRING_LEN: usize = 1 << 10; // 1024 bytes
+const MAX_FIELDS: usize = 32;
+
+/// Wrapper around GuestEvent to implement Arbitrary
+#[derive(Debug)]
+struct EventInput(GuestEvent);
+
+impl EventInput {
+    /// Consumes the wrapper and returns the inner GuestEvent
+    fn into_inner(self) -> GuestEvent {
+        self.0
+    }
+}
+
+impl<'a> Arbitrary<'a> for EventInput {
+    fn arbitrary(u: &mut Unstructured<'a>) -> FuzzResult<Self> {
+        // Choose a variant of GuestEvent to generate
+        let discriminator = u.arbitrary::<u8>()? % 5;
+
+        // Generate each variant with appropriate random data
+        let event = match discriminator {
+            0 => GuestEvent::OpenSpan {
+                id: u.arbitrary::<u64>()?,
+                parent_id: arbitrary_parent(u)?,
+                name: limited_string(u, MAX_STRING_LEN)?,
+                target: limited_string(u, MAX_STRING_LEN)?,
+                tsc: u.arbitrary::<u64>()?,
+                fields: arbitrary_fields(u)?,
+            },
+            1 => GuestEvent::CloseSpan {
+                id: u.arbitrary::<u64>()?,
+                tsc: u.arbitrary::<u64>()?,
+            },
+            2 => GuestEvent::LogEvent {
+                parent_id: u.arbitrary::<u64>()?,
+                name: limited_string(u, MAX_STRING_LEN)?,
+                tsc: u.arbitrary::<u64>()?,
+                fields: arbitrary_fields(u)?,
+            },
+            3 => GuestEvent::EditSpan {
+                id: u.arbitrary::<u64>()?,
+                fields: arbitrary_fields(u)?,
+            },
+            _ => GuestEvent::GuestStart {
+                tsc: u.arbitrary::<u64>()?,
+            },
+        };
+
+        Ok(EventInput(event))
+    }
+}
+
+/// Generates an optional parent ID
+fn arbitrary_parent(u: &mut Unstructured<'_>) -> FuzzResult<Option<u64>> {
+    let has_parent = u.arbitrary::<bool>()?;
+    if has_parent {
+        Ok(Some(u.arbitrary::<u64>()?))
+    } else {
+        Ok(None)
+    }
+}
+
+/// Generates a String with a maximum length of `max_len`
+fn limited_string(u: &mut Unstructured<'_>, max_len: usize) -> FuzzResult<String> {
+    let bytes = u.arbitrary::<&[u8]>()?;
+    let s = std::str::from_utf8(bytes)
+        // Fallback to repeating 'x' if not valid UTF-8
+        .unwrap_or(&"x".repeat(bytes.len() % max_len))
+        .chars()
+        .take(max_len)
+        .collect::<String>();
+
+    Ok(s)
+}
+
+/// Generates a vector of EventKeyValue pairs
+fn arbitrary_fields(u: &mut Unstructured<'_>) -> FuzzResult<Vec<EventKeyValue>> {
+    let field_count = (u.arbitrary::<u8>()? as usize).min(MAX_FIELDS);
+    let mut fields = Vec::with_capacity(field_count);
+    for _ in 0..field_count {
+        let key = limited_string(u, MAX_STRING_LEN)?;
+        let value = limited_string(u, MAX_STRING_LEN)?;
+        fields.push(EventKeyValue { key, value });
+    }
+    Ok(fields)
+}
+
+/// Encodes a GuestEvent into a byte vector
+fn encode(event: &GuestEvent) -> Vec<u8> {
+    // Use the estimate plus some slack to avoid reallocation during encoding
+    let mut encoder = EventsBatchEncoder::new(estimate_event(event).saturating_add(256), |_| {});
+    encoder.encode(event);
+    encoder.finish().to_vec()
+}
+
+/// Decodes a byte slice into a GuestEvent
+fn decode(data: &[u8]) -> Option<GuestEvent> {
+    let decoder = EventsBatchDecoder {};
+    let mut events = decoder.decode(data).ok()?;
+
+    if events.len() == 1 {
+        events.pop()
+    } else {
+        None
+    }
+}
+
+/// Asserts that the estimated size is within acceptable bounds of the actual size
+/// Allows for a 10% slack or minimum of 128 bytes
+fn assert_estimate_bounds(actual: usize, estimate: usize) {
+    assert!(
+        estimate >= actual,
+        "estimate {} smaller than actual {}",
+        estimate,
+        actual,
+    );
+
+    let slack = (actual / 10).max(128);
+    let upper_bound = actual + slack;
+    assert!(
+        estimate <= upper_bound,
+        "estimate {} larger than allowable upper bound {} (actual {})",
+        estimate,
+        upper_bound,
+        actual,
+    );
+}
+
+fuzz_target!(|input: EventInput| {
+    let event = input.into_inner();
+
+    // Get the size estimate
+    let estimate = estimate_event(&event);
+    // Encode the event
+    let encoded_data = encode(&event);
+    // Assert that the estimate is within bounds
+    assert_estimate_bounds(encoded_data.len(), estimate);
+
+    // Decode the event back
+    let decoded = decode(&encoded_data).expect("decoding failed");
+    // Assert that the decoded event matches the original
+    assert_eq!(event, decoded);
+});