# Mitigate SMI firmware code review findings Workpackage

Status: waiting on srv-rpi00.local coordinator readiness and returned controller-test evidence; firmware implementation complete
Type: workpackage
Updated: 2026-08-02
Next Action: this workspace publishes the exact SMI firmware bundle, diagnostic interface, negative/recovery test contract, and authorization references; the srv-rpi00.local qualification app coordinates every remaining Controller B case after its readiness and exact-case gates pass
Campaign Reason: Historical record from campaign 010-qualify-firmware-negative-and-recovery-behavior; retained independently after campaign closure and requires no separate owner-queue execution.
Campaign: standalone
Canonical Truth: E:/dev/high_point/7520553_SMI_FW/docs/firmware-update-validation-and-recovery.md
Project Map: work/maps/map-smi-firmware-stabilization-and-qualification.md

## 2026-08-02 Controller-Execution Ownership Gate

The remaining qualification runs on Controllers A/B using the exact SMI
firmware, but it is not coordinated from this repository. This firmware
workspace owns source, managed builds, artifact hashes, host/static/model
checks, firmware diagnostics, expected outcomes, safety requirements, immutable
bundle publication, and review of returned evidence.

`jong@srv-rpi00.local:~/dev/sd_qualification` owns exact-bundle import,
live-controller orchestration, programming/verify/reset when authorized,
negative-image and injected-failure execution, recovery, calibration
preservation, generated evidence, terminal disposition, and reviewer-approved
result manifests. It is **NOT READY for controller execution** until its
standalone foundation, live-controller adapter, authorized read-only A/B
identity, stop/recovery, and evidence-path gates pass.

The HPT Lab Power service remains a separate operation provider. The operator
owns safety authorization and irreducible physical observations. Unchecked
target criteria below are inputs to Pi-coordinated immutable campaign instances,
not instructions to operate either controller from this workspace.

## Current Context

Recovery-baseline update (2026-07-27): Controller B `1751219`, J-Link
`69660123`, now has hashed pre-change configuration/calibration and option
memory, an exact matching known-good program image, a freshly reacquired
serial-bound recovery path, and an intentional non-heating configuration that
survived reset. Normal temperature control was then restored and verified
across reset at generation `175` for full-function qualification. The live
cumulative candidate is source commit `2f4d8724...`,
Debug S-record `30C98F00...ADF649`. See
`work/evidence/controller-b-recovery-baseline-20260727.md`. The baseline
dependency is complete; this does not bypass any exact-case physical-safety,
stop, card, or evidence gate.

Final-candidate update (2026-07-25): the active implementation is commit
`391c7b4d053ee146b823c505987897d3f88c0022`, artifact freeze
`c0901312e0fe850d8a0b2452eb20d88c86ce07bd`, and Debug S-record SHA-256
`D386F03460E225B683D5146FFC7AD949CCFEF71A3EC8169C944EFA82B5992F9F`.
The synthetic malformed-image corpus, timeout/bounds source checks, and full
firmware regression suite pass. Both controllers were programmed/verified with
the final image and passed CID-linked full-card read-only scrubs plus the
integrated HPT/chart/logging gate. The immediate remaining gate is operator
visual, audio, and touchscreen feedback. Physical power-loss/recovery cases use
`work/runbooks/runbook-autonomous-hpt-sd-qualification.md` after final
candidate freeze and case-specific authorization; they are not implied by the
completed autonomous electronic baseline.

2026-07-20 update: the integrated mitigations are present in engineering
candidate 00.04.13. Automated SD, FileX, graph, export, evaluator, soak,
configuration, and model checks pass, as do clean Debug/Release builds and
versioned packaging. The image was programmed and verified on assemblies
`1751136` and `1751219`; both booted SD-ready and completed mirrored logging
with zero system/SD errors. Both serial-bound J-Link paths also passed
source-mapped breakpoint, backtrace, step, detach, and final-running-state
checks. This closes basic representative-target smoke, but it does not satisfy
the still-open malformed-image corpus, injected timeout/storage-failure, or
power-loss/recovery milestones.

The full review identified 10 findings: two critical, four high, and four medium.
The original evidence and line references remain historical review input in
`E:/dev/high_point/AI_Discovery/7520553_SMI_FW/code-review.md`; current update
behavior is owned by the repository documentation named above.

The firmware controls heating equipment and can replace its own internal-flash
image from USB. Mitigations must preserve the known-good Renesas Synergy 2.6.1,
ThreadX, GCC 10.3.1, GuiX, FileX, USBX, custom QSPI/SDRAM, debugger, and J-Flash
environment. There is no existing host test suite. The current branch also has
uncommitted e2 setup, warning, packaging, and Tool Shed changes that must not be
lost or mixed into remediation commits accidentally.

Finding map:

| Finding | Severity | Surface | Primary files |
| --- | --- | --- | --- |
| CR-01 | Critical | Update authenticity | `src/Thread_Usb_entry.c`, signing/build tooling |
| CR-02 | Critical | Update partition bounds | `src/Thread_Usb_entry.c`, update header/constants |
| CR-03 | High | Flash timeout hang | `src/Thread_Usb_entry.c` |
| CR-04 | High | NVRAM callback hang | `src/Thread_SystemMonitor_entry.c`, generated/config RTOS objects |
| CR-05 | High | SD locking race | `src/Thread_SdCard_entry.c`, USB/storage callers |
| CR-06 | High | Unreserved SDRAM buffers | linker script, `src/Thread_SdCard_entry.c` |
| CR-07 | Medium | Short update reads | `src/Thread_Usb_entry.c` |
| CR-08 | Medium | Export filename overflow | `src/HMI_MainEventHandler.c`, `src/AppVariables.h` |
| CR-09 | Medium | Keyboard termination | `src/HMI_MainEventHandler.c/.h` |
| CR-10 | Medium | Persisted model OOB | `src/AppVeeprom.c`, `src/ShelConfig.h`, HMI users |

## Recommendation

Deliver the mitigations as independently testable safety slices, ordered so
unrecoverable hangs and memory corruption are removed before the update format is
changed. Establish tests before refactoring. Keep generated and vendored code
unchanged unless e2 configuration must generate a new ThreadX object.

Do not describe CRC as authentication, add ad-hoc spinlock flags, or reserve
SDRAM only by comments/hard-coded addresses.
Keep each finding traceable even when the owner-approved final qualification
uses one integrated firmware image.

## Current State

Completed:

- Repository reconnaissance and full code review.
- Clean managed and isolated-headless Debug build baselines.
- Stable/versioned artifact packaging and J-Flash paths.
- Exact source locations, impacts, and initial recommendations for CR-01..CR-10.
- CR-03 now returns immediately when flash P/E mode entry times out.
- CR-03 write and erase entry waits and FRDY exit waits use the correct bounded
  counters; erase cannot underflow a zero block count.
- CR-04 now drains stale completion tokens, uses a bounded ThreadX semaphore
  wait, distinguishes abort/timeout, and resets the SPI peripheral for recovery.
- CR-08/09 now reserve the filename terminator and terminate general keyboard copies.
- CR-10 now bounds every identified model-table consumer. Invalid persisted
  configuration remains diagnostic and is not silently repaired or persisted;
  safe display/filename fallback prevents out-of-bounds access.
- Milestone 1 software changes pass the isolated clean e2 build and packaging pipeline.
- CR-05 was re-audited: all internal SD access is owned by
  `Thread_SdCard_entry.c`. The same-thread `gIsSdCardBusy` wait was not a mutex,
  could not serialize another caller, and was removed. No cross-thread writer
  exists to justify adding an RTOS mutex.
- CR-06 export/chart buffers are linker-owned arrays. The 2026-07-12 Debug map
  places display SDRAM through `0x90232800`, export through `0x90FF2580`, and
  chart through `0x91A2A048`; all overlap/capacity assertions pass.
- CR-02/07 structural checks reject wrong format/product, nonzero SMI I/O
  payload, zero/short/oversized/misaligned components, wrong total file size,
  and short reads. Exact declared payload CRCs are checked before any QSPI
  erase/programming; QSPI driver failures and ready timeouts are propagated.
- Update-file ownership is closed on every successful-open exit, including
  validation failure and the final update transition.
- A 16-case synthetic update-validation corpus passes, including truncation,
  trailing data, payload bit flips, and empty-audio stale-CRC behavior.
- Fifteen focused code-review source checks and all existing graph, export, SD,
  HPT commissioning/export/scrub/CID/routing, circular-log, and exhaustive
  write-cut model gates pass.
- Clean Debug and Release rebuilds and versioned packaging pass for 00.05.00;
  exact hashes are recorded in the integrated software-gate evidence.
- Clean isolated Debug build and versioned packaging passed after Stages 3 and 4.
- Authentication policy is captured in a rejected ADR rather than silently
  selecting a key or breaking deployed update compatibility.
- CR-01 firmware authentication was explicitly declined by the project owner on
  2026-07-12. The deployed CRC-compatible format remains; authenticity risk is
  accepted and recorded in `work/adr/adr-smi-firmware-update-authentication.md`.

Incomplete:

- Pi-coordinated, separately scoped representative-target malformed-update,
  injected-timeout/storage-failure, regression, and recovery evidence.
- G-PWR-gated physical update power-loss and recovery evidence.
- Final service/update procedure and field rollout closeout after target review.

The owner reported PASS on 2026-07-25 for the scoped visual, audio, and
touchscreen review on both final-candidate controllers.

## Goal

The SMI firmware rejects malformed, truncated, oversized, corrupt, or
wrong-target updates before erasing storage; every hardware wait and lock is
bounded and recoverable; SDRAM ownership is linker-enforced; UI/persistent inputs
cannot exceed buffer/table bounds; and all behavior is demonstrated by automated
checks plus representative-target fault tests without regressions to control,
logging, UI, update recovery, debug, or programming workflows.

## Why It Matters

Current defects can enable arbitrary firmware replacement, corrupt QSPI/SDRAM,
hang alarm monitoring, race FileX access, or write outside a global buffer. These
failures affect control integrity, service recovery, data retention, and product
safety rather than code style alone.

## Major Outcomes

- Recoverable flash/NVRAM/storage failure handling.
- Strict update manifest, length, partition, file-size, and read validation.
- Explicitly documented acceptance of CRC-only update authenticity risk.
- RTOS-native storage/NVRAM synchronization.
- Linker-owned SDRAM layout with build-time overlap failure.
- Central bounded-string handling and bounded persisted-model consumption.
- Automated negative tests and hardware fault/recovery evidence.

## Delivery Stages

Use stages when sequencing, parity gates, or handoff cost matter.

| Stage | Outcome | Entry Evidence | Exit Evidence |
| --- | --- | --- | --- |
| 0 | Reproducible baseline and safety harness | Current clean build/artifacts; review findings | Baseline image archived; host tests run; target smoke checklist passes |
| 1 | Hangs and bounded UI defects removed (CR-03/04/08/09/10) | Tests reproduce timeout/max-input/invalid-index cases | Faults return/report safely; boundary tests and clean build pass |
| 2 | FileX ownership resolved (CR-05) | Storage owner/caller inventory and race analysis | Single SD-thread ownership is demonstrated; false same-thread busy flag is removed; any future external caller must add an RTOS ownership API |
| 3 | SDRAM ownership enforced (CR-06) | Current map and record sizes captured | Linker sections/assertions pass; deliberate overlap test fails link |
| 4 | Structurally safe update parser (CR-02/07) | QSPI map and update file layout approved | Bounds/file-size/exact-read negative matrix passes before any erase |
| 5 | Authentication decision (CR-01) | Owner reviews compatibility and security impact | Declined mitigation and accepted CRC-only risk recorded in ADR |
| 6 | Pi-coordinated integrated hardware qualification and rollout | Stages 1-5 reviewed independently; exact bundle imported after coordinator readiness | Power-loss/fault/update/control/logging/UI regression suite passes; returned manifest is accepted for release |

## Related Artifacts

- Review evidence: `E:/dev/high_point/AI_Discovery/7520553_SMI_FW/code-review.md`
- Architecture context: `E:/dev/high_point/AI_Discovery/7520553_SMI_FW/architecture.md`
- Build context: `E:/dev/high_point/AI_Discovery/7520553_SMI_FW/build-and-release.md`
- Authentication decision: `work/adr/adr-smi-firmware-update-authentication.md`.
- Controller-test contract imported by the Pi qualifier:
  `work/checklists/checklist-smi-firmware-mitigation-hardware-qualification.md`.
- Future runbook: release packaging and recovery.
- Future checklist: target qualification and power-interruption matrix.

## Rough Sequence

1. Freeze and archive a known-good baseline ELF/SREC/MAP plus configuration and
   representative SD/USB media; record expected control/log/UI behavior.
2. Add host-testable pure helpers for checked arithmetic, update-manifest
   validation, bounded string handling, and persistent semantic validation.
3. Fix CR-03 first: every flash register wait returns a distinct timeout and
   restores/records state; test register-never-ready behavior.
4. Fix CR-04 with ThreadX semaphore/event completion and timeout recovery. Keep
   alarm processing alive after NVRAM failure.
5. Fix CR-08/09 with capacity-aware helpers. For CR-10, range-check at every
   model-table consumption point, use a safe display index, retain a diagnostic,
   and exercise zero, maximum, oversized, and invalid persisted values. Do not
   add a competing broad semantic validator here.
6. Inventory every SD/FileX caller, prove the current single-thread ownership,
   remove the false same-thread `gIsSdCardBusy` gate, and require an RTOS
   ownership API before any future external caller is added.
7. Move export/chart storage into explicit linker sections and add overlap/size
   assertions using the real record counts and 32-MiB SDRAM boundary.
8. Define QSPI partitions and checked update arithmetic. Validate header size,
   update format version, nonzero/max/aligned component lengths, non-overlap,
   exact total file size, exact `fx_file_read` byte counts, and CRC before erase.
9. Record the owner's decision to retain CRC-only update compatibility and do
   not implement the rejected signature-format proposal.
10. Through the ready Pi coordinator, run integrated qualification: normal update, corrupt/truncated/oversized/
    wrong-machine/wrong-key/old-version images, QSPI/SD/NVRAM timeouts, USB removal,
    repeated reset, and power interruption at each destructive update phase.
11. Promote settled format, key, memory-map, recovery, and operator procedures to
    repository docs; complete this workpackage only after evidence is linked.
2. ...
3. ...

## Milestones

### Milestone 1: Immediate containment

Completion criteria:

- [x] CR-03 timeout cannot spin or underflow.
- [x] CR-04 cannot block System Monitor indefinitely in software control flow.
- [x] CR-08/09 maximum inputs remain terminated and within bounds in code.
- [x] CR-10 invalid persisted model cannot index a model table out of bounds.
- [x] CR-10 handling is reduced to safe consumption/diagnostics and does not
  persistently rewrite configuration or gate heating solely from model identity.
- [x] Clean isolated Debug build and artifact packaging pass.
- [ ] Representative-target smoke and injected timeout tests pass.

### Milestone 2: Deterministic memory and storage ownership

- [x] All internal SD/FileX access uses the demonstrated single SD-thread owner;
  no cross-thread mutex is currently required.
- [x] The false same-thread busy-wait flag is removed; future external access is
  prohibited until an RTOS ownership API and timeout diagnostic are added.
- [x] SDRAM buffers are linker symbols/sections, not literal pointers.
- [x] Map-file validation proves separation and total SDRAM bounds.

### Milestone 3: Safe update structure

- [x] Update manifest validation has no unchecked addition/multiplication.
- [x] Component partitions and maximums are documented constants.
- [x] Exact file size and read byte counts are enforced before destructive work.
- [x] Negative update corpus passes automatically.

### Milestone 4: Update release and accepted authenticity risk

- [x] Authentication/key/legacy/rollback decision is recorded; authentication was declined.
- [x] Existing unsigned/CRC update compatibility is intentionally preserved.
- [ ] Corrupt, truncated, oversized, and wrong-product images fail safely on target.
- [ ] Power-loss and recovery matrix passes on representative hardware.
- [x] Repository update validation/recovery documentation is current for the
  implemented software behavior; field rollout sign-off remains a closeout gate.

## Open Questions

- Authentication and update-format changes are out of scope by explicit owner decision.
- What is the required behavior after NVRAM/SD failure: degraded operation,
  latched alarm, retry cadence, or service lockout?
- Which hardware revisions and product models form the qualification matrix?
- Is a bootloader redesign required for power-fail-safe internal flash recovery?

## CR-10 Scope Boundary

CR-10 owns the narrow memory-safety requirement: every persisted model value is
range-checked before table access, UI uses a safe fallback index, and invalid
input remains diagnosable. Model 0 / Not Configured is a supported factory and
commissioning state, so identity alone must not disable heating or trigger a
broad persistent repair.

Current-SMI schema fingerprinting, automatic clean hard reset for nonmatching
records, cross-field semantics, requested-versus-effective heater permission,
recovery boot, reset policy, and transactional dual-block persistence are owned by
`work/wp/completed/wp-semantic-configuration-validation-and-recovery.md`. CR-10
qualification should consume that contract and must not implement a second
validator.

## Completion Standard

This workpackage is complete when CR-01 has a recorded risk decision, CR-02
through CR-10 have reviewed fixes or evidence-backed reclassification,
automated boundary/fault tests, clean build evidence, an accepted immutable Pi
result manifest for representative hardware, no generated/vendor churn,
documented update/memory/key contracts, and
a field-ready recovery/rollout procedure. Closing a compiler warning or reaching
a happy-path build is not sufficient.

## Closeout Routing

- Current truth promoted to: repository README/docs for update format, memory map,
  synchronization ownership, accepted authenticity risk, recovery, and qualification.
- Historical context remains in: this completed workpackage and the discovery
  code review.
- Runtime/status evidence: build artifacts, map checks, automated test logs,
  malformed-image corpus results, and hardware fault/power-cycle records.
- Cleanup deferred to: unrelated compiler-warning and HMI decomposition work;
  track separately unless required by a mitigation.
