fs: serialize littlefs access with a recursive mutex

littlefs is not thread-safe, yet FS is called from three different
FreeRTOS tasks: the SystemTask (fs.Init, controller loads at boot), the
display task (settings and alarm saves from the settings screens,
watchface resource loading) and the BLE host task (FSService performs
full littlefs I/O inside GATT access callbacks, and DFU writes during a
BLE file transfer can run while the user navigates settings screens).
Nothing serializes these calls today. The SpiMaster semaphore only
serializes individual bus transactions; the shared lfs_t state (caches,
open-file list, lookahead) is unprotected, so a BLE filesystem transfer
racing a settings save can corrupt filesystem state.

Take a recursive mutex in every public FS method and expose a RAII
FS::Lock so callers can hold the lock across multi-call sequences. The
recursive type matters: a caller holding FS::Lock still goes through the
public methods. Holding the lock across a sequence is needed wherever an
open file handle must not race a Rename or Delete of the same file,
since lfs_dir_commit invalidates such handles in the mlist.

Cost: one FreeRTOS recursive mutex and one pointer, no API change.
Callers that never overlapped are unaffected; overlapping callers now
briefly wait instead of interleaving inside littlefs.

Author: faisal-shah

src/components/fs/FS.cpp

@@ -2,6 +2,7 @@
 #include <cstring>
 #include <littlefs/lfs.h>
 #include <lvgl/lvgl.h>
+#include "nrf_assert.h"
 
 using namespace Pinetime::Controllers;
 
@@ -26,9 +27,12 @@ FS::FS(Pinetime::Drivers::SpiNorFlash& driver)
       .name_max = 50,
       .attr_max = 50,
     } {
+  mutex = xSemaphoreCreateRecursiveMutex();
+  ASSERT(mutex != nullptr);
 }
 
 void FS::Init() {
+  Lock lock(*this);
 
   // try mount
   int err = lfs_mount(&lfs, &lfsConfig);
@@ -54,58 +58,72 @@ void FS::VerifyResource() {
 }
 
 int FS::FileOpen(lfs_file_t* file_p, const char* fileName, const int flags) {
+  Lock lock(*this);
   return lfs_file_open(&lfs, file_p, fileName, flags);
 }
 
 int FS::FileClose(lfs_file_t* file_p) {
+  Lock lock(*this);
   return lfs_file_close(&lfs, file_p);
 }
 
 int FS::FileRead(lfs_file_t* file_p, uint8_t* buff, uint32_t size) {
+  Lock lock(*this);
   return lfs_file_read(&lfs, file_p, buff, size);
 }
 
 int FS::FileWrite(lfs_file_t* file_p, const uint8_t* buff, uint32_t size) {
+  Lock lock(*this);
   return lfs_file_write(&lfs, file_p, buff, size);
 }
 
 int FS::FileSeek(lfs_file_t* file_p, uint32_t pos) {
+  Lock lock(*this);
   return lfs_file_seek(&lfs, file_p, pos, LFS_SEEK_SET);
 }
 
 int FS::FileDelete(const char* fileName) {
+  Lock lock(*this);
   return lfs_remove(&lfs, fileName);
 }
 
 int FS::DirOpen(const char* path, lfs_dir_t* lfs_dir) {
+  Lock lock(*this);
   return lfs_dir_open(&lfs, lfs_dir, path);
 }
 
 int FS::DirClose(lfs_dir_t* lfs_dir) {
+  Lock lock(*this);
   return lfs_dir_close(&lfs, lfs_dir);
 }
 
 int FS::DirRead(lfs_dir_t* dir, lfs_info* info) {
+  Lock lock(*this);
   return lfs_dir_read(&lfs, dir, info);
 }
 
 int FS::DirRewind(lfs_dir_t* dir) {
+  Lock lock(*this);
   return lfs_dir_rewind(&lfs, dir);
 }
 
 int FS::DirCreate(const char* path) {
+  Lock lock(*this);
   return lfs_mkdir(&lfs, path);
 }
 
 int FS::Rename(const char* oldPath, const char* newPath) {
+  Lock lock(*this);
   return lfs_rename(&lfs, oldPath, newPath);
 }
 
 int FS::Stat(const char* path, lfs_info* info) {
+  Lock lock(*this);
   return lfs_stat(&lfs, path, info);
 }
 
 lfs_ssize_t FS::GetFSSize() {
+  Lock lock(*this);
   return lfs_fs_size(&lfs);
 }
 

src/components/fs/FS.h

@@ -3,13 +3,40 @@
 #include <cstdint>
 #include "drivers/SpiNorFlash.h"
 #include <littlefs/lfs.h>
+#include <FreeRTOS.h>
+#include <semphr.h>
 
 namespace Pinetime {
   namespace Controllers {
     class FS {
     public:
       FS(Pinetime::Drivers::SpiNorFlash&);
 
+      // Serializes littlefs access across tasks (littlefs itself is not
+      // thread-safe and the lfs_t state is shared). Every public FS method
+      // takes it, so single calls need nothing; hold a Lock across a
+      // multi-call sequence whose intermediate state must not be observed
+      // torn - in particular any open-file handle that a concurrent Rename
+      // or Delete of the same file would invalidate.
+      class Lock {
+      public:
+        explicit Lock(FS& fs) : fs {fs} {
+          xSemaphoreTakeRecursive(fs.mutex, portMAX_DELAY);
+        }
+
+        ~Lock() {
+          xSemaphoreGiveRecursive(fs.mutex);
+        }
+
+        Lock(const Lock&) = delete;
+        Lock& operator=(const Lock&) = delete;
+        Lock(Lock&&) = delete;
+        Lock& operator=(Lock&&) = delete;
+
+      private:
+        FS& fs;
+      };
+
       void Init();
 
       int FileOpen(lfs_file_t* file_p, const char* fileName, const int flags);
@@ -41,6 +68,7 @@ namespace Pinetime {
 
     private:
       Pinetime::Drivers::SpiNorFlash& flashDriver;
+      SemaphoreHandle_t mutex = nullptr;
 
       /*
        * External Flash MAP (4 MBytes)