From 47438a8c46afb7908761f1ef47daa133af0398f9 Mon Sep 17 00:00:00 2001 From: Christopher Haster Date: Thu, 16 Jan 2025 14:08:46 -0600 Subject: [PATCH] Fixed test_ck_spam*'s open file bshrub/btree issues - 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%) --- lfs.c | 14 ++++++++++ tests/test_ck.toml | 68 ++++++++++++++++++++++++++++++---------------- 2 files changed, 59 insertions(+), 23 deletions(-) diff --git a/lfs.c b/lfs.c index da2befbb..69ce03b8 100644 --- a/lfs.c +++ b/lfs.c @@ -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; diff --git a/tests/test_ck.toml b/tests/test_ck.toml index 192429cc..7559cec2 100644 --- a/tests/test_ck.toml +++ b/tests/test_ck.toml @@ -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,