Host-side coverage of the whole codebase (found a storage buffer overflow)#6
Merged
Merged
Conversation
… host stubs Add a whole-codebase coverage path to test/Makefile: every first-party .c (hardware/, network/, serial/, storage/, shared/, main.c) is compiled instrumented but unlinked, so a .gcno alone lands it in the report. A --initial baseline (all .gcno at 0%) merged with the harness run data yields a report spanning the whole firmware -- base32/base64/random at their real %, the 15 HW/IO/logic modules at an honest 0%. Add declaration-only host stubs under test/stub/ for the pico-sdk, cyw43, lwip and mbedtls headers those sources include; storage.c/backup.c build against the real littlefs with the same LFS_NO_MALLOC/LFS_NO_DEBUG defines the Pico CMake uses. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Build harness_totp.c + shared/totp.c against the system libmbedtls-dev (real HMAC-SHA1) instead of the compile-only stub, using `-idirafter stub` so the real <mbedtls/md.h> wins while hardware/rtc.h stays stubbed. clock_get_unix_time() is injected in the harness for deterministic time. Asserts the standard RFC 6238 SHA1 test vectors (seed "12345678901234567890") and exercises totp_verify: RTC-not-set, correct code, the T-1/T/T+1 window, and out-of-window/wrong code rejection. shared/totp.c goes 0% -> 100% lines (25/25), 100% funcs (2/2), 100% branches (6/6). Wired into asan, valgrind, and coverage targets. CI note: the test job must apt-install libmbedtls-dev. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…c + backup.c Exercise storage/storage.c and storage/backup.c host-side against the real littlefs, backed by a RAM XIP window (MAP_FIXED at XIP_BASE+STORAGE_FLASH_OFFSET so storage.c's absolute-address flash_read hits host RAM; program/erase shims give NOR semantics). Covers key CRUD, list, update, wifi cfg, backup export/import roundtrip and every rejection path. storage.c 0 -> 92.5% lines, backup.c 0 -> 91.8% lines. Fixes a genuine buffer overflow ASan caught: LFS_CFG.lookahead_size was set to LFS_LOOKAHEAD (64) while lfs_lookahead_buf is only LFS_LOOKAHEAD/8 (8) bytes; littlefs memsets lookahead_size bytes into that buffer, a 56-byte global overflow on every mount/format (on real hardware too). Derive lookahead_size from sizeof(buffer) so the two can never diverge. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…t job Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The github-pages environment is branch-protected to the default branch, so a deploy from this branch is rejected regardless of the workflow. The coverage build itself is already exercised by the test job; keep libmbedtls-dev in pages.yml for when it runs on master. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
… pico/unique_id stub) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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
2 participants
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.
Host-side coverage of the whole codebase
The firmware is cross-compiled for the RP2040 and can't run natively, but almost
all of its logic is hardware-independent. This PR builds every first-party
.cas a native host binary and produces an lcov/genhtml coverage report thatspans the whole codebase: the logic modules at their real percentage, and the
hardware-I/O wrappers at an honest 0%. The report is generated by compiling the
whole first-party set instrumented (a
.gcnobaseline at 0%) and merging in therun data from the real harnesses.
Real tests (ASan + UBSan, and Valgrind)
Three small, table-driven harnesses exercise the non-trivial logic against the
real implementations (no mock reimplementations):
shared/totp.cagainst the systemlibmbedtls(real HMAC-SHA1),checked with the RFC 6238 test vectors. 100%.
storage.c+backup.con top of the reallittlefs, run against a RAM-backed XIP flash window that the harness maps in
with
mmap(MAP_FIXED)over the flash address the firmware uses. ~92%.serial/commands.cdriven throughcommands_dispatch(), asserting routing, admin gating, argc validation andunknown-command handling. 100%.
Every harness runs clean under both ASan/UBSan (
make -C test asan) andValgrind (
make -C test valgrind).The key find: a real buffer overflow in
storage.cASan caught a global-buffer-overflow in
storage.c: littlefs's.lookahead_sizewas set to 64, but the lookahead buffer is only 8 bytes — a56-byte out-of-bounds write on every mount and every format, on real
hardware too (it just happened to clobber adjacent globals silently there).
Fixed by tying the size to the buffer:
.lookahead_size = sizeof(lfs_lookahead_buf).The storage harness is the regression test.
Footprint
The firmware change is a single line (the storage fix). Everything else is test
scaffolding: mostly declaration-only host stubs for the pico-sdk / cyw43 / lwip /
mbedtls headers, plus the three harnesses and the
test/Makefile. The CItestjob installs
libmbedtls-dev(needed for the real TOTP test).Note on CI
mastercarries the CI workflows, so the checks run and appear on this PR.