From b93cd18bf5c637c488f7909ac75cd949ebc3deed Mon Sep 17 00:00:00 2001
From: Christopher Haster <geky@geky.net>
Date: Tue, 16 Jan 2024 13:13:49 -0600
Subject: [PATCH] Tighten bound on mlist isopen asserts

Unbalanced open/close calls continue to be a pain point for users, it
doesn't help that this sometimes results in hard-to-debug infinite loops
caused by the open-file linked-list (the mlist) getting tangled up in
itself.

Moving the mlist isopen asserts lower into the actual list append/remove
functions will help:

1. Make sure coverage of potential linked-list issues is complete.

2. Also assert against multiple close calls, which isn't an issue for
   the mlist, but can result in double free and memory corruption.
---
 lfs.c | 16 +++-------------
 1 file changed, 3 insertions(+), 13 deletions(-)

diff --git a/lfs.c b/lfs.c
index a152687f..7c8d6364 100644
--- a/lfs.c
+++ b/lfs.c
@@ -505,6 +505,7 @@ static bool lfs_mlist_isopen(struct lfs_mlist *head,
 #endif
 
 static void lfs_mlist_remove(lfs_t *lfs, struct lfs_mlist *mlist) {
+    LFS_ASSERT(lfs_mlist_isopen(lfs->mlist, mlist));
     for (struct lfs_mlist **p = &lfs->mlist; *p; p = &(*p)->next) {
         if (*p == mlist) {
             *p = (*p)->next;
@@ -514,6 +515,7 @@ static void lfs_mlist_remove(lfs_t *lfs, struct lfs_mlist *mlist) {
 }
 
 static void lfs_mlist_append(lfs_t *lfs, struct lfs_mlist *mlist) {
+    LFS_ASSERT(!lfs_mlist_isopen(lfs->mlist, mlist));
     mlist->next = lfs->mlist;
     lfs->mlist = mlist;
 }
@@ -3032,8 +3034,7 @@ static int lfs_file_rawopencfg(lfs_t *lfs, lfs_file_t *file,
     // allocate entry for file if it doesn't exist
     lfs_stag_t tag = lfs_dir_find(lfs, &file->m, &path, &file->id);
     if (tag < 0 && !(tag == LFS_ERR_NOENT && file->id != 0x3ff)) {
-        err = tag;
-        goto cleanup;
+        return tag;
     }
 
     // get id, add to list of mdirs to catch update changes
@@ -5929,7 +5930,6 @@ int lfs_file_open(lfs_t *lfs, lfs_file_t *file, const char *path, int flags) {
     }
     LFS_TRACE("lfs_file_open(%p, %p, \"%s\", %x)",
             (void*)lfs, (void*)file, path, flags);
-    LFS_ASSERT(!lfs_mlist_isopen(lfs->mlist, (struct lfs_mlist*)file));
 
     err = lfs_file_rawopen(lfs, file, path, flags);
 
@@ -5950,7 +5950,6 @@ int lfs_file_opencfg(lfs_t *lfs, lfs_file_t *file,
                  ".buffer=%p, .attrs=%p, .attr_count=%"PRIu32"})",
             (void*)lfs, (void*)file, path, flags,
             (void*)cfg, cfg->buffer, (void*)cfg->attrs, cfg->attr_count);
-    LFS_ASSERT(!lfs_mlist_isopen(lfs->mlist, (struct lfs_mlist*)file));
 
     err = lfs_file_rawopencfg(lfs, file, path, flags, cfg);
 
@@ -5965,7 +5964,6 @@ int lfs_file_close(lfs_t *lfs, lfs_file_t *file) {
         return err;
     }
     LFS_TRACE("lfs_file_close(%p, %p)", (void*)lfs, (void*)file);
-    LFS_ASSERT(lfs_mlist_isopen(lfs->mlist, (struct lfs_mlist*)file));
 
     err = lfs_file_rawclose(lfs, file);
 
@@ -5981,7 +5979,6 @@ int lfs_file_sync(lfs_t *lfs, lfs_file_t *file) {
         return err;
     }
     LFS_TRACE("lfs_file_sync(%p, %p)", (void*)lfs, (void*)file);
-    LFS_ASSERT(lfs_mlist_isopen(lfs->mlist, (struct lfs_mlist*)file));
 
     err = lfs_file_rawsync(lfs, file);
 
@@ -5999,7 +5996,6 @@ lfs_ssize_t lfs_file_read(lfs_t *lfs, lfs_file_t *file,
     }
     LFS_TRACE("lfs_file_read(%p, %p, %p, %"PRIu32")",
             (void*)lfs, (void*)file, buffer, size);
-    LFS_ASSERT(lfs_mlist_isopen(lfs->mlist, (struct lfs_mlist*)file));
 
     lfs_ssize_t res = lfs_file_rawread(lfs, file, buffer, size);
 
@@ -6017,7 +6013,6 @@ lfs_ssize_t lfs_file_write(lfs_t *lfs, lfs_file_t *file,
     }
     LFS_TRACE("lfs_file_write(%p, %p, %p, %"PRIu32")",
             (void*)lfs, (void*)file, buffer, size);
-    LFS_ASSERT(lfs_mlist_isopen(lfs->mlist, (struct lfs_mlist*)file));
 
     lfs_ssize_t res = lfs_file_rawwrite(lfs, file, buffer, size);
 
@@ -6035,7 +6030,6 @@ lfs_soff_t lfs_file_seek(lfs_t *lfs, lfs_file_t *file,
     }
     LFS_TRACE("lfs_file_seek(%p, %p, %"PRId32", %d)",
             (void*)lfs, (void*)file, off, whence);
-    LFS_ASSERT(lfs_mlist_isopen(lfs->mlist, (struct lfs_mlist*)file));
 
     lfs_soff_t res = lfs_file_rawseek(lfs, file, off, whence);
 
@@ -6052,7 +6046,6 @@ int lfs_file_truncate(lfs_t *lfs, lfs_file_t *file, lfs_off_t size) {
     }
     LFS_TRACE("lfs_file_truncate(%p, %p, %"PRIu32")",
             (void*)lfs, (void*)file, size);
-    LFS_ASSERT(lfs_mlist_isopen(lfs->mlist, (struct lfs_mlist*)file));
 
     err = lfs_file_rawtruncate(lfs, file, size);
 
@@ -6068,7 +6061,6 @@ lfs_soff_t lfs_file_tell(lfs_t *lfs, lfs_file_t *file) {
         return err;
     }
     LFS_TRACE("lfs_file_tell(%p, %p)", (void*)lfs, (void*)file);
-    LFS_ASSERT(lfs_mlist_isopen(lfs->mlist, (struct lfs_mlist*)file));
 
     lfs_soff_t res = lfs_file_rawtell(lfs, file);
 
@@ -6097,7 +6089,6 @@ lfs_soff_t lfs_file_size(lfs_t *lfs, lfs_file_t *file) {
         return err;
     }
     LFS_TRACE("lfs_file_size(%p, %p)", (void*)lfs, (void*)file);
-    LFS_ASSERT(lfs_mlist_isopen(lfs->mlist, (struct lfs_mlist*)file));
 
     lfs_soff_t res = lfs_file_rawsize(lfs, file);
 
@@ -6128,7 +6119,6 @@ int lfs_dir_open(lfs_t *lfs, lfs_dir_t *dir, const char *path) {
         return err;
     }
     LFS_TRACE("lfs_dir_open(%p, %p, \"%s\")", (void*)lfs, (void*)dir, path);
-    LFS_ASSERT(!lfs_mlist_isopen(lfs->mlist, (struct lfs_mlist*)dir));
 
     err = lfs_dir_rawopen(lfs, dir, path);