From 870f522e8544bc90941d98641143eaae013caea6 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 40f1ceeb61..bf65330674 100644 --- a/examples/companion_radio/DataStore.cpp +++ b/examples/companion_radio/DataStore.cpp @@ -275,42 +275,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); @@ -326,11 +341,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 71b1bf702481aece7c1727178758d131fe8cfb47 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 bf65330674..29cad56d57 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, "") @@ -278,6 +284,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(); @@ -286,6 +293,7 @@ void DataStore::loadContacts(DataStoreHost* host) { file = openRead(fs, "/contacts3.bak"); } } +#endif if (file) { bool full = false; @@ -319,8 +327,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; @@ -351,16 +363,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 e4c480b3d3e0d0334d60a6d1f40a723d1455d44b 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 29cad56d57..afbfc1b2f8 100644 --- a/examples/companion_radio/DataStore.cpp +++ b/examples/companion_radio/DataStore.cpp @@ -365,7 +365,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");