mirror/littlefs
3
0
Fork 0
mirror of https://github.com/littlefs-project/littlefs.git synced 2025-10-29 11:38:32 +00:00
Code Issues Packages Projects Releases Wiki Activity

Fixed dir iteration being broken by concurrent removes

Browse Source 
When removing a file, we mark all open handles as "removed" (
pair={-1,-1}) to avoid trying to later read metadata that no longer
exists. Unfortunately, this also includes open dir handles that happen
to be pointing at the removed file, causing them to return
LFS_ERR_CORRUPT on the next read.

The good news is this is _not_ actual filesystem corruption, only a
logic error in lfs_dir_read.

We actually already have logic in place to nudge the dir to the next id,
but it was unreachable with the existing logic. I suspect this worked at
one point but was broken during a refactor due to lack of testing.

---

Fortunately, all we need to do is _not_ clobber the handle if the
internal type is a dir. Then the dir-nudging logic can correctly take
over.

I've also added test_dirs_remove_read to test this and prevent another
regression, adapted from tests provided by tpwrules that identified the
original bug.

Found by tpwrules
This commit is contained in:
Christopher Haster 2025-02-03 18:05:15 -06:00
parent 0494ce7169
commit caba4f31df
2 changed files with 78 additions and 1 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
lfs.c
View File

@ -2369,7 +2369,8 @@ fixmlist:;
if (d->m.pair != pair) {
for (int i = 0; i < attrcount; i++) {
if (lfs_tag_type3(attrs[i].tag) == LFS_TYPE_DELETE &&
d->id == lfs_tag_id(attrs[i].tag)) {
d->id == lfs_tag_id(attrs[i].tag) &&
d->type != LFS_TYPE_DIR) {
d->m.pair[0] = LFS_BLOCK_NULL;
d->m.pair[1] = LFS_BLOCK_NULL;
} else if (lfs_tag_type3(attrs[i].tag) == LFS_TYPE_DELETE &&

76
tests/test_dirs.toml
View File

@ -725,6 +725,82 @@ code = '''
lfs_unmount(&lfs) => 0;
'''
[cases.test_dirs_remove_read]
defines.N = 10
if = 'N < BLOCK_COUNT/2'
code = '''
lfs_t lfs;
lfs_format(&lfs, cfg) => 0;
lfs_mount(&lfs, cfg) => 0;
lfs_mkdir(&lfs, "prickly-pear") => 0;
for (int i = 0; i < N; i++) {
char path[1024];
sprintf(path, "prickly-pear/cactus%03d", i);
lfs_mkdir(&lfs, path) => 0;
}
lfs_dir_t dir;
lfs_dir_open(&lfs, &dir, "prickly-pear") => 0;
struct lfs_info info;
lfs_dir_read(&lfs, &dir, &info) => 1;
assert(info.type == LFS_TYPE_DIR);
assert(strcmp(info.name, ".") == 0);
lfs_dir_read(&lfs, &dir, &info) => 1;
assert(info.type == LFS_TYPE_DIR);
assert(strcmp(info.name, "..") == 0);
for (int i = 0; i < N; i++) {
char path[1024];
sprintf(path, "cactus%03d", i);
lfs_dir_read(&lfs, &dir, &info) => 1;
assert(info.type == LFS_TYPE_DIR);
assert(strcmp(info.name, path) == 0);
}
lfs_dir_read(&lfs, &dir, &info) => 0;
lfs_dir_close(&lfs, &dir) => 0;
lfs_unmount(&lfs);
for (lfs_size_t k = 0; k < N; k++) {
for (lfs_size_t j = 0; j < N; j++) {
lfs_mount(&lfs, cfg) => 0;
lfs_dir_open(&lfs, &dir, "prickly-pear") => 0;
lfs_dir_read(&lfs, &dir, &info) => 1;
assert(info.type == LFS_TYPE_DIR);
assert(strcmp(info.name, ".") == 0);
lfs_dir_read(&lfs, &dir, &info) => 1;
assert(info.type == LFS_TYPE_DIR);
assert(strcmp(info.name, "..") == 0);
// iterate over dirs < j
for (unsigned i = 0; i < j; i++) {
char path[1024];
sprintf(path, "cactus%03d", i);
lfs_dir_read(&lfs, &dir, &info) => 1;
assert(info.type == LFS_TYPE_DIR);
assert(strcmp(info.name, path) == 0);
}
// remove k while iterating
char path[1024];
sprintf(path, "prickly-pear/cactus%03d", k);
lfs_remove(&lfs, path) => 0;
// iterate over dirs >= j
for (unsigned i = j; i < ((k >= j) ? N-1 : N); i++) {
char path[1024];
sprintf(path, "cactus%03d", (k >= j && i >= k) ? i+1 : i);
lfs_dir_read(&lfs, &dir, &info) => 1;
assert(info.type == LFS_TYPE_DIR);
assert(strcmp(info.name, path) == 0);
}
lfs_dir_read(&lfs, &dir, &info) => 0;
lfs_dir_close(&lfs, &dir) => 0;
// recreate k
sprintf(path, "prickly-pear/cactus%03d", k);
lfs_mkdir(&lfs, path) => 0;
lfs_unmount(&lfs) => 0;
}
}
'''
[cases.test_dirs_other_errors]
code = '''
lfs_t lfs;