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.
This commit is contained in:
Christopher Haster
2024-01-16 13:13:49 -06:00
parent 3513ff1afc
commit b93cd18bf5
+3 -13
View File
@@ -505,6 +505,7 @@ static bool lfs_mlist_isopen(struct lfs_mlist *head,
#endif #endif
static void lfs_mlist_remove(lfs_t *lfs, struct lfs_mlist *mlist) { 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) { for (struct lfs_mlist **p = &lfs->mlist; *p; p = &(*p)->next) {
if (*p == mlist) { if (*p == mlist) {
*p = (*p)->next; *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) { static void lfs_mlist_append(lfs_t *lfs, struct lfs_mlist *mlist) {
LFS_ASSERT(!lfs_mlist_isopen(lfs->mlist, mlist));
mlist->next = lfs->mlist; mlist->next = lfs->mlist;
lfs->mlist = 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 // allocate entry for file if it doesn't exist
lfs_stag_t tag = lfs_dir_find(lfs, &file->m, &path, &file->id); lfs_stag_t tag = lfs_dir_find(lfs, &file->m, &path, &file->id);
if (tag < 0 && !(tag == LFS_ERR_NOENT && file->id != 0x3ff)) { if (tag < 0 && !(tag == LFS_ERR_NOENT && file->id != 0x3ff)) {
err = tag; return tag;
goto cleanup;
} }
// get id, add to list of mdirs to catch update changes // 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)", LFS_TRACE("lfs_file_open(%p, %p, \"%s\", %x)",
(void*)lfs, (void*)file, path, flags); (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); 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"})", ".buffer=%p, .attrs=%p, .attr_count=%"PRIu32"})",
(void*)lfs, (void*)file, path, flags, (void*)lfs, (void*)file, path, flags,
(void*)cfg, cfg->buffer, (void*)cfg->attrs, cfg->attr_count); (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); 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; return err;
} }
LFS_TRACE("lfs_file_close(%p, %p)", (void*)lfs, (void*)file); 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); err = lfs_file_rawclose(lfs, file);
@@ -5981,7 +5979,6 @@ int lfs_file_sync(lfs_t *lfs, lfs_file_t *file) {
return err; return err;
} }
LFS_TRACE("lfs_file_sync(%p, %p)", (void*)lfs, (void*)file); 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); 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")", LFS_TRACE("lfs_file_read(%p, %p, %p, %"PRIu32")",
(void*)lfs, (void*)file, buffer, size); (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); 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")", LFS_TRACE("lfs_file_write(%p, %p, %p, %"PRIu32")",
(void*)lfs, (void*)file, buffer, size); (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); 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)", LFS_TRACE("lfs_file_seek(%p, %p, %"PRId32", %d)",
(void*)lfs, (void*)file, off, whence); (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); 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")", LFS_TRACE("lfs_file_truncate(%p, %p, %"PRIu32")",
(void*)lfs, (void*)file, size); (void*)lfs, (void*)file, size);
LFS_ASSERT(lfs_mlist_isopen(lfs->mlist, (struct lfs_mlist*)file));
err = lfs_file_rawtruncate(lfs, file, size); 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; return err;
} }
LFS_TRACE("lfs_file_tell(%p, %p)", (void*)lfs, (void*)file); 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); 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; return err;
} }
LFS_TRACE("lfs_file_size(%p, %p)", (void*)lfs, (void*)file); 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); 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; return err;
} }
LFS_TRACE("lfs_dir_open(%p, %p, \"%s\")", (void*)lfs, (void*)dir, path); 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); err = lfs_dir_rawopen(lfs, dir, path);