review: iteration-2 fixes (fragment assertion, evidence paths, hardening)

- build-kernel.sh fragment assertion: survives a missing trailing newline
  (the read footgun, reproduced) and treats an absent symbol on a disable
  line as FATAL, symmetric with the enable arm.
- fetch-kernel-tarball.sh checks the pin before downloading; both fetchers
  add --retry-connrefused.
- mkimage rejects '.'/'..' state keys.
- ui-shot: VM liveness checked before every QMP call, console.log preserved
  as evidence on every failure path, repaint deadline widened to 90s with
  the contended-runner rationale documented.
- rs485-bridge: overflow discards back off one gap and rate-limit their log
  line, mirroring the accept-loop fix; clippy nit fixed.
- .gitignore ignores *.elf/*.map so the untracked artifacts cannot silently
  return; CI shellcheck glob now covers build/ and the rootfs boot scripts
  (directives added for the deliberate in-guest source paths).
- Docs: NPU parity row matches its sibling verification docs; line-pinned
  audit cross-references unpinned; CROSS_COMPILE documented in the build
  header; payload README lists warden-ui; ci-cd tense settled.

Verified: guards negative-tested (bad state keys, no-newline fragment);
boot smoke, portal scenario, ui-shot all PASS; 53 tests green; shellcheck
clean across the widened glob; clippy zero.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018HUayid7W5w7jBdb9Rrj1K
This commit is contained in:
BFE Engineering
2026-08-30 08:34:47 -06:00
co-authored by Claude Fable 5
parent 2756de0b46
commit 973a414f07
17 changed files with 72 additions and 26 deletions
+3 -1
View File
@@ -108,7 +108,9 @@ jobs:
- name: shellcheck qemu scripts - name: shellcheck qemu scripts
run: | run: |
sudo apt-get update -qq && sudo apt-get install -y -qq shellcheck sudo apt-get update -qq && sudo apt-get install -y -qq shellcheck
shellcheck -x qemu/*.sh qemu/tests/*.sh shellcheck -x qemu/*.sh qemu/tests/*.sh build/*.sh \
qemu/rootfs/etc/warden-lib.sh qemu/rootfs/etc/rc \
qemu/rootfs/sbin/init qemu/rootfs/init
- name: cache pinned busybox - name: cache pinned busybox
uses: actions/cache@v4 uses: actions/cache@v4
with: with:
+2
View File
@@ -6,6 +6,8 @@ target/
# build artifacts (firmware/kernel objects are rebuilt from source) # build artifacts (firmware/kernel objects are rebuilt from source)
*.o *.o
*.a *.a
*.elf
*.map
# scratch / editor # scratch / editor
*.swp *.swp
+8 -4
View File
@@ -10,6 +10,8 @@
# Env: # Env:
# KERNEL_TARBALL path to a local linux-6.18.46.tar.xz (skips the download) # KERNEL_TARBALL path to a local linux-6.18.46.tar.xz (skips the download)
# SDK_TC dir holding the arm-rockchip830 uclibc cross toolchain bin/ # SDK_TC dir holding the arm-rockchip830 uclibc cross toolchain bin/
# CROSS_COMPILE cross-compiler prefix (default arm-rockchip830-linux-uclibcgnueabihf-;
# CI overrides with the generic arm-linux-gnueabihf-)
# WORK build scratch dir (default: a mktemp under $TMPDIR) # WORK build scratch dir (default: a mktemp under $TMPDIR)
# JOBS parallel make jobs (default: nproc) # JOBS parallel make jobs (default: nproc)
# WARDEN_KCONFIG_FRAGMENT # WARDEN_KCONFIG_FRAGMENT
@@ -119,7 +121,7 @@ make -C "$SRC" ARCH=arm CROSS_COMPILE="$CROSS_COMPILE" olddefconfig >/dev/null
# request in the fragment survived into the final .config; fail loud if not. # request in the fragment survived into the final .config; fail loud if not.
if [ -n "${WARDEN_KCONFIG_FRAGMENT:-}" ]; then if [ -n "${WARDEN_KCONFIG_FRAGMENT:-}" ]; then
frag_fail=0 frag_fail=0
while IFS= read -r line; do while IFS= read -r line || [ -n "$line" ]; do
case "$line" in case "$line" in
CONFIG_*=*) CONFIG_*=*)
grep -qxF "$line" "$SRC/.config" || { grep -qxF "$line" "$SRC/.config" || {
@@ -127,9 +129,11 @@ if [ -n "${WARDEN_KCONFIG_FRAGMENT:-}" ]; then
frag_fail=1 frag_fail=1
} ;; } ;;
"# CONFIG_"*" is not set") "# CONFIG_"*" is not set")
opt="${line#\# }"; opt="${opt% is not set}" # Symmetric with the enable arm: the exact disable line must be
grep -qE "^$opt=" "$SRC/.config" && { # present. A symbol absent entirely means a typo'd/renamed option,
echo "FATAL: fragment disabled '$opt' but it is set in the final .config" >&2 # not a successful disable.
grep -qxF "$line" "$SRC/.config" || {
echo "FATAL: fragment line '$line' not reflected in the final .config" >&2
frag_fail=1 frag_fail=1
} ;; } ;;
esac esac
+6 -4
View File
@@ -14,14 +14,16 @@ URL="https://cdn.kernel.org/pub/linux/kernel/v6.x/linux-$KVER.tar.xz"
TB="${1:?usage: fetch-kernel-tarball.sh <destination-path>}" TB="${1:?usage: fetch-kernel-tarball.sh <destination-path>}"
if [ ! -f "$TB" ]; then # Pin first: a forgotten pin on a KVER bump should refuse BEFORE burning a
echo "== downloading $URL" # 140MB download it will then reject anyway.
curl --retry 3 --retry-delay 5 -fSL "$URL" -o "$TB"
fi
[ -f "$SHA_FILE" ] || { [ -f "$SHA_FILE" ] || {
echo "FATAL: no pinned sha256 for linux-$KVER (expected $SHA_FILE) — refusing an unverified tarball" >&2 echo "FATAL: no pinned sha256 for linux-$KVER (expected $SHA_FILE) — refusing an unverified tarball" >&2
exit 1 exit 1
} }
if [ ! -f "$TB" ]; then
echo "== downloading $URL"
curl --retry 3 --retry-delay 5 --retry-connrefused -fSL "$URL" -o "$TB"
fi
want="$(cat "$SHA_FILE")" want="$(cat "$SHA_FILE")"
got="$(sha256sum "$TB" | awk '{print $1}')" got="$(sha256sum "$TB" | awk '{print $1}')"
[ "$want" = "$got" ] || { echo "FATAL: tarball sha256 mismatch: want $want got $got" >&2; exit 1; } [ "$want" = "$got" ] || { echo "FATAL: tarball sha256 mismatch: want $want got $got" >&2; exit 1; }
+1 -1
View File
@@ -2,7 +2,7 @@
`.github/workflows/ci.yml` — every job runs on GitHub-hosted `ubuntu-latest`. `.github/workflows/ci.yml` — every job runs on GitHub-hosted `ubuntu-latest`.
No self-hosted runner is (or may be) reachable from this repo's workflows: No self-hosted runner is (or may be) reachable from this repo's workflows:
the repo is going public, and a fork PR that gets one approved run could the repo is public, and a fork PR that gets one approved run could
otherwise execute code on private infrastructure (ADR-0007). otherwise execute code on private infrastructure (ADR-0007).
## Jobs ## Jobs
+1 -1
View File
@@ -38,7 +38,7 @@ c8a3, not just compiled.
| GPIO_SYSFS / crypto / CFG80211 | — | mainline (config) | [x] =y (batch2) | | GPIO_SYSFS / crypto / CFG80211 | — | mainline (config) | [x] =y (batch2) |
| AIC8800 wifi (bsp/fdrv) | aic8800_* | **out-of-tree** | [x] M5 — wlan0 up, scanned the site AP at 43dBm (modules, `wifi/VERIFIED-on-c8a3.md`) | | AIC8800 wifi (bsp/fdrv) | aic8800_* | **out-of-tree** | [x] M5 — wlan0 up, scanned the site AP at 43dBm (modules, `wifi/VERIFIED-on-c8a3.md`) |
| AIC8800 BT (btlpm) | aic8800_btlpm | **out-of-tree** | [wip] module built (6.18 vermagic); HCI bring-up not yet exercised | | AIC8800 BT (btlpm) | aic8800_btlpm | **out-of-tree** | [wip] module built (6.18 vermagic); HCI bring-up not yet exercised |
| NPU (rknpu, ff660000) | rknpu, ff660000.npu | **out-of-tree** | [wip] M6 built, 0 errors/0 warnings, 99 `rknpu`-prefixed symbols in `System.map`, `&npu {status="okay"}` in the dtb — **not yet flashed/probed on hardware** (build-only session; see `npu/PORT-PROGRESS.md`) | | NPU (rknpu, ff660000) | rknpu, ff660000.npu | **out-of-tree** | [x] open GPL driver VERIFIED on hardware — `/dev/dri/card1`, `rknpu_version_test` PASS (power/clock/reset path exercised); open *compute* (regcmd) remains a from-scratch RE project (`npu/VERIFIED.md`, `npu/OPEN-NPU-PLAN.md`) |
| RGA 2D (rga2) | rga2 | ported (vendor char-dev) | [x] /dev/rga, hw 3.3.87975 | | RGA 2D (rga2) | rga2 | ported (vendor char-dev) | [x] /dev/rga, hw 3.3.87975 |
| I2S audio (i2s-tdm) | i2s | rv1126 fallback (=y) | [x] cpu DAI registers (part of the card below) | | I2S audio (i2s-tdm) | i2s | rv1126 fallback (=y) | [x] cpu DAI registers (part of the card below) |
| Audio codec (acodec) | rockchip,rv1106-codec | ported (rv1106_codec.c) | [x] card `rv1106-acodec`, pcmC0D0p/c (`audio/`); audible test @ bench | | Audio codec (acodec) | rockchip,rv1106-codec | ported (rv1106_codec.c) | [x] card `rv1106-acodec`, pcmC0D0p/c (`audio/`); audible test @ bench |
+2 -2
View File
@@ -25,7 +25,7 @@ watchdog exactly as-is (different threat model, different job).
- **Controller (Linux side): non-issue.** `drivers/mailbox/rockchip-mailbox.c` is - **Controller (Linux side): non-issue.** `drivers/mailbox/rockchip-mailbox.c` is
upstream in mainline 6.18 and **already binds on our exact kernel** via the upstream in mainline 6.18 and **already binds on our exact kernel** via the
generic `rockchip,rk3368-mailbox` **fallback compatible** with **zero patching** generic `rockchip,rk3368-mailbox` **fallback compatible** with **zero patching**
— recorded in `CAPABILITIES-AUDIT.md:30`, confirmed by source read. RV1106's DT — recorded in `CAPABILITIES-AUDIT.md`'s Remaining-blocks table (mailbox row), confirmed by source read. RV1106's DT
declares both instances with that fallback string. Gated today only by declares both instances with that fallback string. Gated today only by
`status="disabled"` + `CONFIG_ROCKCHIP_MBOX` being absent from the defconfig. `status="disabled"` + `CONFIG_ROCKCHIP_MBOX` being absent from the defconfig.
- **HPMCU firmware (MCU side): we already do the hard part.** WardenOS has a - **HPMCU firmware (MCU side): we already do the hard part.** WardenOS has a
@@ -241,7 +241,7 @@ adopt rpmsg/virtio unless the payload complexity genuinely demands it.
moot, since our `hpmcu.rs` is an independent hardware-validated reimplementation. moot, since our `hpmcu.rs` is an independent hardware-validated reimplementation.
--- ---
_Cross-refs: `../CAPABILITIES-AUDIT.md:30`, `../REMAINING-PORTS.md §7`, _Cross-refs: `../CAPABILITIES-AUDIT.md`, `../REMAINING-PORTS.md §7`,
`../../luckfox-pico-86-panel/riscv-mcu.md`, `../../luckfox-pico-86-panel/riscv-mcu.md`,
`.../raw/followup-riscv-mcu.md`, `.../raw/followup-riscv-mcu.md`,
`flare-edge/major-app-additions/docs/decisions/0002-hpmcu-watchdog.md`, `flare-edge/major-app-additions/docs/decisions/0002-hpmcu-watchdog.md`,
@@ -197,7 +197,8 @@ the whole problem.
`PROVENANCE.md`: the kernel driver is portable GPL; the closed piece is the `PROVENANCE.md`: the kernel driver is portable GPL; the closed piece is the
userspace RKNN runtime + regcmd format (a blob). **Per directive we do not ship userspace RKNN runtime + regcmd format (a blob). **Per directive we do not ship
that blob.** `CAPABILITIES-AUDIT.md:32` rates NPU "not worth shipping" until an that blob.** `CAPABILITIES-AUDIT.md`'s Remaining-blocks table rates the NPU
"open driver VERIFIED; compute deferred" until an
open encoder exists. open encoder exists.
### URLs ### URLs
@@ -299,5 +300,5 @@ ever begun; otherwise this is the documented reason open NPU compute is deferred
--- ---
_Cross-refs: `PORT-PLAN.md` (authoritative file-by-file kernel port), _Cross-refs: `PORT-PLAN.md` (authoritative file-by-file kernel port),
`../../docs/npu-graphics-feasibility.md`, `../CAPABILITIES-AUDIT.md:32`, `../../docs/npu-graphics-feasibility.md`, `../CAPABILITIES-AUDIT.md`,
`../PROVENANCE.md`, `../DRIVER-PARITY.md:41`, `../REMAINING-PORTS.md §6`._ `../PROVENANCE.md`, `../DRIVER-PARITY.md:41`, `../REMAINING-PORTS.md §6`._
+1 -1
View File
@@ -18,7 +18,7 @@ qemu_get_busybox() {
BB="${BUSYBOX:-$out/busybox-armv7l}" BB="${BUSYBOX:-$out/busybox-armv7l}"
if [ ! -f "$BB" ]; then if [ ! -f "$BB" ]; then
qemu_log "downloading $BB_URL" qemu_log "downloading $BB_URL"
curl --retry 3 --retry-delay 5 -fSL "$BB_URL" -o "$BB" curl --retry 3 --retry-delay 5 --retry-connrefused -fSL "$BB_URL" -o "$BB"
fi fi
[ -f "$sha_file" ] || { [ -f "$sha_file" ] || {
echo "FATAL: no pinned sha256 for busybox (expected $sha_file) — refusing to build from an unverified binary" >&2 echo "FATAL: no pinned sha256 for busybox (expected $sha_file) — refusing to build from an unverified binary" >&2
+3
View File
@@ -36,6 +36,9 @@ while [ $# -gt 0 ]; do
*) echo "FATAL: --state needs KEY=VALUE, got '$2'" >&2; exit 1 ;; *) echo "FATAL: --state needs KEY=VALUE, got '$2'" >&2; exit 1 ;;
esac esac
case "${2%%=*}" in case "${2%%=*}" in
.|..)
echo "FATAL: --state key cannot be '.' or '..'" >&2
exit 1 ;;
*[!A-Za-z0-9_.]*|'') *[!A-Za-z0-9_.]*|'')
echo "FATAL: --state key '${2%%=*}' must match [A-Za-z0-9_.]+ (it becomes a filename)" >&2 echo "FATAL: --state key '${2%%=*}' must match [A-Za-z0-9_.]+ (it becomes a filename)" >&2
exit 1 ;; exit 1 ;;
+4 -3
View File
@@ -19,6 +19,7 @@ Then:
cp <flare-edge>/target/armv7-unknown-linux-musleabihf/release/warden-flared qemu/payload/ cp <flare-edge>/target/armv7-unknown-linux-musleabihf/release/warden-flared qemu/payload/
``` ```
Stage-2 init starts `warden-flared` and `warden-modbus` automatically when Stage-2 init starts `warden-flared`, `warden-modbus`, and `warden-ui` (the
present (logs land in `/tmp/<name>.log` inside the guest). An empty payload is UI additionally needs `--display on|headless` + the virt.fragment kernel for
valid — the image boots to a busybox-only userspace. /dev/fb0) automatically when present (logs land in `/tmp/<name>.log` inside
the guest). An empty payload is valid — the image boots busybox-only.
+1
View File
@@ -10,6 +10,7 @@
# exits and the kernel panics; the applet-existence check below catches the # exits and the kernel panics; the applet-existence check below catches the
# only preventable variant of that. # only preventable variant of that.
# shellcheck source=qemu/rootfs/etc/warden-lib.sh disable=SC1091
. /etc/warden-lib.sh . /etc/warden-lib.sh
warden_populate_by_name warden_populate_by_name
+1
View File
@@ -29,6 +29,7 @@ warden_populate_by_name() {
# VALIDATE it — echoes "_a" or "_b", falling back to _a with a warning. # VALIDATE it — echoes "_a" or "_b", falling back to _a with a warning.
warden_slot() { warden_slot() {
slot="_a" slot="_a"
# shellcheck disable=SC2013 # cmdline TOKENS are the unit here, not lines
for tok in $(cat /proc/cmdline); do for tok in $(cat /proc/cmdline); do
case "$tok" in case "$tok" in
warden.slot=*) slot="${tok#warden.slot=}" ;; warden.slot=*) slot="${tok#warden.slot=}" ;;
+1
View File
@@ -21,6 +21,7 @@ echo "WARDEN-QEMU-BOOT-OK"
# slot select, switch_root). It only returns on failure — then fall through to # slot select, switch_root). It only returns on failure — then fall through to
# the diskless shell/poweroff behavior below. # the diskless shell/poweroff behavior below.
if [ -b /dev/vda ]; then if [ -b /dev/vda ]; then
# shellcheck source=qemu/rootfs/etc/rc disable=SC1091
. /etc/rc . /etc/rc
fi fi
+1
View File
@@ -13,6 +13,7 @@ mount -t proc proc /proc
mount -t sysfs sysfs /sys mount -t sysfs sysfs /sys
mount -t tmpfs tmpfs /tmp mount -t tmpfs tmpfs /tmp
# shellcheck source=qemu/rootfs/etc/warden-lib.sh disable=SC1091
. /etc/warden-lib.sh . /etc/warden-lib.sh
# Fresh devtmpfs — repopulate the by-name contract; same VALIDATED slot rule # Fresh devtmpfs — repopulate the by-name contract; same VALIDATED slot rule
+14 -5
View File
@@ -61,6 +61,7 @@ pub fn pump_serial(
stream.set_read_timeout(Some(gap))?; stream.set_read_timeout(Some(gap))?;
let mut buf: Vec<u8> = Vec::new(); let mut buf: Vec<u8> = Vec::new();
let mut chunk = [0u8; 256]; let mut chunk = [0u8; 256];
let mut discards: u64 = 0;
loop { loop {
match (&*stream).read(&mut chunk) { match (&*stream).read(&mut chunk) {
Ok(0) => { Ok(0) => {
@@ -72,12 +73,20 @@ pub fn pump_serial(
Ok(n) => { Ok(n) => {
buf.extend_from_slice(&chunk[..n]); buf.extend_from_slice(&chunk[..n]);
if buf.len() > MAX_PENDING { if buf.len() > MAX_PENDING {
eprintln!( // Rate-limit the log and back off for one gap so a master
"rs485: {} bytes buffered with no inter-frame gap — discarding \ // streaming continuously cannot peg a core and flood
(misbehaving master streaming continuously?)", // stderr — mirroring the accept-loop backoff.
buf.len() discards += 1;
); if discards == 1 || discards.is_multiple_of(256) {
eprintln!(
"rs485: {} bytes buffered with no inter-frame gap — \
discarding (misbehaving master? {} discards so far)",
buf.len(),
discards
);
}
buf.clear(); buf.clear();
std::thread::sleep(gap);
} }
} }
Err(e) Err(e)
+20 -2
View File
@@ -94,16 +94,29 @@ sys.exit(0 if distinct > 16 else 1)
EOF EOF
} }
# The VM can die mid-poll (OOM, crash): check liveness before every QMP
# call so the failure is OUR message + console evidence, not a python
# traceback — and preserve the console log before the trap removes $WORK.
vm_alive_or_die() {
kill -0 "$QEMU_PID" 2>/dev/null && return 0
echo "FATAL: VM exited during the screendump poll" >&2
tail -25 "$WORK/console.log" >&2
mkdir -p "$OUTDIR"; cp "$WORK/console.log" "$OUTDIR/ui-shot-console.log" || true
exit 1
}
# Poll for the first rendered frame (bounded, no guessed sleep). # Poll for the first rendered frame (bounded, no guessed sleep).
rendered=0 rendered=0
deadline=$((SECONDS + 90)) deadline=$((SECONDS + 90))
while [ $SECONDS -lt $deadline ]; do while [ $SECONDS -lt $deadline ]; do
vm_alive_or_die
qmp screendump "$WORK/shot1.ppm" qmp screendump "$WORK/shot1.ppm"
if frame_rendered "$WORK/shot1.ppm"; then rendered=1; break; fi if frame_rendered "$WORK/shot1.ppm"; then rendered=1; break; fi
sleep 3 sleep 3
done done
[ "$rendered" = 1 ] || { [ "$rendered" = 1 ] || {
echo "FATAL: UI never rendered a non-blank frame within 90s" >&2 echo "FATAL: UI never rendered a non-blank frame within 90s" >&2
mkdir -p "$OUTDIR"; cp "$WORK/console.log" "$OUTDIR/ui-shot-console.log" || true
exit 1 exit 1
} }
@@ -112,9 +125,13 @@ done
# repaint rather than guessing a delay. # repaint rather than guessing a delay.
qmp tap 16975 1820 qmp tap 16975 1820
changed=0 changed=0
deadline=$((SECONDS + 30)) # 90s, matching the first-frame budget: TCG repaints are CPU-bound and a
# contended CI runner can be arbitrarily slower than this dev box (same
# margin reasoning as the rs485 test-gap widening).
deadline=$((SECONDS + 90))
while [ $SECONDS -lt $deadline ]; do while [ $SECONDS -lt $deadline ]; do
sleep 2 sleep 2
vm_alive_or_die
qmp screendump "$WORK/shot2.ppm" qmp screendump "$WORK/shot2.ppm"
if ! cmp -s "$WORK/shot1.ppm" "$WORK/shot2.ppm"; then changed=1; break; fi if ! cmp -s "$WORK/shot1.ppm" "$WORK/shot2.ppm"; then changed=1; break; fi
done done
@@ -124,7 +141,8 @@ cp "$WORK/shot1.ppm" "$OUTDIR/ui-shot1.ppm"
cp "$WORK/shot2.ppm" "$OUTDIR/ui-shot2.ppm" 2>/dev/null || true cp "$WORK/shot2.ppm" "$OUTDIR/ui-shot2.ppm" 2>/dev/null || true
[ "$changed" = 1 ] || { [ "$changed" = 1 ] || {
echo "FATAL: tapping the Metrics tab did not change the frame within 30s — touch is not reaching the UI" >&2 echo "FATAL: tapping the Metrics tab did not change the frame within 90s — touch is not reaching the UI" >&2
cp "$WORK/console.log" "$OUTDIR/ui-shot-console.log" || true
exit 1 exit 1
} }
echo "tap on the Metrics tab repainted the frame (touch reached the UI)" echo "tap on the Metrics tab repainted the frame (touch reached the UI)"