Skip to content

Commit e8ecb41

Browse files
committed
flash: route all writes through GuardedFlash safety transaction (ADR-021)
Single chokepoint for every flash write. begin() takes a mandatory pre-write snapshot via the IOC-free diag path and saves it as a recovery image; commit()/ commit_with() run invariant-driven pre/postflight checks with a severity-driven response. Catches both historical brick mechanisms: - BootCriticalUnchanged (Critical): no byte of 0x540000-0x5A0000 may change -- the unrecoverable boot-input zone (the 2026-05-20 BIOS-into-boot-input brick). - BanksConsistent (Critical): FIRMWARE == BACKUP version (the 2026-05-31 bank-mismatch brick). - TargetReadbackMatches (Error): target region reads back == intended bytes. Reroutes fw write, bios/nvdata write (previously had NO whole-flash check), recover (commit_with), and enforces a mandatory pre-erase snapshot. Deletes the two duplicated post_write_whole_flash_verify helpers. Card::write_region gains a doc safety contract. 13 new pure unit tests (incl. 05-20 brick simulation); 284 total pass; clippy clean.
1 parent 9a373f7 commit e8ecb41

8 files changed

Lines changed: 782 additions & 188 deletions

File tree

src/card/mod.rs

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -186,10 +186,14 @@ pub trait Card: Send {
186186
Err(CardError::NotImplemented("read_region_itype"))
187187
}
188188

189-
/// Write one flash-chip region (FW / BIOS / NVDATA) via FW_DOWNLOAD.
190-
/// DESTRUCTIVE. Shared primitive for `restore` and `fw/bios/nvdata write`.
191-
/// (Erase-if-personality-change and HCB-on-lock live in the higher-level
192-
/// `fw write` logic, not here — this is the raw region write.)
189+
/// Write one flash-chip region (FW / BIOS / NVDATA) via FW_DOWNLOAD. DESTRUCTIVE.
190+
///
191+
/// SAFETY CONTRACT (ADR-021): this is the raw, UNGUARDED region write. CLI verbs and any new
192+
/// callers MUST NOT call it directly — route through `firmware::guard::GuardedFlash`, which
193+
/// takes a mandatory pre-write snapshot and enforces the boot-critical-unchanged + bank-
194+
/// consistency postflight invariants. Two cards were bricked by writes that skipped those
195+
/// checks. The only legitimate direct callers are `GuardedFlash` itself (and a multi-region
196+
/// `restore` invoked *inside* `GuardedFlash::commit_with`).
193197
fn write_region(
194198
&mut self,
195199
_image_type: crate::mpi::messages::ImageType,

src/cli/erase.rs

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,18 @@ pub fn run(
3030

3131
let bdf = crate::card::resolve_bdf(bdf.as_deref())
3232
.map_err(|e| crate::Error::Other(format!("{}", e)))?;
33+
34+
// Mandatory pre-erase recovery snapshot (ADR-021). Erase intentionally clears flash, so the
35+
// boot-critical-unchanged invariant does not apply — but we MUST have a recovery image first.
36+
// This enforces what was previously only a text plea ("ensure a fresh backup exists"): no
37+
// snapshot (IOC-free diag read), no erase.
38+
let snap_dir = std::env::current_dir().unwrap_or_else(|_| std::path::PathBuf::from("."));
39+
let txn = crate::firmware::guard::GuardedFlash::begin(&bdf, &snap_dir)?;
40+
eprintln!(
41+
"erase: pre-erase recovery snapshot captured → {}",
42+
txn.snapshot_path().display()
43+
);
44+
3345
let mut card = crate::card::discover_one(&bdf)
3446
.map_err(|e| crate::Error::Other(format!("discover_one({}): {}", bdf, e)))?;
3547

src/cli/fw.rs

Lines changed: 22 additions & 91 deletions
Original file line numberDiff line numberDiff line change
@@ -9,10 +9,16 @@ use std::path::Path;
99

1010
use sha2::{Digest, Sha256};
1111

12+
use crate::firmware::guard::{GuardedFlash, WriteIntent};
1213
use crate::firmware::validate::{check_fit, validate_image};
13-
use crate::firmware::flash_layout::verify_flash_consistency;
1414
use crate::mpi::messages::ImageType;
1515

16+
/// Directory where guarded-flash writes drop the pre-write recovery snapshot. Current working
17+
/// directory — discoverable and deterministic (no timestamp in the path; one snapshot per card).
18+
fn snapshot_dir() -> std::path::PathBuf {
19+
std::env::current_dir().unwrap_or_else(|_| std::path::PathBuf::from("."))
20+
}
21+
1622
fn sha_hex(b: &[u8]) -> String {
1723
let mut h = Sha256::new();
1824
h.update(b);
@@ -32,68 +38,6 @@ fn fw_banner(data: &[u8]) -> Option<String> {
3238
std::str::from_utf8(&data[pos..end]).ok().map(String::from)
3339
}
3440

35-
/// Perform whole-flash consistency check via diag back-door. Reads full 8MiB from 0xFC000000,
36-
/// calls verify_flash_consistency, and returns Err if FIRMWARE and BACKUP regions disagree.
37-
/// If the diag read itself fails, logs a warning but Ok(()) — don't false-fail the write.
38-
fn post_write_whole_flash_verify(bdf: &str) -> Result<(), crate::Error> {
39-
use crate::sbr::transport::Bar1MmapSbrTransport;
40-
41-
const FLASH_WINDOW_ADDR: u32 = 0xFC000000;
42-
const FLASH_WINDOW_SIZE: usize = 8 * 1024 * 1024;
43-
44-
let mut transport = match Bar1MmapSbrTransport::open(bdf) {
45-
Ok(t) => t,
46-
Err(e) => {
47-
eprintln!(
48-
"WARN: post-write whole-flash verify: could not open diag back-door ({}), \
49-
skipping consistency check — verify backup region manually",
50-
e
51-
);
52-
return Ok(());
53-
}
54-
};
55-
56-
let chip_mem = match transport.read_chip_mem(FLASH_WINDOW_ADDR, FLASH_WINDOW_SIZE) {
57-
Ok(mem) => mem,
58-
Err(e) => {
59-
eprintln!(
60-
"WARN: post-write whole-flash verify: diag read failed ({}), \
61-
skipping consistency check — verify backup region manually",
62-
e
63-
);
64-
return Ok(());
65-
}
66-
};
67-
68-
match verify_flash_consistency(&chip_mem) {
69-
Ok(consistency) => {
70-
if !consistency.consistent {
71-
return Err(crate::Error::Other(format!(
72-
"POST-WRITE VERIFICATION FAILED: FIRMWARE region version '{}' \
73-
does not match BACKUP region version '{}'. The card may be bricked. \
74-
Run 'lsi-flash backup' immediately to capture current state, then restore \
75-
from a known-good backup.",
76-
consistency.firmware_version, consistency.backup_version
77-
)));
78-
}
79-
eprintln!(
80-
"fw write: whole-flash verify OK (FIRMWARE={}, BACKUP={})",
81-
consistency.firmware_version, consistency.backup_version
82-
);
83-
Ok(())
84-
}
85-
Err(e) => {
86-
// If parsing the layout fails, we can't verify — warn but don't fail the write.
87-
eprintln!(
88-
"WARN: post-write whole-flash verify: could not parse flash layout ({}), \
89-
skipping consistency check",
90-
e
91-
);
92-
Ok(())
93-
}
94-
}
95-
}
96-
9741
/// `fw read` — read the firmware region. By default reads the **flash** copy
9842
/// (FW_UPLOAD ITYPE 0x01 = FW_FLASH, the persisted image); `--running` reads
9943
/// the **running** image (ITYPE 0x00 = FW_CURRENT, what the chip booted). The
@@ -206,32 +150,19 @@ pub fn run_write(bdf: String, from_file: &Path, yes: bool) -> Result<(), crate::
206150
}
207151
}
208152

209-
// Write (FW_DOWNLOAD).
210-
card.write_region(ImageType::Fw, &image)
211-
.map_err(|e| crate::Error::Other(format!("fw write: {}", e)))?;
212-
213-
// Check 5 — read-back verify (ADR-015 Rule 5).
214-
match card.read_region(ImageType::Fw) {
215-
Ok(rb) => {
216-
let want = sha_hex(&image);
217-
let got = sha_hex(&rb[..rb.len().min(image.len())]);
218-
if want == got {
219-
eprintln!("fw write: OK ({} bytes), read-back verified ✓", image.len());
220-
} else {
221-
return Err(crate::Error::Other(format!(
222-
"fw write: read-back MISMATCH (wrote {} got {}) — investigate before trusting",
223-
want, got
224-
)));
225-
}
226-
}
227-
Err(e) => eprintln!(
228-
"fw write: OK ({} bytes), but read-back verify failed: {} (verify manually)",
229-
image.len(),
230-
e
231-
),
232-
}
233-
234-
// ADR-020 whole-flash consistency gate: after FW_DOWNLOAD, read full flash via diag back-door
235-
// and require active == backup. Mismatch → fail loudly with both versions named.
236-
post_write_whole_flash_verify(&bdf)
153+
// The guarded transaction (ADR-021) is the ONLY path bytes reach flash: it takes a mandatory
154+
// pre-write snapshot (recovery image), writes via FW_DOWNLOAD, then enforces the postflight
155+
// invariants (boot-critical zone unchanged, banks consistent, read-back matches intent) with
156+
// a severity-driven response. Supersedes the old hand-rolled read-back + whole-flash verify.
157+
let txn = GuardedFlash::begin(&bdf, &snapshot_dir())?;
158+
txn.commit(
159+
&mut *card,
160+
WriteIntent {
161+
region: ImageType::Fw,
162+
bytes: image.clone(),
163+
label: "firmware".into(),
164+
},
165+
)?;
166+
eprintln!("fw write: OK ({} bytes) — guarded transaction verified ✓", image.len());
167+
Ok(())
237168
}

src/cli/recover.rs

Lines changed: 19 additions & 65 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@ use std::path::PathBuf;
77
use thiserror::Error;
88

99
use crate::cli::backup::BackupManifest;
10-
use crate::firmware::flash_layout::verify_flash_consistency;
1110
use crate::mpi::messages::MpiError;
1211

1312
#[derive(Debug, Error)]
@@ -159,13 +158,23 @@ pub fn run(
159158
return Ok(());
160159
}
161160

162-
// 6. Real path: dispatch through Card trait (same as backup)
161+
// 6. Real path: dispatch through Card trait, wrapped in the guarded transaction (ADR-021).
162+
// `restore` writes multiple regions, so we use commit_with: it snapshots first (recovery
163+
// image), runs the multi-region write, then enforces boot-critical-unchanged + bank
164+
// consistency against the snapshot. A restore that disturbs the boot-input zone is caught.
163165
let mut card = crate::card::discover_one(&bdf)
164166
.map_err(|e| crate::Error::Other(format!("recover: discover_one({}): {}", bdf, e)))?;
165167

166-
let report = card
167-
.restore(std::path::Path::new(&backup_dir))
168-
.map_err(|e| crate::Error::Other(format!("recover: card.restore: {}", e)))?;
168+
let txn = crate::firmware::guard::GuardedFlash::begin(&bdf, &snapshot_dir())?;
169+
let mut report_slot = None;
170+
txn.commit_with(|| {
171+
let report = card
172+
.restore(std::path::Path::new(&backup_dir))
173+
.map_err(|e| crate::Error::Other(format!("recover: card.restore: {}", e)))?;
174+
report_slot = Some(report);
175+
Ok(())
176+
})?;
177+
let report = report_slot.expect("restore returned Ok ⇒ report captured");
169178

170179
// 7. Output (honest about which regions were actually written)
171180
if json {
@@ -186,69 +195,14 @@ pub fn run(
186195
}
187196
}
188197

189-
// ADR-020 whole-flash consistency gate after restore (same as fw write).
190-
post_write_whole_flash_verify(&bdf)?;
191-
198+
// Whole-flash safety (boot-critical unchanged + banks consistent) was enforced inside the
199+
// guarded transaction above (commit_with), against the mandatory pre-write snapshot.
192200
Ok(())
193201
}
194202

195-
/// Post-write whole-flash verify helper (mirrors fw.rs implementation).
196-
fn post_write_whole_flash_verify(bdf: &str) -> Result<(), crate::Error> {
197-
use crate::sbr::transport::Bar1MmapSbrTransport;
198-
199-
const FLASH_WINDOW_ADDR: u32 = 0xFC000000;
200-
const FLASH_WINDOW_SIZE: usize = 8 * 1024 * 1024;
201-
202-
let mut transport = match Bar1MmapSbrTransport::open(bdf) {
203-
Ok(t) => t,
204-
Err(e) => {
205-
eprintln!(
206-
"WARN: post-write whole-flash verify: could not open diag back-door ({}), \
207-
skipping consistency check — verify backup region manually",
208-
e
209-
);
210-
return Ok(());
211-
}
212-
};
213-
214-
let chip_mem = match transport.read_chip_mem(FLASH_WINDOW_ADDR, FLASH_WINDOW_SIZE) {
215-
Ok(mem) => mem,
216-
Err(e) => {
217-
eprintln!(
218-
"WARN: post-write whole-flash verify: diag read failed ({}), \
219-
skipping consistency check — verify backup region manually",
220-
e
221-
);
222-
return Ok(());
223-
}
224-
};
225-
226-
match verify_flash_consistency(&chip_mem) {
227-
Ok(consistency) => {
228-
if !consistency.consistent {
229-
return Err(crate::Error::Other(format!(
230-
"POST-RESTORE VERIFICATION FAILED: FIRMWARE region version '{}' \
231-
does not match BACKUP region version '{}'. The card may be bricked. \
232-
Run 'lsi-flash backup' immediately to capture current state, then restore \
233-
from a known-good backup.",
234-
consistency.firmware_version, consistency.backup_version
235-
)));
236-
}
237-
eprintln!(
238-
"restore: whole-flash verify OK (FIRMWARE={}, BACKUP={})",
239-
consistency.firmware_version, consistency.backup_version
240-
);
241-
Ok(())
242-
}
243-
Err(e) => {
244-
eprintln!(
245-
"WARN: post-write whole-flash verify: could not parse flash layout ({}), \
246-
skipping consistency check",
247-
e
248-
);
249-
Ok(())
250-
}
251-
}
203+
/// Directory where guarded-flash writes drop the pre-write recovery snapshot (current dir).
204+
fn snapshot_dir() -> std::path::PathBuf {
205+
std::env::current_dir().unwrap_or_else(|_| std::path::PathBuf::from("."))
252206
}
253207

254208
fn sha256_hex(data: &[u8]) -> String {

src/cli/region.rs

Lines changed: 25 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ use std::path::PathBuf;
1010
use clap::{Args, Subcommand};
1111
use sha2::{Digest, Sha256};
1212

13+
use crate::firmware::guard::{GuardedFlash, WriteIntent};
1314
use crate::mpi::messages::ImageType;
1415

1516
/// Shared read/write subcommands for a flash-chip region.
@@ -99,37 +100,34 @@ pub fn run_write(
99100

100101
let mut card = crate::card::discover_one(&bdf)
101102
.map_err(|e| crate::Error::Other(format!("discover_one({}): {}", bdf, e)))?;
102-
card.write_region(image_type, &data)
103-
.map_err(|e| crate::Error::Other(format!("{} write: {}", region, e)))?;
104103

105-
// Read-back verify (ADR-015 Rule 5): re-upload and compare sha.
106-
match card.read_region(image_type) {
107-
Ok(rb) => {
108-
let want = sha_hex(&data);
109-
let got = sha_hex(&rb[..rb.len().min(data.len())]);
110-
if got == want {
111-
eprintln!(
112-
"{} write: OK ({} bytes), read-back verified ✓",
113-
region,
114-
data.len()
115-
);
116-
} else {
117-
return Err(crate::Error::Other(format!(
118-
"{} write: read-back MISMATCH (wrote {} got {}) — investigate before trusting",
119-
region, want, got
120-
)));
121-
}
122-
}
123-
Err(e) => eprintln!(
124-
"{} write: OK ({} bytes), but read-back verify failed: {} (verify manually)",
125-
region,
126-
data.len(),
127-
e
128-
),
129-
}
104+
// All flash writes go through the guarded transaction (ADR-021). For bios/nvdata this is the
105+
// key fix: previously these writes had NO whole-flash check, so a write that spilled into the
106+
// boot-critical region (the 2026-05-20 brick mechanism) went undetected. Now every byte of the
107+
// boot-input zone is verified unchanged against the mandatory pre-write snapshot, and banks are
108+
// checked for consistency, regardless of which region was targeted.
109+
let txn = GuardedFlash::begin(&bdf, &snapshot_dir())?;
110+
txn.commit(
111+
&mut *card,
112+
WriteIntent {
113+
region: image_type,
114+
bytes: data.clone(),
115+
label: region.to_string(),
116+
},
117+
)?;
118+
eprintln!(
119+
"{} write: OK ({} bytes) — guarded transaction verified ✓",
120+
region,
121+
data.len()
122+
);
130123
Ok(())
131124
}
132125

126+
/// Directory where guarded-flash writes drop the pre-write recovery snapshot (current dir).
127+
fn snapshot_dir() -> std::path::PathBuf {
128+
std::env::current_dir().unwrap_or_else(|_| std::path::PathBuf::from("."))
129+
}
130+
133131
/// Dispatch a `bios`/`nvdata` region subcommand (fw has its own enum w/ extras).
134132
pub fn run(
135133
bdf: String,

0 commit comments

Comments
 (0)