Github/littlefs
1
Fork 0
Code Actions Activity

Fixed test_ck_spam*'s open file bshrub/btree issues

Browse Source 
- In lfsr_mtree_traverse, we traverse open file bshrubs/btrees before
  we validate the gcksum, which means bugs/asserts can slip through
  before we have a chance to detect something is wrong.

  To work around this, I've added an explicit mdir cksum check right
  before we start traversing an open mdir's bshrubs/btrees. If an open
  mdir doesn't match the on-disk state, the on-disk state must contain
  an error (or the RAM, but that's a different story and wayyy out of
  scope).

  It might be better to rearrange lfsr_mtree_traverse to check gcksums
  first, but this will require another look at our traversal clobbering
  logic.

- For a similar reason, ckfetches can't detect open bshrub/btree
  corruption as is. As its name suggests, ckfetches only checks fetches,
  so any corruption after we've fetched bshrubs/btrees in lfsr_file_open
  will go undetected.

  Fortunately this just means we need a full ckmeta-scan in
  test_ck_spam* tests that keep open files.

  In real use, full ckmeta-scans should be preferred anyways. Limiting
  these scans to mtreeonly was just an attempt to better stress btree
  ckfetches.

  At least we're still testing ckmeta+mtreeonly+ckfetches in
  test_ck_spam_dir_fuzz and test_ck_spam_file_fuzz.

This gets the test_ck_spam* tests running under all of the current
interesting ck-modes.

Code changes:

           code          stack          ctx
  before: 38560           2640          644
  after   38572 (+0.0%)   2640 (+0.0%)  644 (+0.0%)
This commit is contained in:
Christopher Haster
2025-01-16 14:08:46 -06:00
parent cae8b08dc9
commit 47438a8c46
2 changed files with 59 additions and 23 deletions
Download Patch File Download Diff File Expand all files Collapse all files
lfs.c
+14
View File
@@ -9477,6 +9477,20 @@ static int lfsr_mtree_traverse_(lfs_t *lfs, lfsr_traversal_t *t,
continue;
}
// hold on, does our mdir cksum match? if not we found some
// sort of error
if (t->ot->mdir.rbyd.cksum != t->o.o.mdir.rbyd.cksum) {
LFS_DEBUG("Found mdir cksum mismatch %"PRId32" "
"0x{%"PRIx32",%"PRIx32"}, "
"cksum %08"PRIx32" (!= %08"PRIx32")",
t->o.o.mdir.mid >> lfs->mdir_bits,
t->o.o.mdir.rbyd.blocks[0],
t->o.o.mdir.rbyd.blocks[1],
t->o.o.mdir.rbyd.cksum,
t->ot->mdir.rbyd.cksum);
return LFS_ERR_CORRUPT;
}
// start traversing the file
const lfsr_file_t *file = (const lfsr_file_t*)t->ot;
t->o.bshrub = file->o.bshrub;
tests/test_ck.toml
+45 -23
View File
@@ -2451,6 +2451,9 @@ defines.BADBLOCK_BEHAVIOR = '''
? LFS_EMUBD_BADBLOCK_PROGFLIP
: LFS_EMUBD_BADBLOCK_MANUAL
'''
defines.CKMETA = 'METHOD == 2 || METHOD == 3'
defines.CKDATA = 'METHOD == 1'
defines.MTREEONLY = 'METHOD == 2'
defines.CKPROGS = 'METHOD == 0'
defines.CKFETCHES = 'METHOD == 2'
defines.CKPARITY = false
@@ -2526,21 +2529,21 @@ code = '''
}
// run ckdata?
if (METHOD == 1) {
if (CKDATA) {
int err = lfsr_fs_ckdata(&lfs);
assert(!err || err == LFS_ERR_CORRUPT);
if (err == LFS_ERR_CORRUPT) {
goto corrupt_mounted;
}
// run ckmeta?
} else if (METHOD == 3) {
} else if (CKMETA && !MTREEONLY) {
int err = lfsr_fs_ckmeta(&lfs);
assert(!err || err == LFS_ERR_CORRUPT);
if (err == LFS_ERR_CORRUPT) {
goto corrupt_mounted;
}
// run ckmeta mtreeonly?
} else if (METHOD == 2) {
} else if (CKMETA && MTREEONLY) {
// need an explicit traversal for this
lfsr_traversal_t t;
lfsr_traversal_open(&lfs, &t,
@@ -2749,6 +2752,9 @@ defines.BADBLOCK_BEHAVIOR = '''
? LFS_EMUBD_BADBLOCK_PROGFLIP
: LFS_EMUBD_BADBLOCK_MANUAL
'''
defines.CKMETA = 'METHOD == 2 || METHOD == 3'
defines.CKDATA = 'METHOD == 1'
defines.MTREEONLY = 'METHOD == 2'
defines.CKPROGS = 'METHOD == 0'
defines.CKFETCHES = 'METHOD == 2'
defines.CKPARITY = false
@@ -2835,21 +2841,21 @@ code = '''
}
// run ckdata?
if (METHOD == 1) {
if (CKDATA) {
int err = lfsr_fs_ckdata(&lfs);
assert(!err || err == LFS_ERR_CORRUPT);
if (err == LFS_ERR_CORRUPT) {
goto corrupt_mounted;
}
// run ckmeta?
} else if (METHOD == 3) {
} else if (CKMETA && !MTREEONLY) {
int err = lfsr_fs_ckmeta(&lfs);
assert(!err || err == LFS_ERR_CORRUPT);
if (err == LFS_ERR_CORRUPT) {
goto corrupt_mounted;
}
// run ckmeta mtreeonly?
} else if (METHOD == 2) {
} else if (CKMETA && MTREEONLY) {
// need an explicit traversal for this
lfsr_traversal_t t;
lfsr_traversal_open(&lfs, &t,
@@ -3112,8 +3118,7 @@ corrupt_mounted:;
# METHOD=1 => ckdata
# METHOD=2 => ckmeta+ckfetches
# METHOD=3 => ckmeta+ckdatacksums
# note only ckprogs works with open files
defines.METHOD = [0]
defines.METHOD = [0, 1, 2, 3]
defines.PERIOD = 10
# protecting the mrootanchor encourages more interesting failures, and
# simulates storage with hardened {0,1} blocks
@@ -3126,6 +3131,11 @@ defines.BADBLOCK_BEHAVIOR = '''
? LFS_EMUBD_BADBLOCK_PROGFLIP
: LFS_EMUBD_BADBLOCK_MANUAL
'''
defines.CKMETA = 'METHOD == 2 || METHOD == 3'
defines.CKDATA = 'METHOD == 1'
# note we need a full ckmeta if we have open files, ckfetches does not
# recheck open btrees
defines.MTREEONLY = false
defines.CKPROGS = 'METHOD == 0'
defines.CKFETCHES = 'METHOD == 2'
defines.CKPARITY = false
@@ -3241,21 +3251,21 @@ code = '''
}
// run ckdata?
if (METHOD == 1) {
if (CKDATA) {
int err = lfsr_fs_ckdata(&lfs);
assert(!err || err == LFS_ERR_CORRUPT);
if (err == LFS_ERR_CORRUPT) {
goto corrupt_open;
}
// run ckmeta?
} else if (METHOD == 3) {
} else if (CKMETA && !MTREEONLY) {
int err = lfsr_fs_ckmeta(&lfs);
assert(!err || err == LFS_ERR_CORRUPT);
if (err == LFS_ERR_CORRUPT) {
goto corrupt_open;
}
// run ckmeta mtreeonly?
} else if (METHOD == 2) {
} else if (CKMETA && MTREEONLY) {
// need an explicit traversal for this
lfsr_traversal_t t;
lfsr_traversal_open(&lfs, &t,
@@ -3311,8 +3321,11 @@ code = '''
// sync?
if (SYNC) {
int err = lfsr_file_sync(&lfs, &file);
assert(!err || err == LFS_ERR_NOSPC);
if (err == LFS_ERR_NOSPC) {
assert(!err
|| err == LFS_ERR_NOSPC
|| (err == LFS_ERR_CORRUPT
&& (METHOD == 2 || METHOD == 3)));
if (err == LFS_ERR_NOSPC || err == LFS_ERR_CORRUPT) {
goto corrupt_open;
}
}
@@ -3385,6 +3398,7 @@ code = '''
goto corrupt_mounted;
corrupt_open:;
lfsr_file_desync(&lfs, &file) => 0;
lfsr_file_close(&lfs, &file) => 0;
corrupt_mounted:;
lfsr_unmount(&lfs) => 0;
@@ -3399,8 +3413,7 @@ corrupt_mounted:;
# METHOD=1 => ckdata
# METHOD=2 => ckmeta+ckfetches
# METHOD=3 => ckmeta+ckdatacksums
# note only ckprogs works with open files
defines.METHOD = [0]
defines.METHOD = [0, 1, 2, 3]
defines.PERIOD = 10
# protecting the mrootanchor encourages more interesting failures, and
# simulates storage with hardened {0,1} blocks
@@ -3413,6 +3426,11 @@ defines.BADBLOCK_BEHAVIOR = '''
? LFS_EMUBD_BADBLOCK_PROGFLIP
: LFS_EMUBD_BADBLOCK_MANUAL
'''
defines.CKMETA = 'METHOD == 2 || METHOD == 3'
defines.CKDATA = 'METHOD == 1'
# note we need a full ckmeta if we have open files, ckfetches does not
# recheck open btrees
defines.MTREEONLY = false
defines.CKPROGS = 'METHOD == 0'
defines.CKFETCHES = 'METHOD == 2'
defines.CKPARITY = false
@@ -3509,21 +3527,21 @@ code = '''
}
// run ckdata?
if (METHOD == 1) {
if (CKDATA) {
int err = lfsr_fs_ckdata(&lfs);
assert(!err || err == LFS_ERR_CORRUPT);
if (err == LFS_ERR_CORRUPT) {
goto corrupt_mounted;
}
// run ckmeta?
} else if (METHOD == 3) {
} else if (CKMETA && !MTREEONLY) {
int err = lfsr_fs_ckmeta(&lfs);
assert(!err || err == LFS_ERR_CORRUPT);
if (err == LFS_ERR_CORRUPT) {
goto corrupt_mounted;
}
// run ckmeta mtreeonly?
} else if (METHOD == 2) {
} else if (CKMETA && MTREEONLY) {
// need an explicit traversal for this
lfsr_traversal_t t;
lfsr_traversal_open(&lfs, &t,
@@ -3901,8 +3919,7 @@ corrupt_mounted:;
# METHOD=1 => ckdata
# METHOD=2 => ckmeta+ckfetches
# METHOD=3 => ckmeta+ckdatacksums
# note only ckprogs works with open files
defines.METHOD = [0]
defines.METHOD = [0, 1, 2, 3]
defines.PERIOD = 10
# protecting the mrootanchor encourages more interesting failures, and
# simulates storage with hardened {0,1} blocks
@@ -3915,6 +3932,11 @@ defines.BADBLOCK_BEHAVIOR = '''
? LFS_EMUBD_BADBLOCK_PROGFLIP
: LFS_EMUBD_BADBLOCK_MANUAL
'''
defines.CKMETA = 'METHOD == 2 || METHOD == 3'
defines.CKDATA = 'METHOD == 1'
# note we need a full ckmeta if we have open files, ckfetches does not
# recheck open btrees
defines.MTREEONLY = false
defines.CKPROGS = 'METHOD == 0'
defines.CKFETCHES = 'METHOD == 2'
defines.CKPARITY = false
@@ -4012,21 +4034,21 @@ code = '''
}
// run ckdata?
if (METHOD == 1) {
if (CKDATA) {
int err = lfsr_fs_ckdata(&lfs);
assert(!err || err == LFS_ERR_CORRUPT);
if (err == LFS_ERR_CORRUPT) {
goto corrupt_mounted;
}
// run ckmeta?
} else if (METHOD == 3) {
} else if (CKMETA && !MTREEONLY) {
int err = lfsr_fs_ckmeta(&lfs);
assert(!err || err == LFS_ERR_CORRUPT);
if (err == LFS_ERR_CORRUPT) {
goto corrupt_mounted;
}
// run ckmeta mtreeonly?
} else if (METHOD == 2) {
} else if (CKMETA && MTREEONLY) {
// need an explicit traversal for this
lfsr_traversal_t t;
lfsr_traversal_open(&lfs, &t,