From 58427f3571fc6a9e05c1b3e00a60c32dfa9383e4 Mon Sep 17 00:00:00 2001 From: Griffin Francis Date: Wed, 12 Aug 2026 20:41:51 -0400 Subject: [PATCH] [utils] erase both swap pages in Flash::Wipe() (#13419) `Flash::Wipe()` erased only swap page 0, so stored settings (including the network key, PSKc, and SRP keys) could persist in the other page: superseded copies left behind by `Swap()`, or the full active data set when page 1 is the active page at reset time. Erase both pages, with the non-active page erased first so an interrupted wipe cannot leave a stale-but-active secondary page to be picked up by the next `Init()`. The active page index is determined defensively because `Init()` invokes `Wipe()` with an out-of-range `mSwapIndex` when no active page is found. A unit test verifying both pages are fully erased (and that blank-flash `Init()` recovery still works) is included. Co-authored-by: aussinfosec --- src/core/utils/flash.cpp | 13 ++++- tests/unit/test_flash.cpp | 105 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 117 insertions(+), 1 deletion(-) diff --git a/src/core/utils/flash.cpp b/src/core/utils/flash.cpp index b3014e6c4..78fd6c6ca 100644 --- a/src/core/utils/flash.cpp +++ b/src/core/utils/flash.cpp @@ -297,7 +297,18 @@ Error Flash::Delete(uint16_t aKey, int aIndex) void Flash::Wipe(void) { - otPlatFlashErase(&GetInstance(), 0); + // The non-active page is erased first so that an interrupted wipe + // cannot leave a stale-but-active secondary page to be resurrected on + // the next `Init()`. `Init()` may invoke `Wipe()` with `mSwapIndex == + // 2` (no active page found), so the active index is determined + // defensively before any erase. + + uint8_t activeIndex = (mSwapIndex <= 1) ? mSwapIndex : 0; + uint8_t inactiveIndex = !activeIndex; + + otPlatFlashErase(&GetInstance(), inactiveIndex); + otPlatFlashErase(&GetInstance(), activeIndex); + otPlatFlashWrite(&GetInstance(), 0, 0, &sSwapActive, sizeof(sSwapActive)); mSwapIndex = 0; diff --git a/tests/unit/test_flash.cpp b/tests/unit/test_flash.cpp index c4b23bfbf..9228a7440 100644 --- a/tests/unit/test_flash.cpp +++ b/tests/unit/test_flash.cpp @@ -186,11 +186,116 @@ void TestFlash(void) #endif // OPENTHREAD_CONFIG_PLATFORM_FLASH_API_ENABLE } +void TestFlashWipe(void) +{ +#if OPENTHREAD_CONFIG_PLATFORM_FLASH_API_ENABLE + uint8_t readBuffer[256]; + uint8_t writeBuffer[32]; + + Instance *instance = testInitInstance(); + Flash flash(*instance); + + flash.Init(); + + // Add enough records to force at least one swap, so that both swap areas + // contain (possibly superseded) settings data. + + for (uint16_t index = 0; index < 400; index++) + { + memset(writeBuffer, index & 0xff, sizeof(writeBuffer)); + SuccessOrQuit(flash.Set(index & 0x0f, writeBuffer, sizeof(writeBuffer))); + } + + flash.Wipe(); + + // After `Wipe()` the first four bytes of swap area 0 must hold a freshly + // written active swap marker, not the erased flash pattern (0xffffffff). + + { + uint32_t marker; + + otPlatFlashRead(instance, 0, 0, &marker, sizeof(marker)); + VerifyOrQuit(marker != 0xffffffff, "Wipe() did not write the active swap marker"); + } + + // After `Wipe()` no swap area may retain residual settings data: area 0 + // holds only the active swap marker (first four bytes), everything else + // must be fully erased (0xff). + + for (uint8_t swapIndex = 0; swapIndex < 2; swapIndex++) + { + uint32_t swapSize = otPlatFlashGetSwapSize(instance); + + for (uint32_t offset = (swapIndex == 0) ? sizeof(uint32_t) : 0; offset < swapSize;) + { + uint32_t size = swapSize - offset; + + if (size > sizeof(readBuffer)) + { + size = sizeof(readBuffer); + } + + otPlatFlashRead(instance, swapIndex, offset, readBuffer, size); + + for (uint32_t i = 0; i < size; i++) + { + VerifyOrQuit(readBuffer[i] == 0xff, "Wipe() left residual data in a swap area"); + } + + offset += size; + } + } + + // After `Wipe()` the previously stored records must be inaccessible via + // the API, and the instance must be immediately usable without a new + // `Init()`: a fresh `Set()`/`Get()` round-trip must succeed. + + for (uint16_t key = 0; key < 16; key++) + { + VerifyOrQuit(flash.Get(key, 0, nullptr, nullptr) == kErrorNotFound); + VerifyOrQuit(flash.Delete(key, 0) == kErrorNotFound); + } + + { + uint16_t length = sizeof(readBuffer); + + memset(writeBuffer, 0xa5, sizeof(writeBuffer)); + SuccessOrQuit(flash.Set(2, writeBuffer, sizeof(writeBuffer))); + SuccessOrQuit(flash.Get(2, 0, readBuffer, &length)); + VerifyOrQuit(length == sizeof(writeBuffer), "Get() did not return expected length"); + VerifyOrQuit(memcmp(readBuffer, writeBuffer, length) == 0, "Get() did not return expected value"); + } + + // Blank flash: `Init()` finds no active swap marker and falls back to + // `Wipe()` with an out-of-range `mSwapIndex`. It must recover into a + // usable state without passing an invalid swap index to the platform + // (the platform APIs only accept swap indices 0 and 1). + + otPlatFlashErase(instance, 0); + otPlatFlashErase(instance, 1); + + flash.Init(); + + { + uint16_t length = sizeof(readBuffer); + + memset(writeBuffer, 0x5a, sizeof(writeBuffer)); + SuccessOrQuit(flash.Set(1, writeBuffer, sizeof(writeBuffer))); + SuccessOrQuit(flash.Get(1, 0, readBuffer, &length)); + VerifyOrQuit(length == sizeof(writeBuffer), "Get() did not return expected length"); + VerifyOrQuit(memcmp(readBuffer, writeBuffer, length) == 0, "Get() did not return expected value"); + } + + testFreeInstance(instance); +#endif // OPENTHREAD_CONFIG_PLATFORM_FLASH_API_ENABLE +} + } // namespace ot int main(void) { ot::TestFlash(); + ot::TestFlashWipe(); printf("All tests passed\n"); return 0; }