From 4da26d0836a3aeb79b1ac156470a96875c76c187 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 bf2f36c3d9..387efdb1b7 100644 --- a/examples/companion_radio/DataStore.cpp +++ b/examples/companion_radio/DataStore.cpp @@ -279,42 +279,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, bool (*filter)(const ContactInfo& c)) { - 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)) { if (filter && !filter(c)) { @@ -334,11 +349,26 @@ void DataStore::saveContacts(DataStoreHost* host, bool (*filter)(const ContactIn 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 f587b53fa85c2a36832f9b3ac446d36b98b3fec2 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 387efdb1b7..e629032c9d 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, "") @@ -282,6 +288,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(); @@ -290,6 +297,7 @@ void DataStore::loadContacts(DataStoreHost* host) { file = openRead(fs, "/contacts3.bak"); } } +#endif if (file) { bool full = false; @@ -323,8 +331,12 @@ void DataStore::loadContacts(DataStoreHost* host) { void DataStore::saveContacts(DataStoreHost* host, bool (*filter)(const ContactInfo& c)) { 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; @@ -359,16 +371,17 @@ void DataStore::saveContacts(DataStoreHost* host, bool (*filter)(const ContactIn 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 146c859a6560725aab5508ffce658734d5d138cb 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 e629032c9d..de9f9eef20 100644 --- a/examples/companion_radio/DataStore.cpp +++ b/examples/companion_radio/DataStore.cpp @@ -373,7 +373,6 @@ void DataStore::saveContacts(DataStoreHost* host, bool (*filter)(const ContactIn #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");