From c0096070e7207f7a3522f524fb19e0e386082051 Mon Sep 17 00:00:00 2001 From: Wessel Nieboer Date: Sat, 10 Jan 2026 16:42:39 +0100 Subject: [PATCH 1/3] Backup contacts to tmpFile before saving --- examples/companion_radio/DataStore.cpp | 86 +++++++++++++++++--------- 1 file changed, 58 insertions(+), 28 deletions(-) diff --git a/examples/companion_radio/DataStore.cpp b/examples/companion_radio/DataStore.cpp index fba64e8c6..7a5c9072c 100644 --- a/examples/companion_radio/DataStore.cpp +++ b/examples/companion_radio/DataStore.cpp @@ -271,42 +271,57 @@ void DataStore::savePrefs(const NodePrefs& _prefs, double node_lat, double node_ } void DataStore::loadContacts(DataStoreHost* host) { -File file = openRead(_getContactsChannelsFS(), "/contacts3"); - if (file) { - bool full = false; - while (!full) { - ContactInfo c; - uint8_t pub_key[32]; - uint8_t unused; - - bool success = (file.read(pub_key, 32) == 32); - success = success && (file.read((uint8_t *)&c.name, 32) == 32); - success = success && (file.read(&c.type, 1) == 1); - success = success && (file.read(&c.flags, 1) == 1); - success = success && (file.read(&unused, 1) == 1); - success = success && (file.read((uint8_t *)&c.sync_since, 4) == 4); // was 'reserved' - success = success && (file.read((uint8_t *)&c.out_path_len, 1) == 1); - success = success && (file.read((uint8_t *)&c.last_advert_timestamp, 4) == 4); - success = success && (file.read(c.out_path, 64) == 64); - success = success && (file.read((uint8_t *)&c.lastmod, 4) == 4); - success = success && (file.read((uint8_t *)&c.gps_lat, 4) == 4); - success = success && (file.read((uint8_t *)&c.gps_lon, 4) == 4); - - if (!success) break; // EOF + FILESYSTEM* fs = _getContactsChannelsFS(); + File file = openRead(fs, "/contacts3"); + + // If main file doesn't exist or is empty, try backup + if (!file || file.size() == 0) { + if (file) file.close(); + if (fs->exists("/contacts3.bak")) { + MESH_DEBUG_PRINTLN("WARN: contacts3 missing/empty, loading from backup"); + file = openRead(fs, "/contacts3.bak"); + } + } - c.id = mesh::Identity(pub_key); - if (!host->onContactLoaded(c)) full = true; - } - file.close(); + if (file) { + bool full = false; + while (!full) { + ContactInfo c; + uint8_t pub_key[32]; + uint8_t unused; + + bool success = (file.read(pub_key, 32) == 32); + success = success && (file.read((uint8_t *)&c.name, 32) == 32); + success = success && (file.read(&c.type, 1) == 1); + success = success && (file.read(&c.flags, 1) == 1); + success = success && (file.read(&unused, 1) == 1); + success = success && (file.read((uint8_t *)&c.sync_since, 4) == 4); // was 'reserved' + success = success && (file.read((uint8_t *)&c.out_path_len, 1) == 1); + success = success && (file.read((uint8_t *)&c.last_advert_timestamp, 4) == 4); + success = success && (file.read(c.out_path, 64) == 64); + success = success && (file.read((uint8_t *)&c.lastmod, 4) == 4); + success = success && (file.read((uint8_t *)&c.gps_lat, 4) == 4); + success = success && (file.read((uint8_t *)&c.gps_lon, 4) == 4); + + if (!success) break; // EOF + + c.id = mesh::Identity(pub_key); + if (!host->onContactLoaded(c)) full = true; } + file.close(); + } } void DataStore::saveContacts(DataStoreHost* host) { - File file = openWrite(_getContactsChannelsFS(), "/contacts3"); + FILESYSTEM* fs = _getContactsChannelsFS(); + + // Write to temp file first (atomic write pattern) + File file = openWrite(fs, "/contacts3.tmp"); if (file) { uint32_t idx = 0; ContactInfo c; uint8_t unused = 0; + bool write_success = true; while (host->getContactForSave(idx, c)) { bool success = (file.write(c.id.pub_key, 32) == 32); @@ -322,11 +337,26 @@ void DataStore::saveContacts(DataStoreHost* host) { success = success && (file.write((uint8_t *)&c.gps_lat, 4) == 4); success = success && (file.write((uint8_t *)&c.gps_lon, 4) == 4); - if (!success) break; // write failed + if (!success) { + write_success = false; + break; // write failed + } idx++; // advance to next contact } + file.flush(); file.close(); + + if (write_success) { + // Atomic swap: remove old backup, rename current to backup, rename temp to current + fs->remove("/contacts3.bak"); + fs->rename("/contacts3", "/contacts3.bak"); + fs->rename("/contacts3.tmp", "/contacts3"); + } else { + // Write failed, remove incomplete temp file + fs->remove("/contacts3.tmp"); + MESH_DEBUG_PRINTLN("ERROR: saveContacts write failed, temp file removed"); + } } } From bb4234dd7f85dae15212202fc16ac0052fb7b59c Mon Sep 17 00:00:00 2001 From: Wessel Nieboer Date: Thu, 22 Jan 2026 08:37:01 +0100 Subject: [PATCH 2/3] Only apply to devices with sufficient storage --- examples/companion_radio/DataStore.cpp | 21 +++++++++++++++++---- 1 file changed, 17 insertions(+), 4 deletions(-) diff --git a/examples/companion_radio/DataStore.cpp b/examples/companion_radio/DataStore.cpp index 7a5c9072c..6c65c407d 100644 --- a/examples/companion_radio/DataStore.cpp +++ b/examples/companion_radio/DataStore.cpp @@ -7,6 +7,12 @@ #define MAX_BLOBRECS 20 #endif +// Atomic writes require ~2x storage for contacts file +// Only enable on platforms with sufficient flash +#if !defined(NRF52_PLATFORM) || defined(EXTRAFS) || defined(QSPIFLASH) + #define HAS_ATOMIC_WRITE_SUPPORT +#endif + DataStore::DataStore(FILESYSTEM& fs, mesh::RTCClock& clock) : _fs(&fs), _fsExtra(nullptr), _clock(&clock), #if defined(NRF52_PLATFORM) || defined(STM32_PLATFORM) identity_store(fs, "") @@ -274,6 +280,7 @@ void DataStore::loadContacts(DataStoreHost* host) { FILESYSTEM* fs = _getContactsChannelsFS(); File file = openRead(fs, "/contacts3"); +#ifdef HAS_ATOMIC_WRITE_SUPPORT // If main file doesn't exist or is empty, try backup if (!file || file.size() == 0) { if (file) file.close(); @@ -282,6 +289,7 @@ void DataStore::loadContacts(DataStoreHost* host) { file = openRead(fs, "/contacts3.bak"); } } +#endif if (file) { bool full = false; @@ -315,8 +323,12 @@ void DataStore::loadContacts(DataStoreHost* host) { void DataStore::saveContacts(DataStoreHost* host) { FILESYSTEM* fs = _getContactsChannelsFS(); - // Write to temp file first (atomic write pattern) +#ifdef HAS_ATOMIC_WRITE_SUPPORT File file = openWrite(fs, "/contacts3.tmp"); +#else + File file = openWrite(fs, "/contacts3"); +#endif + if (file) { uint32_t idx = 0; ContactInfo c; @@ -347,16 +359,17 @@ void DataStore::saveContacts(DataStoreHost* host) { file.flush(); file.close(); +#ifdef HAS_ATOMIC_WRITE_SUPPORT if (write_success) { - // Atomic swap: remove old backup, rename current to backup, rename temp to current fs->remove("/contacts3.bak"); fs->rename("/contacts3", "/contacts3.bak"); fs->rename("/contacts3.tmp", "/contacts3"); + fs->remove("/contacts3.bak"); } else { - // Write failed, remove incomplete temp file fs->remove("/contacts3.tmp"); - MESH_DEBUG_PRINTLN("ERROR: saveContacts write failed, temp file removed"); + MESH_DEBUG_PRINTLN("ERROR: saveContacts write failed"); } +#endif } } From ff716bed6a291cf37213c683a879f403dab729f7 Mon Sep 17 00:00:00 2001 From: Wessel Nieboer Date: Wed, 11 Feb 2026 05:12:40 +0100 Subject: [PATCH 3/3] Remove unnecessary backup deletion before rename The fs->remove("/contacts3.bak") before the rename sequence creates a vulnerability window: if power is lost right after removing the backup but before the rename completes, both the backup and primary file could be lost. The remove is unnecessary since rename() on both SPIFFS and LittleFS replaces the target if it already exists. --- examples/companion_radio/DataStore.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/examples/companion_radio/DataStore.cpp b/examples/companion_radio/DataStore.cpp index 6c65c407d..f6e204ff8 100644 --- a/examples/companion_radio/DataStore.cpp +++ b/examples/companion_radio/DataStore.cpp @@ -361,7 +361,6 @@ void DataStore::saveContacts(DataStoreHost* host) { #ifdef HAS_ATOMIC_WRITE_SUPPORT if (write_success) { - fs->remove("/contacts3.bak"); fs->rename("/contacts3", "/contacts3.bak"); fs->rename("/contacts3.tmp", "/contacts3"); fs->remove("/contacts3.bak");