Github/littlefs
1
Fork 0
Code Actions Activity

Fixed writes bypassing/not updating holes in tracked file leaves

Browse Source 
Turns out we were using a slightly wrong condition for when to discard
file leaves in lfs3_file_flush_. An unsurprising mistake given size vs
weight subtleties. As a result, it was possible for a write to bypass
the leaf, leaving it with an outdated weight, resulting in an unexpected
hole in the file.

This was surprisingly hard to find as most writes don't leave the leaf
with hole information, only reads.

Fortunately a solution is easy. Just don't use the bptr size here,
instead use the full leaf weight to decide when to discard tracked file
leaves.

Code changes humorously canceling out the Valgrind fix:

           code          stack          ctx
  before: 35260           2136          660
  after:  35256 (-0.0%)   2136 (+0.0%)  660 (+0.0%)

---

This was found by test_fsync_rwtfrwtf_sparse_fuzz, but only by luck
after the CRYSTAL_THRESH/8 -> CRYSTAL_THRESH/16 tweak.

To prevent a regression, and hopefully catch other bugs like this
(something something cache coherency hard problem), I added a couple
"clip" tests that try to force the cache/leaf bypassing behavior:

- test_fwrite_clip_cache - try clipping the file cache
- test_fwrite_clip_leaf - try clipping the file leaf
- test_fwrite_clip_hole - try clipping the file leaf+hole

test_fwrite_clip_hole does reproduce the bug.
This commit is contained in:
Christopher Haster
2026-02-04 17:55:18 -06:00
parent 7397605517
commit 26e8bc2e9e
2 changed files with 498 additions and 1 deletions
Download Patch File Download Diff File Expand all files Collapse all files
tests/test_fwrite.toml
+497
View File
@@ -1198,6 +1198,503 @@ code = '''
lfs3_unmount(&lfs3) => 0;
'''
# test some annoying corner cases with overwriting file caches/leaves
[cases.test_fwrite_clip_cache]
defines.SIZE = '4*BLOCK_SIZE'
# MASK&0x1 => clip before
# MASK&0x2 => clip after
defines.MASK = [0, 1, 2, 3]
defines.CLIP = [
# large enough to bypass the cache
'BLOCK_SIZE',
# large enough to bypass the cache, but not steal the file leaf!
'FCACHE_SIZE + 1',
]
if = [
# this just saves testing time
'SIZE <= 4*1024*FRAGMENT_SIZE',
]
in = 'lfs3.c'
code = '''
lfs3_t lfs3;
lfs3_format(&lfs3, LFS3_F_RDWR, CFG) => 0;
lfs3_mount(&lfs3, LFS3_M_RDWR, CFG) => 0;
// create a file
lfs3_file_t file;
lfs3_file_open(&lfs3, &file, "hello",
LFS3_O_RDWR | LFS3_O_CREAT | LFS3_O_EXCL) => 0;
uint8_t wbuf[SIZE];
uint32_t prng = 42;
for (lfs3_size_t i = 0; i < SIZE; i++) {
wbuf[i] = 'a' + (TEST_PRNG(&prng) % 26);
}
lfs3_file_write(&lfs3, &file, wbuf, SIZE) => SIZE;
// simulate our file in ram
uint8_t sim[SIZE];
memcpy(sim, wbuf, SIZE);
// write a bit to fill the cache
lfs3_off_t off = BLOCK_SIZE;
lfs3_off_t size = FCACHE_SIZE/2;
lfs3_file_seek(&lfs3, &file, off, LFS3_SEEK_SET) => off;
for (lfs3_size_t i = 0; i < size; i++) {
wbuf[i] = 'a' + (TEST_PRNG(&prng) % 26);
}
lfs3_file_write(&lfs3, &file, wbuf, size) => size;
memcpy(sim + off, wbuf, size);
// cache filled?
assert(file.cache.pos == BLOCK_SIZE);
assert(file.cache.size == FCACHE_SIZE/2);
// write a bit before, clipping our cache
if (MASK & 0x1) {
lfs3_off_t off = BLOCK_SIZE - CLIP + 1;
lfs3_off_t size = CLIP;
lfs3_file_seek(&lfs3, &file, off, LFS3_SEEK_SET) => off;
for (lfs3_size_t i = 0; i < size; i++) {
wbuf[i] = 'a' + (TEST_PRNG(&prng) % 26);
}
lfs3_file_write(&lfs3, &file, wbuf, size) => size;
memcpy(sim + off, wbuf, size);
}
// write a bit after, clipping our cache
if (MASK & 0x2) {
lfs3_off_t off = BLOCK_SIZE + (FCACHE_SIZE/2) - 1;
lfs3_off_t size = CLIP;
lfs3_file_seek(&lfs3, &file, off, LFS3_SEEK_SET) => off;
for (lfs3_size_t i = 0; i < size; i++) {
wbuf[i] = 'a' + (TEST_PRNG(&prng) % 26);
}
lfs3_file_write(&lfs3, &file, wbuf, size) => size;
memcpy(sim + off, wbuf, size);
}
// before we close! try reading our file
//
// this has the highest chance of reading something wrong
uint8_t rbuf[2*SIZE];
memset(rbuf, 0xaa, 2*SIZE);
// first try reading _in_ our cache (which may have been discarded)
off = BLOCK_SIZE + 1;
size = FCACHE_SIZE/2 - 2;
lfs3_file_seek(&lfs3, &file, off, LFS3_SEEK_SET) => off;
lfs3_file_read(&lfs3, &file, rbuf, size) => size;
// does our file match our simulation?
assert(memcmp(rbuf, sim + off, size) == 0);
// then try reading the full cache (which may have been discarded)
off = BLOCK_SIZE;
size = FCACHE_SIZE/2;
lfs3_file_seek(&lfs3, &file, off, LFS3_SEEK_SET) => off;
lfs3_file_read(&lfs3, &file, rbuf, size) => size;
// does our file match our simulation?
assert(memcmp(rbuf, sim + off, size) == 0);
// then try reading the full file
lfs3_file_seek(&lfs3, &file, 0, LFS3_SEEK_SET) => 0;
lfs3_file_read(&lfs3, &file, rbuf, 2*SIZE) => SIZE;
// does our file match our simulation?
assert(memcmp(rbuf, sim, SIZE) == 0);
lfs3_file_close(&lfs3, &file) => 0;
// try reading after closing
for (int remount = 0; remount < 2; remount++) {
// remount?
if (remount) {
lfs3_unmount(&lfs3) => 0;
lfs3_mount(&lfs3, LFS3_M_RDWR, CFG) => 0;
}
// check our file with stat
struct lfs3_info info;
lfs3_stat(&lfs3, "hello", &info) => 0;
assert(strcmp(info.name, "hello") == 0);
assert(info.type == LFS3_TYPE_REG);
assert(info.size == SIZE);
// and with dir read
lfs3_dir_t dir;
lfs3_dir_open(&lfs3, &dir, "/") => 0;
lfs3_dir_read(&lfs3, &dir, &info) => 0;
assert(strcmp(info.name, ".") == 0);
assert(info.type == LFS3_TYPE_DIR);
assert(info.size == 0);
lfs3_dir_read(&lfs3, &dir, &info) => 0;
assert(strcmp(info.name, "..") == 0);
assert(info.type == LFS3_TYPE_DIR);
assert(info.size == 0);
lfs3_dir_read(&lfs3, &dir, &info) => 0;
assert(strcmp(info.name, "hello") == 0);
assert(info.type == LFS3_TYPE_REG);
assert(info.size == SIZE);
lfs3_dir_read(&lfs3, &dir, &info) => LFS3_ERR_NOENT;
lfs3_dir_close(&lfs3, &dir) => 0;
// try reading our file
lfs3_file_open(&lfs3, &file, "hello", LFS3_O_RDONLY) => 0;
// is size correct?
lfs3_file_size(&lfs3, &file) => SIZE;
// try reading
uint8_t rbuf[2*SIZE];
memset(rbuf, 0xaa, 2*SIZE);
lfs3_file_read(&lfs3, &file, rbuf, 2*SIZE) => SIZE;
// does our file match our simulation?
assert(memcmp(rbuf, sim, SIZE) == 0);
lfs3_file_close(&lfs3, &file) => 0;
}
lfs3_unmount(&lfs3) => 0;
'''
[cases.test_fwrite_clip_leaf]
defines.SIZE = '4*BLOCK_SIZE'
# MASK&0x1 => clip before
# MASK&0x2 => clip after
defines.MASK = [0, 1, 2, 3]
defines.CLIP = [
# large enough to bypass the cache
'BLOCK_SIZE',
# large enough to bypass the cache, but not steal the file leaf!
'FCACHE_SIZE + 1',
]
if = [
# this just saves testing time
'SIZE <= 4*1024*FRAGMENT_SIZE',
]
in = 'lfs3.c'
code = '''
lfs3_t lfs3;
lfs3_format(&lfs3, LFS3_F_RDWR, CFG) => 0;
lfs3_mount(&lfs3, LFS3_M_RDWR, CFG) => 0;
// create a file
lfs3_file_t file;
lfs3_file_open(&lfs3, &file, "hello",
LFS3_O_RDWR | LFS3_O_CREAT | LFS3_O_EXCL) => 0;
uint8_t wbuf[SIZE];
uint32_t prng = 42;
for (lfs3_size_t i = 0; i < SIZE; i++) {
wbuf[i] = 'a' + (TEST_PRNG(&prng) % 26);
}
lfs3_file_write(&lfs3, &file, wbuf, SIZE) => SIZE;
// simulate our file in ram
uint8_t sim[SIZE];
memcpy(sim, wbuf, SIZE);
// write a bit to crystallize a leaf
lfs3_off_t off = BLOCK_SIZE;
lfs3_off_t size = BLOCK_SIZE/2;
lfs3_file_seek(&lfs3, &file, off, LFS3_SEEK_SET) => off;
for (lfs3_size_t i = 0; i < size; i++) {
wbuf[i] = 'a' + (TEST_PRNG(&prng) % 26);
}
lfs3_file_write(&lfs3, &file, wbuf, size) => size;
memcpy(sim + off, wbuf, size);
// do we have a tracked leaf now?
assert(file.leaf.pos == BLOCK_SIZE);
assert(lfs3_bptr_size(&file.leaf.bptr) == BLOCK_SIZE/2);
assert(file.leaf.weight == BLOCK_SIZE/2);
// write a bit before, clipping our leaf
if (MASK & 0x1) {
lfs3_off_t off = BLOCK_SIZE - CLIP + 1;
lfs3_off_t size = CLIP;
lfs3_file_seek(&lfs3, &file, off, LFS3_SEEK_SET) => off;
for (lfs3_size_t i = 0; i < size; i++) {
wbuf[i] = 'a' + (TEST_PRNG(&prng) % 26);
}
lfs3_file_write(&lfs3, &file, wbuf, size) => size;
memcpy(sim + off, wbuf, size);
}
// write a bit after, clipping our leaf
if (MASK & 0x2) {
lfs3_off_t off = BLOCK_SIZE + (BLOCK_SIZE/2) - 1;
lfs3_off_t size = CLIP;
lfs3_file_seek(&lfs3, &file, off, LFS3_SEEK_SET) => off;
for (lfs3_size_t i = 0; i < size; i++) {
wbuf[i] = 'a' + (TEST_PRNG(&prng) % 26);
}
lfs3_file_write(&lfs3, &file, wbuf, size) => size;
memcpy(sim + off, wbuf, size);
}
// before we close! try reading our file
//
// this has the highest chance of reading something wrong
uint8_t rbuf[2*SIZE];
memset(rbuf, 0xaa, 2*SIZE);
// first try reading _in_ our leaf (which may have been discarded)
off = BLOCK_SIZE + 1;
size = BLOCK_SIZE/2 - 2;
lfs3_file_seek(&lfs3, &file, off, LFS3_SEEK_SET) => off;
lfs3_file_read(&lfs3, &file, rbuf, size) => size;
// does our file match our simulation?
assert(memcmp(rbuf, sim + off, size) == 0);
// then try reading the full leaf (which may have been discarded)
off = BLOCK_SIZE;
size = BLOCK_SIZE/2;
lfs3_file_seek(&lfs3, &file, off, LFS3_SEEK_SET) => off;
lfs3_file_read(&lfs3, &file, rbuf, size) => size;
// does our file match our simulation?
assert(memcmp(rbuf, sim + off, size) == 0);
// then try reading the full file
lfs3_file_seek(&lfs3, &file, 0, LFS3_SEEK_SET) => 0;
lfs3_file_read(&lfs3, &file, rbuf, 2*SIZE) => SIZE;
// does our file match our simulation?
assert(memcmp(rbuf, sim, SIZE) == 0);
lfs3_file_close(&lfs3, &file) => 0;
// try reading after closing
for (int remount = 0; remount < 2; remount++) {
// remount?
if (remount) {
lfs3_unmount(&lfs3) => 0;
lfs3_mount(&lfs3, LFS3_M_RDWR, CFG) => 0;
}
// check our file with stat
struct lfs3_info info;
lfs3_stat(&lfs3, "hello", &info) => 0;
assert(strcmp(info.name, "hello") == 0);
assert(info.type == LFS3_TYPE_REG);
assert(info.size == SIZE);
// and with dir read
lfs3_dir_t dir;
lfs3_dir_open(&lfs3, &dir, "/") => 0;
lfs3_dir_read(&lfs3, &dir, &info) => 0;
assert(strcmp(info.name, ".") == 0);
assert(info.type == LFS3_TYPE_DIR);
assert(info.size == 0);
lfs3_dir_read(&lfs3, &dir, &info) => 0;
assert(strcmp(info.name, "..") == 0);
assert(info.type == LFS3_TYPE_DIR);
assert(info.size == 0);
lfs3_dir_read(&lfs3, &dir, &info) => 0;
assert(strcmp(info.name, "hello") == 0);
assert(info.type == LFS3_TYPE_REG);
assert(info.size == SIZE);
lfs3_dir_read(&lfs3, &dir, &info) => LFS3_ERR_NOENT;
lfs3_dir_close(&lfs3, &dir) => 0;
// try reading our file
lfs3_file_open(&lfs3, &file, "hello", LFS3_O_RDONLY) => 0;
// is size correct?
lfs3_file_size(&lfs3, &file) => SIZE;
// try reading
uint8_t rbuf[2*SIZE];
memset(rbuf, 0xaa, 2*SIZE);
lfs3_file_read(&lfs3, &file, rbuf, 2*SIZE) => SIZE;
// does our file match our simulation?
assert(memcmp(rbuf, sim, SIZE) == 0);
lfs3_file_close(&lfs3, &file) => 0;
}
lfs3_unmount(&lfs3) => 0;
'''
[cases.test_fwrite_clip_hole]
defines.SIZE = '4*BLOCK_SIZE'
# MASK&0x1 => clip before
# MASK&0x2 => clip after
defines.MASK = [0, 1, 2, 3]
defines.CLIP = [
# large enough to bypass the cache
'BLOCK_SIZE',
# large enough to bypass the cache, but not steal the file leaf!
'FCACHE_SIZE + 1',
]
if = [
# this just saves testing time
'SIZE <= 4*1024*FRAGMENT_SIZE',
]
in = 'lfs3.c'
code = '''
lfs3_t lfs3;
lfs3_format(&lfs3, LFS3_F_RDWR, CFG) => 0;
lfs3_mount(&lfs3, LFS3_M_RDWR, CFG) => 0;
// create a _sparse_ file
lfs3_file_t file;
lfs3_file_open(&lfs3, &file, "hello",
LFS3_O_RDWR | LFS3_O_CREAT | LFS3_O_EXCL) => 0;
// simulate our file in ram
uint8_t sim[SIZE];
memset(sim, 0, SIZE);
// write some data, but only to the head and tail, we want a nice
// big hole in the middle
uint8_t wbuf[SIZE];
uint32_t prng = 42;
for (lfs3_size_t i = 0; i < BLOCK_SIZE; i++) {
wbuf[i] = 'a' + (TEST_PRNG(&prng) % 26);
}
lfs3_file_write(&lfs3, &file, wbuf, BLOCK_SIZE) => BLOCK_SIZE;
memcpy(sim, wbuf, BLOCK_SIZE);
lfs3_file_seek(&lfs3, &file, 3*BLOCK_SIZE, LFS3_SEEK_SET)
=> 3*BLOCK_SIZE;
for (lfs3_size_t i = 0; i < BLOCK_SIZE; i++) {
wbuf[i] = 'a' + (TEST_PRNG(&prng) % 26);
}
lfs3_file_write(&lfs3, &file, wbuf, BLOCK_SIZE) => BLOCK_SIZE;
memcpy(sim + 3*BLOCK_SIZE, wbuf, BLOCK_SIZE);
// write a bit to crystallize a leaf
lfs3_off_t off = BLOCK_SIZE;
lfs3_off_t size = BLOCK_SIZE/2;
lfs3_file_seek(&lfs3, &file, off, LFS3_SEEK_SET) => off;
for (lfs3_size_t i = 0; i < size; i++) {
wbuf[i] = 'a' + (TEST_PRNG(&prng) % 26);
}
lfs3_file_write(&lfs3, &file, wbuf, size) => size;
memcpy(sim + off, wbuf, size);
// read a full block somewhere else to force our leaf into the bshrub
lfs3_file_seek(&lfs3, &file, 0, LFS3_SEEK_SET) => 0;
uint8_t rbuf[2*SIZE];
lfs3_file_read(&lfs3, &file, rbuf, BLOCK_SIZE/2) => BLOCK_SIZE/2;
assert(memcmp(rbuf, sim, BLOCK_SIZE/2) == 0);
// and then read our leaf again, note we can't just call
// lfs3_file_flush because it wouldn't fetch the hole information
lfs3_file_seek(&lfs3, &file, BLOCK_SIZE, LFS3_SEEK_SET) => BLOCK_SIZE;
lfs3_file_read(&lfs3, &file, rbuf, BLOCK_SIZE/2) => BLOCK_SIZE/2;
assert(memcmp(rbuf, sim + BLOCK_SIZE, BLOCK_SIZE/2) == 0);
// do we have a tracked leaf now?
assert(file.leaf.pos == BLOCK_SIZE);
assert(lfs3_bptr_size(&file.leaf.bptr) == BLOCK_SIZE/2);
// note the big hole in our leaf!
assert(file.leaf.weight == 2*BLOCK_SIZE);
// write a bit before, clipping our leaf
if (MASK & 0x1) {
lfs3_off_t off = BLOCK_SIZE - CLIP + 1;
lfs3_off_t size = CLIP;
lfs3_file_seek(&lfs3, &file, off, LFS3_SEEK_SET) => off;
for (lfs3_size_t i = 0; i < size; i++) {
wbuf[i] = 'a' + (TEST_PRNG(&prng) % 26);
}
lfs3_file_write(&lfs3, &file, wbuf, size) => size;
memcpy(sim + off, wbuf, size);
}
// write a bit after, clipping our leaf
if (MASK & 0x2) {
lfs3_off_t off = BLOCK_SIZE + (2*BLOCK_SIZE) - 1;
lfs3_off_t size = CLIP;
lfs3_file_seek(&lfs3, &file, off, LFS3_SEEK_SET) => off;
for (lfs3_size_t i = 0; i < size; i++) {
wbuf[i] = 'a' + (TEST_PRNG(&prng) % 26);
}
lfs3_file_write(&lfs3, &file, wbuf, size) => size;
memcpy(sim + off, wbuf, size);
}
// before we close! try reading our file
//
// this has the highest chance of reading something wrong
memset(rbuf, 0xaa, 2*SIZE);
// first try reading _in_ our leaf (which may have been discarded)
off = BLOCK_SIZE + 1;
size = 2*BLOCK_SIZE - 2;
lfs3_file_seek(&lfs3, &file, off, LFS3_SEEK_SET) => off;
lfs3_file_read(&lfs3, &file, rbuf, size) => size;
// does our file match our simulation?
assert(memcmp(rbuf, sim + off, size) == 0);
// then try reading the full leaf (which may have been discarded)
off = BLOCK_SIZE;
size = 2*BLOCK_SIZE;
lfs3_file_seek(&lfs3, &file, off, LFS3_SEEK_SET) => off;
lfs3_file_read(&lfs3, &file, rbuf, size) => size;
// does our file match our simulation?
assert(memcmp(rbuf, sim + off, size) == 0);
// then try reading the full file
lfs3_file_seek(&lfs3, &file, 0, LFS3_SEEK_SET) => 0;
lfs3_file_read(&lfs3, &file, rbuf, 2*SIZE) => SIZE;
// does our file match our simulation?
assert(memcmp(rbuf, sim, SIZE) == 0);
lfs3_file_close(&lfs3, &file) => 0;
// try reading after closing
for (int remount = 0; remount < 2; remount++) {
// remount?
if (remount) {
lfs3_unmount(&lfs3) => 0;
lfs3_mount(&lfs3, LFS3_M_RDWR, CFG) => 0;
}
// check our file with stat
struct lfs3_info info;
lfs3_stat(&lfs3, "hello", &info) => 0;
assert(strcmp(info.name, "hello") == 0);
assert(info.type == LFS3_TYPE_REG);
assert(info.size == SIZE);
// and with dir read
lfs3_dir_t dir;
lfs3_dir_open(&lfs3, &dir, "/") => 0;
lfs3_dir_read(&lfs3, &dir, &info) => 0;
assert(strcmp(info.name, ".") == 0);
assert(info.type == LFS3_TYPE_DIR);
assert(info.size == 0);
lfs3_dir_read(&lfs3, &dir, &info) => 0;
assert(strcmp(info.name, "..") == 0);
assert(info.type == LFS3_TYPE_DIR);
assert(info.size == 0);
lfs3_dir_read(&lfs3, &dir, &info) => 0;
assert(strcmp(info.name, "hello") == 0);
assert(info.type == LFS3_TYPE_REG);
assert(info.size == SIZE);
lfs3_dir_read(&lfs3, &dir, &info) => LFS3_ERR_NOENT;
lfs3_dir_close(&lfs3, &dir) => 0;
// try reading our file
lfs3_file_open(&lfs3, &file, "hello", LFS3_O_RDONLY) => 0;
// is size correct?
lfs3_file_size(&lfs3, &file) => SIZE;
// try reading
uint8_t rbuf[2*SIZE];
memset(rbuf, 0xaa, 2*SIZE);
lfs3_file_read(&lfs3, &file, rbuf, 2*SIZE) => SIZE;
// does our file match our simulation?
assert(memcmp(rbuf, sim, SIZE) == 0);
lfs3_file_close(&lfs3, &file) => 0;
}
lfs3_unmount(&lfs3) => 0;
'''
# simple truncate test
[cases.test_fwrite_truncate]
defines.FROM = [