diff --git a/firmware/platforms/stm32f7/src/f722/config.rs b/firmware/platforms/stm32f7/src/f722/config.rs index 6a7e68c..b0ad462 100644 --- a/firmware/platforms/stm32f7/src/f722/config.rs +++ b/firmware/platforms/stm32f7/src/f722/config.rs @@ -197,7 +197,10 @@ const F722_FIRST_LARGE_SECTOR: usize = 5; const FLASH_KEY1: u32 = 0x4567_0123; const FLASH_KEY2: u32 = 0xcdef_89ab; const FLASH_ERROR_FLAGS: u32 = 0x01f2; -const BUSY_POLL_LIMIT: u32 = 1_000_000_000; +// The F722 datasheet bounds one x32 128-KiB sector erase at 1.322 seconds. +// The already-running 216-MHz DWT counter gives every operation a two-second +// deadline without turning an instruction-count guess into a timing contract. +const FLASH_OPERATION_TIMEOUT_CYCLES: u32 = 2 * 216_000_000; /// One schema-independent journal shared by every STM32F722 board image. pub(crate) type F722ConfigStore = @@ -212,12 +215,16 @@ pub(crate) type F722ConfigManager = #[derive(Clone, Copy, Debug, Eq, PartialEq)] pub(crate) enum F722FlashError { Bounds, - BusyTimeout, Unlock, Controller, Verification, } +enum F722FlashOperation { + Erase { sector: u8 }, + Program { address: usize, word: u32 }, +} + /// Sole owner of the F722 flash controller used by the configuration journal. pub(crate) struct F722Nor { flash: stm32f722::FLASH, @@ -275,20 +282,106 @@ impl F722Nor { .map_err(|_| F722FlashError::Bounds) } - fn wait_ready(&self) -> Result<(), F722FlashError> { - let mut remaining = BUSY_POLL_LIMIT; + /// Executes one complete Flash operation from SRAM with IRQs masked. + /// + /// A controller that remains busy beyond the datasheet bound cannot be + /// recovered by writing `FLASH_CR`: RM0431 specifies that such a write + /// stalls until `BSY` clears. The trigger, poll, cleanup, and timeout reset + /// all stay in SRAM so neither an instruction fetch nor an IRQ vector can + /// strand execution in Flash while `BSY` remains asserted. + #[cfg_attr( + target_os = "none", + unsafe(link_section = ".data.kaptan_flash_operation") + )] + #[inline(never)] + fn execute(&self, operation: F722FlashOperation) -> Result<(), F722FlashError> { + #[cfg(target_os = "none")] + let primask: u32; + #[cfg(target_os = "none")] + // SAFETY: these instructions only read and set the core interrupt mask. + // The previous mask is restored after Flash is idle and relocked. + unsafe { + core::arch::asm!( + "mrs {primask}, PRIMASK", + "cpsid i", + primask = out(reg) primask, + options(nomem, nostack, preserves_flags), + ); + } + + match operation { + F722FlashOperation::Erase { sector } => { + // SAFETY: the caller resolved one of the two reserved sectors. + self.flash.cr().modify(|_, write| unsafe { + write + .pg() + .clear_bit() + .ser() + .set_bit() + .snb() + .bits(sector) + .psize() + .psize32() + .strt() + .start() + }); + } + F722FlashOperation::Program { address, word } => { + self.flash + .cr() + .modify(|_, write| write.ser().clear_bit().psize().psize32().pg().set_bit()); + // SAFETY: the caller checked that this word is aligned and + // contained in one reserved erased journal sector. + unsafe { core::ptr::write_volatile(address as *mut u32, word) }; + } + } + + #[cfg(target_os = "none")] + // SAFETY: this operand-free barrier orders the erase trigger or + // programming store before the status-register poll. + unsafe { + core::arch::asm!("dsb sy", options(nostack, preserves_flags)); + } + let started = cortex_m::peripheral::DWT::cycle_count(); while self.flash.sr().read().bsy().is_busy() { - if remaining == 0 { - return Err(F722FlashError::BusyTimeout); + if cortex_m::peripheral::DWT::cycle_count().wrapping_sub(started) + >= FLASH_OPERATION_TIMEOUT_CYCLES + { + reset_after_flash_timeout(); } - remaining -= 1; core::hint::spin_loop(); } - Ok(()) + + let status = self.flash.sr().read(); + self.flash.cr().modify(|_, write| { + write.pg().clear_bit(); + write.ser().clear_bit(); + write.lock().locked() + }); + if status.eop().is_active() { + self.flash.sr().write(|write| write.eop().clear()); + } + + #[cfg(target_os = "none")] + if primask & 1 == 0 { + // SAFETY: Flash is idle and relocked, so restoring the caller's + // enabled interrupt state cannot vector through a busy Flash bank. + unsafe { + core::arch::asm!("cpsie i", options(nomem, nostack, preserves_flags)); + } + } + + if status.bits() & FLASH_ERROR_FLAGS != 0 { + Err(F722FlashError::Controller) + } else { + Ok(()) + } } fn unlock(&self) -> Result<(), F722FlashError> { - self.wait_ready()?; + if self.flash.sr().read().bsy().is_busy() { + return Err(F722FlashError::Controller); + } if self.flash.cr().read().lock().is_locked() { // SAFETY: these are the exact two full-width FPEC unlock keys from // RM0431, written in the required order while the controller is idle. @@ -316,23 +409,39 @@ impl F722Nor { .write(|write| unsafe { write.bits(FLASH_ERROR_FLAGS) }); Ok(()) } +} - fn finish(&self) -> Result<(), F722FlashError> { - self.wait_ready()?; - let status = self.flash.sr().read(); - self.flash.cr().modify(|_, write| { - write.pg().clear_bit(); - write.ser().clear_bit(); - write.lock().locked() - }); - if status.bits() & FLASH_ERROR_FLAGS != 0 { - return Err(F722FlashError::Controller); +/// Requests a system reset without fetching code or data from embedded Flash. +#[inline(always)] +fn reset_after_flash_timeout() -> ! { + #[cfg(target_os = "none")] + { + const AIRCR: *mut u32 = 0xe000_ed0c as *mut u32; + const VECTKEY: u32 = 0x05fa << 16; + const PRIGROUP: u32 = 0x7 << 8; + const SYSRESETREQ: u32 = 1 << 2; + + // SAFETY: this barrier has no operands or memory address. It orders the + // preceding peripheral reads before the reset request. + unsafe { core::arch::asm!("dsb sy", options(nostack, preserves_flags)) }; + // SAFETY: AIRCR is a word-aligned private-peripheral register. VECTKEY is + // required for the write; retaining PRIGROUP avoids changing reset policy. + unsafe { + let priority_group = core::ptr::read_volatile(AIRCR) & PRIGROUP; + core::ptr::write_volatile(AIRCR, VECTKEY | priority_group | SYSRESETREQ); } - if status.eop().is_active() { - self.flash.sr().write(|write| write.eop().clear()); + // SAFETY: this barrier ensures the reset request reaches the system + // control block before the processor waits for reset. + unsafe { core::arch::asm!("dsb sy", options(nostack, preserves_flags)) }; + loop { + // SAFETY: `nop` has no operands, memory access, or observable state + // beyond keeping the reset wait loop present. + unsafe { core::arch::asm!("nop", options(nomem, nostack, preserves_flags)) }; } - Ok(()) } + + #[cfg(not(target_os = "none"))] + panic!("flash timeout reset is an embedded-only path") } impl DualSlotNor for F722Nor { @@ -356,25 +465,7 @@ impl DualSlotNor for F722Nor { fn erase_slot(&mut self, slot: usize) -> Result<(), Self::Error> { let sector = Self::sector(slot)?; self.prepare()?; - // SAFETY: RM0431 assigns sectors 6 and 7 to the two reserved 128 KiB - // ranges above. No application section is linked into either sector. - self.flash.cr().modify(|_, write| unsafe { - write - .pg() - .clear_bit() - .ser() - .set_bit() - .snb() - .bits(sector) - .psize() - .psize32() - .strt() - .start() - }); - // RM0431 permits a DSB between asserting STRT and polling BSY so the - // control-register write cannot be observed out of order. - cortex_m::asm::dsb(); - self.finish()?; + self.execute(F722FlashOperation::Erase { sector })?; let mut probe = [0_u8; 4]; self.read(slot, 0, &mut probe)?; if probe != [0xff; 4] { @@ -389,16 +480,7 @@ impl DualSlotNor for F722Nor { } let address = Self::address(slot, offset, 4)?; self.prepare()?; - self.flash - .cr() - .modify(|_, write| write.ser().clear_bit().psize().psize32().pg().set_bit()); - // SAFETY: the checked address is aligned and contained in a reserved - // erased journal sector, and PG with x32 parallelism is active. - unsafe { core::ptr::write_volatile(address as *mut u32, word) }; - // RM0431 requires the programming store to complete before BSY is - // polled or the PG/control state is changed. - cortex_m::asm::dsb(); - self.finish()?; + self.execute(F722FlashOperation::Program { address, word })?; // SAFETY: the same checked flash word is readable after BSY clears. if unsafe { core::ptr::read_volatile(address as *const u32) } != word { return Err(F722FlashError::Verification); diff --git a/firmware/platforms/stm32f7/tests/platform_safety_boundary.rs b/firmware/platforms/stm32f7/tests/platform_safety_boundary.rs index ee35fbd..d196ab3 100644 --- a/firmware/platforms/stm32f7/tests/platform_safety_boundary.rs +++ b/firmware/platforms/stm32f7/tests/platform_safety_boundary.rs @@ -183,6 +183,20 @@ fn ordinary_f722_image_has_direct_configuration_after_output_inhibition() { ); } +#[test] +fn flash_busy_deadline_resets_entirely_from_sram() { + let config = platform_source("src/f722/config.rs"); + assert!(config.contains("link_section = \".data.kaptan_flash_operation\"")); + assert!(config.contains("fn execute(&self, operation: F722FlashOperation)")); + assert!(config.contains("\"mrs {primask}, PRIMASK\"")); + assert!(config.contains("\"cpsid i\"")); + assert!(config.contains("\"cpsie i\"")); + assert!(config.contains("fn reset_after_flash_timeout() -> !")); + assert!(config.contains("SYSRESETREQ")); + assert!(config.contains("core::arch::asm!(\"dsb sy\"")); + assert!(!config.contains("BusyTimeout")); +} + #[test] fn usart_tx_dma_arms_each_stream_before_its_request() { let usart = platform_source("src/f722/peripherals/usart.rs"); diff --git a/firmware/scripts/analyze-fw.py b/firmware/scripts/analyze-fw.py index e189369..11e9be9 100755 --- a/firmware/scripts/analyze-fw.py +++ b/firmware/scripts/analyze-fw.py @@ -19,7 +19,7 @@ REGION = re.compile( re.MULTILINE, ) -Section = namedtuple("Section", "name size address allocated") +Section = namedtuple("Section", "name size address allocated executable") LoadSegment = namedtuple( "LoadSegment", "virtual_address physical_address file_size memory_size" ) @@ -78,6 +78,7 @@ def elf_layout(output): section["Size"], section["Address"], "SHF_ALLOC" in flags, + "SHF_EXECINSTR" in flags, ) ) @@ -186,6 +187,76 @@ def validation_errors(layout, section_list, load_segments): return errors +def flash_operation_section_errors(layout, section_list): + """Validate the output section that contains the F722 Flash operation.""" + matches = [section for section in section_list if section.name == ".data"] + if len(matches) != 1: + return ["firmware must contain exactly one .data section"] + section = matches[0] + if not section.allocated or not section.executable or section.size == 0: + return [".data must contain executable SRAM Flash-operation code"] + if region_for_span(section.address, section.size, layout) != "RAM": + return ["executable .data must reside completely in RAM"] + return [] + + +INSTRUCTION = re.compile( + r"^\s*([0-9A-Fa-f]+):\s+([A-Za-z][A-Za-z0-9.]*)\s*(.*?)\s*$" +) +DIRECT_BRANCH = re.compile( + r"^(?:b(?:eq|ne|cs|hs|cc|lo|mi|pl|vs|vc|hi|ls|ge|lt|gt|le)?" + r"(?:\.[nw])?|blx?|bx|cbz|cbnz)$" +) + + +def sram_control_flow_errors(section, disassembly): + """Reject external control flow and PC-relative loads from SRAM code.""" + errors = [] + instructions = 0 + end = section.address + section.size + for line in disassembly.splitlines(): + match = INSTRUCTION.match(line) + if match is None: + continue + instructions += 1 + address = int(match.group(1), 16) + mnemonic = match.group(2).lower() + operands = match.group(3).split("@", 1)[0].strip().lower() + if not section.address <= address < end: + errors.append(f"instruction 0x{address:08X} is outside .data") + continue + + if DIRECT_BRANCH.fullmatch(mnemonic): + target = re.search(r"\b0x([0-9a-f]+)\b", operands) + if target is None: + if mnemonic == "bx" and operands == "lr": + continue + errors.append( + f"0x{address:08X} uses indirect control transfer {mnemonic}" + ) + continue + destination = int(target.group(1), 16) + if not section.address <= destination < end: + errors.append( + f"0x{address:08X} branches outside SRAM .data to " + f"0x{destination:08X}" + ) + continue + + if mnemonic in ("tbb", "tbh"): + errors.append(f"0x{address:08X} uses indirect control transfer {mnemonic}") + elif mnemonic == "pop" and re.search(r"\bpc\b", operands): + continue + elif re.match(r"^pc(?:\s|,|$)", operands): + errors.append(f"0x{address:08X} writes the program counter indirectly") + elif mnemonic.startswith("ldr") and re.search(r"\[\s*pc\b", operands): + errors.append(f"0x{address:08X} performs a PC-relative load") + + if instructions == 0: + errors.append("llvm-objdump found no instructions in executable .data") + return errors + + def main(): if len(sys.argv) != 3: raise ValueError(f"usage: {sys.argv[0]} ") @@ -248,6 +319,26 @@ def main(): if errors: raise ValueError("firmware memory validation failed:\n " + "\n ".join(errors)) + errors = flash_operation_section_errors(layout, section_list) + if errors: + raise ValueError("Flash-operation placement validation failed:\n " + "\n ".join(errors)) + data = next(section for section in section_list if section.name == ".data") + objdump = llvm_tool("llvm-objdump") + errors = sram_control_flow_errors( + data, + run( + [ + str(objdump), + "-d", + "--no-show-raw-insn", + "--section=.data", + str(elf), + ] + ), + ) + if errors: + raise ValueError("Flash-operation control-flow validation failed:\n " + "\n ".join(errors)) + if __name__ == "__main__": try: diff --git a/firmware/scripts/test-analyze-fw.py b/firmware/scripts/test-analyze-fw.py index 1fe3e2d..ee18d6c 100644 --- a/firmware/scripts/test-analyze-fw.py +++ b/firmware/scripts/test-analyze-fw.py @@ -28,7 +28,12 @@ class FirmwareMemoryValidationTests(unittest.TestCase): "Name": {"Name": ".text"}, "Size": 8, "Address": 0x08000000, - "Flags": {"Flags": [{"Name": "SHF_ALLOC"}]}, + "Flags": { + "Flags": [ + {"Name": "SHF_ALLOC"}, + {"Name": "SHF_EXECINSTR"}, + ] + }, } }, { @@ -68,8 +73,8 @@ class FirmwareMemoryValidationTests(unittest.TestCase): self.assertEqual( sections, [ - ANALYZER.Section(".text", 8, 0x08000000, True), - ANALYZER.Section(".debug_info", 100, 0, False), + ANALYZER.Section(".text", 8, 0x08000000, True, True), + ANALYZER.Section(".debug_info", 100, 0, False, False), ], ) self.assertEqual( @@ -79,10 +84,10 @@ class FirmwareMemoryValidationTests(unittest.TestCase): def test_accepts_allocated_memory_and_ignores_debug_sections(self): sections = [ - ANALYZER.Section(".text", 0x80, 0x08000000, True), - ANALYZER.Section(".bss", 0x40, 0x20000000, True), - ANALYZER.Section(".debug_info", 0x1000, 0, False), - ANALYZER.Section(".empty", 0, 0xDEADBEEF, True), + ANALYZER.Section(".text", 0x80, 0x08000000, True, True), + ANALYZER.Section(".bss", 0x40, 0x20000000, True, False), + ANALYZER.Section(".debug_info", 0x1000, 0, False, False), + ANALYZER.Section(".empty", 0, 0xDEADBEEF, True, False), ] segments = [ ANALYZER.LoadSegment(0x08000000, 0x08000000, 0x80, 0x80), @@ -92,8 +97,8 @@ class FirmwareMemoryValidationTests(unittest.TestCase): self.assertEqual(ANALYZER.validation_errors(LAYOUT, sections, segments), []) def test_rejects_allocated_sections_outside_or_crossing_a_region(self): - outside = [ANALYZER.Section(".text", 4, 0x08000200, True)] - crossing = [ANALYZER.Section(".text", 2, 0x080000FF, True)] + outside = [ANALYZER.Section(".text", 4, 0x08000200, True, True)] + crossing = [ANALYZER.Section(".text", 2, 0x080000FF, True, True)] self.assertIn( "allocated section .text", @@ -106,8 +111,8 @@ class FirmwareMemoryValidationTests(unittest.TestCase): def test_usage_counts_data_load_image_without_double_counting_text(self): sections = [ - ANALYZER.Section(".text", 0x40, 0x08000000, True), - ANALYZER.Section(".data", 0x10, 0x20000000, True), + ANALYZER.Section(".text", 0x40, 0x08000000, True, True), + ANALYZER.Section(".data", 0x10, 0x20000000, True, False), ] segments = [ ANALYZER.LoadSegment(0x08000000, 0x08000000, 0x40, 0x40), @@ -131,6 +136,53 @@ class FirmwareMemoryValidationTests(unittest.TestCase): self.assertTrue(any("virtual span" in error for error in errors)) self.assertTrue(any("physical span" in error for error in errors)) + def test_requires_one_executable_data_section_in_ram(self): + valid = [ANALYZER.Section(".data", 0x20, 0x20000000, True, True)] + non_executable = [ + ANALYZER.Section(".data", 0x20, 0x20000000, True, False) + ] + in_flash = [ANALYZER.Section(".data", 0x20, 0x08000000, True, True)] + + self.assertEqual(ANALYZER.flash_operation_section_errors(LAYOUT, valid), []) + self.assertIn( + "executable SRAM", + ANALYZER.flash_operation_section_errors(LAYOUT, non_executable)[0], + ) + self.assertIn( + "reside completely in RAM", + ANALYZER.flash_operation_section_errors(LAYOUT, in_flash)[0], + ) + + def test_sram_control_flow_rejects_external_or_indirect_fetches(self): + section = ANALYZER.Section(".data", 0x20, 0x20000000, True, True) + safe = """ +20000000: bne 0x20000008 <.data+0x8> +20000004: ldr r0, [r1] +20000008: pop {r7, pc} +""" + external = "20000000: bl 0x08000100 " + indirect_call = "20000000: blx r3" + indirect_branch = "20000000: bx r3" + literal = "20000000: ldr r0, [pc, #4]" + + self.assertEqual(ANALYZER.sram_control_flow_errors(section, safe), []) + self.assertIn( + "branches outside SRAM", + ANALYZER.sram_control_flow_errors(section, external)[0], + ) + self.assertIn( + "indirect control transfer", + ANALYZER.sram_control_flow_errors(section, indirect_call)[0], + ) + self.assertIn( + "indirect control transfer", + ANALYZER.sram_control_flow_errors(section, indirect_branch)[0], + ) + self.assertIn( + "PC-relative load", + ANALYZER.sram_control_flow_errors(section, literal)[0], + ) + if __name__ == "__main__": unittest.main()