Add fuzzing - #52
Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds a cargo-fuzz setup for ms-tcg-tpm-sys, including multiple fuzz targets for TPM command streams and persisted blobs, and updates the build script to instrument the vendored C TPM library when building under fuzzing.
Changes:
- Add a dedicated
fuzz/workspace containing shared harness code, targets, dictionary, and docs for running libFuzzer. - Update
build.rsto detect fuzz builds (CARGO_CFG_FUZZING) and configure clang + sanitizer/coverage flags for the C TPM build. - Document fuzzing usage and repository layout updates in the top-level README.
Reviewed changes
Copilot reviewed 12 out of 30 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| README.md | Documents the new fuzz/ directory and how to run fuzzing. |
| fuzz/tpm.dict | Adds a libFuzzer dictionary with TPM wire-format constants. |
| fuzz/src/lib.rs | Implements shared deterministic platform callbacks and a reusable TPM instance + helpers. |
| fuzz/README.md | Adds detailed fuzzing documentation (targets, oracles, determinism, instrumentation). |
| fuzz/fuzz_targets/fuzz_tpm.rs | Fuzz target for raw TPM command stream dispatch via execute_command. |
| fuzz/fuzz_targets/fuzz_tpm_session.rs | Fuzz target for sequences of platform operations + commands. |
| fuzz/fuzz_targets/fuzz_restore_state.rs | Fuzz target for hostile/corrupted saved-state blobs. |
| fuzz/fuzz_targets/fuzz_nvmem.rs | Fuzz target for corrupted persisted nvmem blobs + boot/command execution. |
| fuzz/Cargo.toml | Adds the fuzz crate package definition and feature wiring to the root crate. |
| fuzz/Cargo.lock | Locks fuzz crate dependencies for reproducibility. |
| fuzz/.gitignore | Ignores fuzzing outputs (target/corpus/artifacts/coverage). |
| Cargo.toml | Adds cc as a build-dependency to support fuzzing-time compiler detection. |
| Cargo.lock | Records the new direct cc build-dependency for the root crate. |
| build.rs | Adds fuzzing-time C instrumentation configuration and warnings for prebuilt libs. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 12 out of 30 changed files in this pull request and generated 3 comments.
Suppressed comments (3)
fuzz/src/lib.rs:127
size_ofisn't in the prelude; this won't compile as written. Usestd::mem::size_of::<u64>()(or import it) when chunking the buffer.
for chunk in buf.chunks_mut(size_of::<u64>()) {
fuzz/README.md:29
- This corpus example uses
fuzz/...paths, which would becomefuzz/fuzz/...once youcd fuzz(and they also won’t work from the repo root since the fuzz crate isn’t in the root workspace). After running fromfuzz/, drop the leadingfuzz/prefixes and point-dictattpm.dict.
mkdir -p fuzz/corpus/fuzz_tpm
cargo +nightly fuzz run fuzz_tpm fuzz/corpus/fuzz_tpm fuzz/seed_corpus/fuzz_tpm \
-- -dict=fuzz/tpm.dict
fuzz/README.md:40
- After switching to running from inside
fuzz/(as the earlier section needs), these artifact paths should also drop the leadingfuzz/prefix; otherwise they point at a non-existentfuzz/fuzz/...location.
cargo +nightly fuzz run fuzz_tpm fuzz/artifacts/fuzz_tpm/crash-<hash>
cargo +nightly fuzz tmin fuzz_tpm fuzz/artifacts/fuzz_tpm/crash-<hash>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 12 out of 30 changed files in this pull request and generated 1 comment.
Suppressed comments (1)
fuzz/src/lib.rs:42
size_of::<u64>()is used later (e.g., inget_crypt_random) butsize_ofisn’t imported, so this crate won’t compile.
use std::sync::Mutex;
use std::sync::OnceLock;
use std::sync::atomic::AtomicU64;
use std::sync::atomic::Ordering::Relaxed;
use std::time::Duration;
| //! The blob always stays the full `NV_MEMORY_SIZE`. The platform now rejects | ||
| //! any other size up front - a shorter region let the TPM library address NV | ||
| //! memory that wasn't there - so mutating the length here would just bounce off | ||
| //! that check and waste the iteration. `tests/nvmem_size.rs` covers it instead. |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 12 out of 129 changed files in this pull request and generated no new comments.
Suppressed comments (1)
fuzz/fuzz_targets/fuzz_nvmem.rs:20
- The docs reference
tests/nvmem_size.rs, but there is no such file in this repo. This makes it harder to find where theNV_MEMORY_SIZEenforcement actually lives.
//! The blob always stays the full `NV_MEMORY_SIZE`. The platform now rejects
//! any other size up front - a shorter region let the TPM library address NV
//! memory that wasn't there - so mutating the length here would just bounce off
//! that check and waste the iteration. `tests/nvmem_size.rs` covers it instead.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 12 out of 129 changed files in this pull request and generated no new comments.
Suppressed comments (1)
fuzz/src/lib.rs:584
size_ofisn’t in scope here, so this won’t compile (it’sstd::mem::size_of). Either import it or fully-qualify the call.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 12 out of 136 changed files in this pull request and generated no new comments.
Suppressed comments (1)
fuzz/src/lib.rs:612
size_ofis used without being imported/qualified, which will not compile. Prefer qualifying withstd::mem::size_of(or add ause std::mem::size_of;import).
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 11 out of 135 changed files in this pull request and generated no new comments.
Suppressed comments (2)
Previously missed (1) — in code that hasn't changed since the last review.
fuzz/src/lib.rs:649
- The module docs say only one
MsTpm185Platformcan be live per process because the underlying C library uses globals, butthread_local!allows oneFuzzTpmper thread. If the fuzzer/runtime ever runs targets on multiple threads, this can panic or race on the C globals. Consider using a single process-globalOnceLock<Mutex<FuzzTpm>>instead.
fuzz/src/lib.rs:612
size_of::<u64>()is used here butsize_ofisn’t in scope, so the fuzz crate won’t compile. Use a fully-qualified path (or importstd::mem::size_of).
This is entirely AI written, and I haven't reviewed it at all, so I'm not sure I really want to merge it just yet. However it has found bugs, so I at least want to keep it pushed and available.