Skip to content

Commit

Permalink
Tighten bound on mlist isopen asserts
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
geky committed Jan 16, 2024
1 parent 3513ff1 commit b93cd18
Showing 1 changed file with 3 additions and 13 deletions.
16 changes: 3 additions & 13 deletions lfs.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
}
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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);

Expand All @@ -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);

Expand All @@ -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);

Expand All @@ -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);

Expand All @@ -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);

Expand All @@ -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);

Expand All @@ -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);

Expand All @@ -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);

Expand All @@ -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);

Expand Down Expand Up @@ -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);

Expand Down Expand Up @@ -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);

Expand Down

0 comments on commit b93cd18

Please sign in to comment.