From 20e8cb524ff82ced4c6d0ae8849cca5029d6e96f Mon Sep 17 00:00:00 2001 From: Brad Barnett <127794626+bdbarnett@users.noreply.github.com> Date: Sat, 26 Sep 2026 15:48:54 -0500 Subject: [PATCH] firmware flash: refuse an app image at the bootloader offset An ESP-IDF application image (micropython.bin) written below the partition table is loaded by the ROM as a bootloader: the board boot-loops on a watchdog reset and the write runs over the partition table. flash_esp32 now reads the image header (esp_app_desc_t magic at byte 32) and refuses before touching the device, naming the build's firmware.bin. mpftp firmware flash also passes --offset through, so an app image can still go to its partition. Fixes #67 --- cli/src/mpftp/cli.py | 4 ++ cli/src/mpftp/firmware.py | 54 +++++++++++++++++ cli/tests/test_app_image_offset.py | 97 ++++++++++++++++++++++++++++++ 3 files changed, 155 insertions(+) create mode 100644 cli/tests/test_app_image_offset.py diff --git a/cli/src/mpftp/cli.py b/cli/src/mpftp/cli.py index 56767d5..02b01ba 100755 --- a/cli/src/mpftp/cli.py +++ b/cli/src/mpftp/cli.py @@ -2042,6 +2042,8 @@ def cmd_firmware(ns: argparse.Namespace) -> None: extra += ["--artifact", ns.artifact] if getattr(ns, "family", None): extra += ["--family", ns.family] + if getattr(ns, "offset", None): + extra += ["--offset", ns.offset] if getattr(ns, "erase", False): extra.append("--erase") if getattr(ns, "uf2", False): @@ -2615,6 +2617,8 @@ def build_parser() -> argparse.ArgumentParser: fwf = fwsub.add_parser("flash", parents=[fw_sel, device_opts], help="Flash a built or downloaded artifact") fwf.add_argument("--artifact", help="Explicit firmware file (else last build)") fwf.add_argument("--family", default="", help="MCU family for flash offset (download mode)") + fwf.add_argument("--offset", default="", + help="esp32 flash offset (default: the bootloader offset for the chip)") fwf.add_argument("--erase", action="store_true", help="esp32: erase flash first") fwf.add_argument("--uf2", action="store_true", help="Copy a .uf2 to a bootloader volume instead of flashing over serial") diff --git a/cli/src/mpftp/firmware.py b/cli/src/mpftp/firmware.py index b8024b7..b9bfcaa 100644 --- a/cli/src/mpftp/firmware.py +++ b/cli/src/mpftp/firmware.py @@ -1963,6 +1963,56 @@ def _esp32_layout_check( } +# An ESP-IDF image starts with 0xE9. An application image carries its +# esp_app_desc_t at the start of its first segment: after the 24-byte image +# header and the 8-byte segment header, so at byte 32. A bootloader, and the +# combined firmware.bin that starts with one, has something else there. +_ESP_IMAGE_MAGIC = 0xE9 +_ESP_APP_DESC_MAGIC = 0xABCD5432 +_ESP_APP_DESC_AT = 32 + + +def esp_image_is_app(artifact: Path) -> bool: + """True when ``artifact`` is an ESP-IDF application image (micropython.bin).""" + try: + with artifact.open("rb") as fh: + head = fh.read(_ESP_APP_DESC_AT + 4) + except OSError: + return False + if len(head) < _ESP_APP_DESC_AT + 4 or head[0] != _ESP_IMAGE_MAGIC: + return False + return struct.unpack_from(" Optional[str]: + """Why writing ``artifact`` at ``offset`` would brick the boot, or None. + + Everything below the partition table (0x8000) is the second-stage + bootloader's. An application image written there is loaded by the ROM as a + bootloader, so the board boot-loops on a watchdog reset, and the write + runs over the partition table too. The image meant for that offset is the + combined ``firmware.bin``, which starts with the bootloader. + """ + try: + at = int(str(offset), 0) + except (TypeError, ValueError): + return None + if at >= _PARTITION_TABLE_OFFSET or not esp_image_is_app(artifact): + return None + combined = artifact.parent / "firmware.bin" + hint = ( + f"Flash {combined} instead" + if combined.is_file() and combined != artifact + else "Flash the build's combined firmware.bin instead" + ) + return ( + f"{artifact.name} is an application image, and {hex(at)} is the " + "bootloader's offset: the board would boot-loop and lose its partition " + f"table. {hint}, or pass the application partition's offset " + "(usually 0x10000) with --offset." + ) + + def flash_esp32(ns: argparse.Namespace, mp: Optional[Path], artifact: Path) -> None: port_dir = (mp / "ports" / ns.port) if mp else Path(".") family = getattr(ns, "family", "") or "" @@ -1977,6 +2027,10 @@ def flash_esp32(ns: argparse.Namespace, mp: Optional[Path], artifact: Path) -> N ), ) emit_log(f"[mpftp] flash offset {offset}") + wrong_image = app_image_at_bootloader_error(artifact, offset) + if wrong_image: + emit_result(False, error=wrong_image) + return fw = str(artifact) if HOST == "wsl": fw = _wslpath_w(fw) diff --git a/cli/tests/test_app_image_offset.py b/cli/tests/test_app_image_offset.py new file mode 100644 index 0000000..1dddc37 --- /dev/null +++ b/cli/tests/test_app_image_offset.py @@ -0,0 +1,97 @@ +"""An application image written at the bootloader offset boot-loops the board. + +mpftp#67: ``micropython.bin`` flashed with ``--artifact`` on the P4 went to +0x2000, the ROM loaded it as a bootloader, and the board sat in a watchdog +reset loop with its partition table overwritten. The combined +``firmware.bin`` is what belongs there. Each test plants the image that +caused it, not only a good one. +""" + +from __future__ import annotations + +import argparse +import struct +import tempfile +import unittest +from pathlib import Path +from unittest import mock + +from mpftp import firmware + + +def image(desc_word: int) -> bytes: + """An ESP image header, one segment header, then a 4-byte word at byte 32.""" + header = bytes([0xE9, 1, 2, 0x20]) + b"\x00" * 20 + segment = struct.pack(" Path: + path = self.dir / name + path.write_bytes(data) + return path + + def test_an_app_image_is_recognised(self): + self.assertTrue(firmware.esp_image_is_app(self.write("micropython.bin", APP))) + + def test_a_bootloader_led_image_is_not_an_app(self): + self.assertFalse(firmware.esp_image_is_app(self.write("firmware.bin", BOOTLOADER))) + + def test_a_file_that_is_not_an_esp_image_is_not_an_app(self): + self.assertFalse(firmware.esp_image_is_app(self.write("x.bin", b"\x00" * 64))) + self.assertFalse(firmware.esp_image_is_app(self.write("short.bin", APP[:20]))) + + def test_app_at_the_bootloader_offset_is_refused_and_names_firmware_bin(self): + app = self.write("micropython.bin", APP) + self.write("firmware.bin", BOOTLOADER) + for offset in ("0x2000", "0x1000", "0x0"): + with self.subTest(offset=offset): + why = firmware.app_image_at_bootloader_error(app, offset) + self.assertIsNotNone(why) + self.assertIn(str(self.dir / "firmware.bin"), why) + + def test_app_at_the_app_partition_is_allowed(self): + app = self.write("micropython.bin", APP) + self.assertIsNone(firmware.app_image_at_bootloader_error(app, "0x10000")) + + def test_combined_image_at_the_bootloader_offset_is_allowed(self): + fw = self.write("firmware.bin", BOOTLOADER) + self.assertIsNone(firmware.app_image_at_bootloader_error(fw, "0x2000")) + + +class FlashEsp32Tests(unittest.TestCase): + def test_flash_refuses_before_touching_the_device(self): + with tempfile.TemporaryDirectory() as tmp: + app = Path(tmp) / "micropython.bin" + app.write_bytes(APP) + ns = argparse.Namespace( + port="esp32", board="", family="esp32p4", offset="", device="COM4", + baud=460800, erase=False, before="", after="", board_dir="", + ) + with mock.patch.object(firmware, "emit_result") as result, \ + mock.patch.object(firmware, "emit_log"), \ + mock.patch.object(firmware, "_esptool_cmd", return_value=["esptool"]), \ + mock.patch.object(firmware, "_esp32_layout_check") as layout, \ + mock.patch.object(firmware, "stream_process") as run: + firmware.flash_esp32(ns, None, app) + run.assert_not_called() + layout.assert_not_called() + args, kwargs = result.call_args + self.assertFalse(args[0]) + self.assertIn("application image", kwargs["error"]) + + +if __name__ == "__main__": + unittest.main()