diff --git a/src/helpers/ota/OtaManager.h b/src/helpers/ota/OtaManager.h index 26e640fd1..457c0a192 100644 --- a/src/helpers/ota/OtaManager.h +++ b/src/helpers/ota/OtaManager.h @@ -324,15 +324,19 @@ public: }; // A lightweight catalog entry: what we advertise per mota + how to load its ServeView on demand. struct ServeEntry { - uint8_t mid[4]; - uint32_t target_id, fw_version; - uint8_t codec_id, flags; - uint32_t have_count; // blocks we currently hold (== block_count when complete) - bool is_self; // true => entry is view0 (our own fw / RAM mota) + // Named fields only: this is not a wire/persistent layout. Group aligned + // values ahead of byte flags instead of paying three padding gaps per row. MotaSource* src; // load from this external source ... - uint8_t src_idx; // ... at this index MotaDesc desc; // cached region offsets for the source entry + uint32_t target_id, fw_version; + uint32_t have_count; // blocks we currently hold (== block_count when complete) + uint8_t mid[4]; + uint8_t codec_id, flags; + bool is_self; // true => entry is view0 (our own fw / RAM mota) + uint8_t src_idx; // ... at this index }; + static_assert(sizeof(ServeEntry) <= sizeof(MotaDesc) + sizeof(MotaSource*) + 20u, + "OTA source catalog must not add per-entry alignment padding"); // Context for the source-payload reader trampoline. struct SrcReadCtx { MotaSource* src; uint8_t idx; uint32_t payload_off; }; @@ -905,17 +909,18 @@ private: struct Source { uint8_t seeder[4]; uint8_t digest[4]; - uint8_t n_motas; uint32_t last_ms; + uint32_t have_mask; + uint32_t query_at; + uint32_t query_retry_at; + uint8_t n_motas; bool have_catalog; uint8_t have_total; - uint32_t have_mask; bool query_pending; bool query_owned; uint8_t query_retries; - uint32_t query_at; - uint32_t query_retry_at; }; + static_assert(sizeof(Source) == 32u, "OTA source discovery rows must retain compact natural alignment"); Source _sources[OTA_MAX_SOURCES]; uint8_t _n_src = 0; CatRow _catalog_inline[OTA_INLINE_CATALOG]; diff --git a/src/helpers/ota/OtaStoreQspiNrf52.h b/src/helpers/ota/OtaStoreQspiNrf52.h index 7dc70bf39..c21e70eec 100644 --- a/src/helpers/ota/OtaStoreQspiNrf52.h +++ b/src/helpers/ota/OtaStoreQspiNrf52.h @@ -126,7 +126,22 @@ class OtaStoreQspiNrf52 : public OtaStore { static const uint32_t PAGE = 4096; static const uint32_t PROGRAM = 256; static const uint32_t MAX_FLASH = 16UL * 1024 * 1024; - static const uint32_t MAX_PAGES = MAX_FLASH / PAGE; + // Exact-board recipes already reject a different physical capacity before + // any page access. Track only that accepted geometry, without reducing the + // 16 MiB capability of generic builds or either staging page buffer. +#if defined(OTA_QSPI_EXPECTED_SIZE) + // Check the original constant before narrowing: 4 GiB + 2 MiB must not + // silently wrap into an apparently valid 2 MiB geometry. + static_assert((OTA_QSPI_EXPECTED_SIZE) >= (1ULL << 20) && + (OTA_QSPI_EXPECTED_SIZE) <= MAX_FLASH && + ((OTA_QSPI_EXPECTED_SIZE) & ((OTA_QSPI_EXPECTED_SIZE) - 1ULL)) == 0ULL, + "expected QSPI capacity must match accepted 1..16 MiB JEDEC geometry"); + static const uint32_t TRACKED_FLASH = OTA_QSPI_EXPECTED_SIZE; +#else + static const uint32_t TRACKED_FLASH = MAX_FLASH; +#endif + static const uint32_t MAX_PAGES = TRACKED_FLASH / PAGE; + static const uint32_t KNOWN_PAGE_BYTES = (MAX_PAGES + 7u) / 8u; static const uint32_t INVALID_PAGE = 0xFFFFFFFFUL; uint32_t _total = 0; @@ -150,7 +165,7 @@ class OtaStoreQspiNrf52 : public OtaStore { alignas(4) uint8_t _meta_page[PAGE]; alignas(4) uint8_t _data_page[PAGE]; alignas(4) uint8_t _bounce[PROGRAM]; - uint8_t _known_pages[MAX_PAGES / 8]; + uint8_t _known_pages[KNOWN_PAGE_BYTES ? KNOWN_PAGE_BYTES : 1u]; void fail(const char *message); bool ensureFlash(); diff --git a/test/test_ota_catalog_lifetime.py b/test/test_ota_catalog_lifetime.py index e9c6fd6b8..124f607be 100644 --- a/test/test_ota_catalog_lifetime.py +++ b/test/test_ota_catalog_lifetime.py @@ -7,6 +7,9 @@ import subprocess import tempfile import unittest +# Existing CI entry point also protects feature-preserving OTA RAM layouts. +from test_ota_memory_layout import OtaMemoryLayoutTest + ROOT = Path(__file__).resolve().parents[1] diff --git a/test/test_ota_memory_layout.py b/test/test_ota_memory_layout.py new file mode 100644 index 000000000..b05c12895 --- /dev/null +++ b/test/test_ota_memory_layout.py @@ -0,0 +1,137 @@ +#!/usr/bin/env python3 +"""Preserve OTA capacities while removing padding and oversized page bitmaps.""" + +from pathlib import Path +import shutil +import subprocess +import tempfile +import unittest + +from test_replay_reset_integration import extract_braced + + +ROOT = Path(__file__).resolve().parents[1] + + +class OtaMemoryLayoutTest(unittest.TestCase): + def test_real_headers_preserve_natural_alignment_and_descriptor_initialization(self): + source = r''' +#include +#include +#include +#include +using namespace mesh::ota; +static_assert(sizeof(OtaManager::ServeEntry) == (sizeof(void*) == 4 ? 60u : 64u), + "source entries must retain every field without old padding"); +static_assert(alignof(OtaManager::ServeEntry) >= alignof(MotaSource*), "unaligned source pointer"); +static_assert(offsetof(OtaManager::ServeEntry, desc) % alignof(MotaDesc) == 0, "unaligned descriptor"); +static_assert(offsetof(OtaManager::ServeEntry, target_id) % alignof(uint32_t) == 0, "unaligned word"); +static_assert(sizeof(OtaStoreQspiNrf52) > 8192u, "both staging page buffers must remain"); +int main() { + OtaManager::ServeEntry row; + assert(row.desc.target_id == 0 && row.desc.fw_version == 0 && row.desc.source_caps == 0); + assert(row.desc.mid[0] == 0 && row.desc.mid[3] == 0 && row.desc.total_size == 0); + row.mid[0] = 17; row.target_id = 123; row.fw_version = 456; + row.have_count = 789; row.src = nullptr; row.src_idx = 255; + row.codec_id = 2; row.flags = 3; row.is_self = true; + row.desc.target_id = 987; row.desc.payload_size = 654; + const OtaManager::ServeEntry copy = row; + assert(copy.mid[0] == 17 && copy.target_id == 123 && copy.fw_version == 456); + assert(copy.have_count == 789 && copy.src == nullptr && copy.src_idx == 255); + assert(copy.codec_id == 2 && copy.flags == 3 && copy.is_self); + assert(copy.desc.target_id == 987 && copy.desc.payload_size == 654); +} +''' + with tempfile.TemporaryDirectory(prefix="ota-layout-") as directory: + work = Path(directory) + fixture = work / "layout.cpp" + fixture.write_text(source, encoding="ascii") + flags = ["-std=c++17", "-DNRF52_PLATFORM=1", "-DOTA_QSPI_STORE=1", + "-DOTA_QSPI_EXPECTED_SIZE=2097152UL", "-I", str(ROOT / "src")] + compiler = shutil.which("c++") or shutil.which("g++") + self.assertIsNotNone(compiler) + binary = work / "layout" + result = subprocess.run([compiler, *flags, "-fsanitize=address,undefined", "-fno-pie", "-no-pie", + str(fixture), "-o", str(binary)], capture_output=True, text=True) + self.assertEqual(result.returncode, 0, result.stdout + result.stderr) + subprocess.run([str(binary)], check=True, timeout=10) + arm = shutil.which("arm-none-eabi-g++") + if arm: + result = subprocess.run([arm, *flags, "-mcpu=cortex-m4", "-mthumb", "-c", str(fixture), + "-o", str(work / "layout-arm.o")], capture_output=True, + text=True, timeout=30) + self.assertEqual(result.returncode, 0, result.stdout + result.stderr) + + def test_production_bitmap_geometry_and_boundary_accesses(self): + header = (ROOT / "src/helpers/ota/OtaStoreQspiNrf52.h").read_text() + source = (ROOT / "src/helpers/ota/OtaStoreQspiNrf52.cpp").read_text() + constants = header.split(" static const uint32_t PAGE =", 1)[1] + constants = " static const uint32_t PAGE =" + constants.split( + " static const uint32_t INVALID_PAGE", 1)[0] + member = next(line.strip() for line in header.splitlines() if "uint8_t _known_pages[" in line) + methods = "\n".join(extract_braced(source, signature) for signature in ( + "bool OtaStoreQspiNrf52::pageKnown(uint32_t page) const", + "void OtaStoreQspiNrf52::setPageKnown(uint32_t page)")) + fixture = """ +#include +#include +#include +class Bitmap { +public: +CONSTANTS + uint8_t before[8]; + MEMBER + uint8_t after[8]; + bool pageKnown(uint32_t page) const; + void setPageKnown(uint32_t page); +}; +METHODS +int main() { + Bitmap value; + static_assert(sizeof(value._known_pages) == EXPECT_BITMAP_BYTES, "bitmap was not sized to accepted capacity"); + static_assert(sizeof(value._known_pages) > 0u, "zero-length bitmap"); + memset(value.before, 0xA7, sizeof(value.before)); + memset(value.after, 0xD3, sizeof(value.after)); + memset(value._known_pages, 0, sizeof(value._known_pages)); + for (uint32_t page = 0; page < Bitmap::MAX_PAGES; ++page) { + assert(!value.pageKnown(page)); value.setPageKnown(page); assert(value.pageKnown(page)); + if (page + 1 < Bitmap::MAX_PAGES) assert(!value.pageKnown(page + 1)); + } + uint8_t snapshot[sizeof(value._known_pages)]; + memcpy(snapshot, value._known_pages, sizeof(snapshot)); + const uint32_t invalid[] = {Bitmap::MAX_PAGES, Bitmap::MAX_PAGES + 1u, UINT32_MAX}; + for (uint32_t page : invalid) { assert(!value.pageKnown(page)); value.setPageKnown(page); } + assert(memcmp(snapshot, value._known_pages, sizeof(snapshot)) == 0); + for (uint8_t byte : value.before) assert(byte == 0xA7); + for (uint8_t byte : value.after) assert(byte == 0xD3); +} +""".replace("CONSTANTS", constants).replace("MEMBER", member).replace( + "METHODS", methods.replace("OtaStoreQspiNrf52::", "Bitmap::")) + compiler = shutil.which("c++") or shutil.which("g++") + self.assertIsNotNone(compiler) + with tempfile.TemporaryDirectory(prefix="ota-bitmap-") as directory: + work = Path(directory) + path = work / "bitmap.cpp" + path.write_text(fixture, encoding="ascii") + for mib in (None, 1, 2, 4, 8, 16): + with self.subTest(expected_mib=mib): + binary = work / ("generic" if mib is None else str(mib)) + flags = [] if mib is None else [f"-DOTA_QSPI_EXPECTED_SIZE={mib << 20}UL"] + expected = (16 if mib is None else mib) * 32 + subprocess.run([compiler, "-std=c++17", "-Wall", "-Wextra", "-Werror", + "-fsanitize=address,undefined", "-fno-pie", "-no-pie", *flags, + f"-DEXPECT_BITMAP_BYTES={expected}", str(path), "-o", str(binary)], + check=True, capture_output=True, text=True, timeout=30) + subprocess.run([str(binary)], check=True, timeout=10) + for invalid in (0, 1, 4096, 1048575, 3145728, 33554432, (1 << 32) + (2 << 20)): + with self.subTest(invalid=invalid): + result = subprocess.run([compiler, "-std=c++17", "-c", str(path), + f"-DOTA_QSPI_EXPECTED_SIZE={invalid}UL", + "-DEXPECT_BITMAP_BYTES=1", "-o", str(work / "invalid.o")], + capture_output=True, text=True, timeout=30) + self.assertNotEqual(result.returncode, 0) + self.assertIn("expected QSPI capacity must match", result.stderr) + + +if __name__ == "__main__": + unittest.main()