harness: fix ARM32 HARNESS_START macros missing DEFAULT_INDEX argument - #379
Open
Wenzel wants to merge 2 commits into
Open
harness: fix ARM32 HARNESS_START macros missing DEFAULT_INDEX argument#379Wenzel wants to merge 2 commits into
Wenzel wants to merge 2 commits into
Conversation
tsffs-gcc-arm32.h's HARNESS_START, HARNESS_START_WITH_MAXIMUM_SIZE, and HARNESS_START_WITH_MAXIMUM_SIZE_AND_PTR call __orr_extended2/__orr_extended3 without the DEFAULT_INDEX argument that every other architecture's header passes (compare tsffs-gcc-x86.h, tsffs-gcc-aarch64.h, tsffs-gcc-riscv32.h, which all pass it explicitly as the first pseudo-argument). TSFFS's ARM harness support (src/arch/arm.rs) expects r10 to hold the harness index and the actual arguments in r9/r8. Without DEFAULT_INDEX, the buffer pointer lands in r10 instead, which the fuzzer reads as a (bogus) harness index - the start harness is silently never recognized, logged only as a debug-level "index is not configured" message. Found and confirmed via two independent bare-metal ARM32 targets on the AST2600 arm-cortex-a7 core in Simics: a synthetic smoke-test target, and a compiled-in harness for openbmc/libpldm's decode_pldm_firmware_update_package(). Both silently produced zero fuzzing activity until this fix was applied locally; after the fix, both fired correctly and ran full fuzzing campaigns (crash detection confirmed working end to end). HARNESS_STOP and HARNESS_ASSERT already pass DEFAULT_INDEX correctly via __orr/__orr_extended1 and are left unchanged.
Adds docs/src/harnessing/bare-metal.md, covering the case of harnessing a single function (a parser or decoder, for example) that is normally part of a much larger system by compiling it into a small freestanding binary and loading it directly onto a CPU model, rather than booting the full system to reach it. Covers loading a binary directly via load-binary/set-pc, the minimal libc surface most parser/decoder code actually needs (memcpy/memcmp/ memset/__assert_fail) and how to find it via undefined-symbol inspection, a GCC ARM32 NEON-codegen gotcha that can make a harness look completely broken (every iteration faults identically on an Undefined Instruction that has nothing to do with the code under test), and querying architecture-specific exception numbers via list-exceptions rather than assuming values across CPU classes. Generalized from getting a compiled-in harness for a real daemon codebase working on an ARM Cortex-A7 core outside its native OS - none of that target-specific detail is included here, just the technique. Wired into docs/src/SUMMARY.md and docs/src/harnessing/README.md right after the existing compiled-in harnessing page, since this is a variant of that approach rather than a separate one.
There was a problem hiding this comment.
Pull request overview
Fixes ARM32 harness argument placement and adds bare-metal harnessing guidance.
Changes:
- Passes
DEFAULT_INDEXto ARM32 start macros. - Adds bare-metal/non-x86 harnessing documentation.
- Links the new guide into documentation navigation.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
harness/tsffs-gcc-arm32.h |
Corrects ARM32 start macro registers. |
docs/src/SUMMARY.md |
Adds the guide to the documentation summary. |
docs/src/harnessing/README.md |
Links the new guide. |
docs/src/harnessing/bare-metal.md |
Documents bare-metal harness setup and troubleshooting. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+156
to
+157
| __orr_extended3(N_START_BUFFER_PTR_SIZE_PTR, DEFAULT_INDEX, buffer, \ | ||
| size_ptr); \ |
Comment on lines
+48
to
+51
| is required. This works for any CPU architecture Simics models, not just | ||
| the ones with dedicated compiled-in harness headers in `harness/`: write a | ||
| small assembly entry stub for the target's calling convention and reset | ||
| behavior, and the same technique applies. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
tsffs-gcc-arm32.h'sHARNESS_START,HARNESS_START_WITH_MAXIMUM_SIZE, andHARNESS_START_WITH_MAXIMUM_SIZE_AND_PTRcall__orr_extended2/__orr_extended3without theDEFAULT_INDEXargument that every other architecture's header passes (comparetsffs-gcc-x86.h,tsffs-gcc-aarch64.h,tsffs-gcc-riscv32.h, which all pass it explicitly as the first pseudo-argument).src/arch/arm.rs) expectsr10to hold the harness index and the actual arguments inr9/r8. WithoutDEFAULT_INDEX, the buffer pointer lands inr10instead, which the fuzzer reads as a (bogus) harness index - the start harness is silently never recognized, logged only as a debug-level "index is not configured" message.HARNESS_STOPandHARNESS_ASSERTalready passDEFAULT_INDEXcorrectly via__orr/__orr_extended1and are left unchanged.docs/src/harnessing/bare-metal.md, a new page covering bare-metal/non-x86 compiled-in harnessing in general (loading a binary directly onto a CPU model, providing a minimal freestanding runtime, a cross-compiler NEON-codegen gotcha that can make a correct harness look completely broken, and querying architecture-specific exception numbers), wired intoSUMMARY.mdandharnessing/README.mdright after the existing compiled-in harnessing page.How this was found
Found and confirmed via two independent bare-metal ARM32 targets on an AST2600
arm-cortex-a7core in Simics: a synthetic smoke-test target, and a compiled-in harness foropenbmc/libpldm'sdecode_pldm_firmware_update_package(). Both silently produced zero fuzzing activity until this fix was applied locally; after the fix, both fired correctly and ran full fuzzing campaigns (crash detection confirmed working end to end).A follow-up PR adds the BMC fuzzing example (BIOS + OpenBMC + this libpldm harness) that depends on this fix - see Wenzel/tsffs#add-bmc-fuzzing-example.
Test plan
aarch64header's pattern (3-arg__orr_extended3with index first)arm-cortex-a7core:HARNESS_STARTnow correctly recognized, ran a multi-thousand-iteration fuzzing campaign with clean snapshot restoreHARNESS_STARTnow correctly recognized, ran a 10-minute fuzzing campaign that found real crashesHARNESS_STOP/HARNESS_ASSERTwere unaffected (already correct) and left untouched