diff --git a/lfs.c b/lfs.c index 951df6ef..13c37d79 100644 --- a/lfs.c +++ b/lfs.c @@ -5521,8 +5521,6 @@ typedef struct lfsr_mtree_traversal { uint8_t tortoise_power; lfs_size_t tortoise_step; lfsr_mpair_t tortoise_mpair; - lfs_block_t tortoise_mtree_block; - lfs_size_t tortoise_mtree_trunk; } lfsr_mtree_traversal_t; enum { @@ -5541,25 +5539,23 @@ static int lfsr_mtree_traversal_next(lfs_t *lfs, lfsr_mtree_traversal_t *traversal, lfs_size_t *mid_, lfsr_tag_t *tag_, lfsr_data_t *data_) { // detect cycles with Brent's algorithm - if (traversal->mdir.rbyd.trunk != 0 - && lfsr_mpair_eq(lfsr_mdir_mpair(&traversal->mdir), - traversal->tortoise_mpair) - && traversal->mtraversal.branch.block - == traversal->tortoise_mtree_block - && traversal->mtraversal.branch.trunk - == traversal->tortoise_mtree_trunk) { - LFS_ERROR("Cycle detected during mtree traversal"); - return LFS_ERR_CORRUPT; + if (traversal->mtraversal.branch.trunk == 0) { + if (traversal->mdir.rbyd.trunk != 0 + && lfsr_mpair_eq(lfsr_mdir_mpair(&traversal->mdir), + traversal->tortoise_mpair)) { + LFS_ERROR("Cycle detected during mtree traversal " + "(0x{%"PRIx32",%"PRIx32"})", + traversal->mdir.rbyd.block, traversal->mdir.other_block); + return LFS_ERR_CORRUPT; + } + if (traversal->tortoise_step + == ((lfs_size_t)1 << traversal->tortoise_power)) { + traversal->tortoise_mpair = lfsr_mdir_mpair(&traversal->mdir); + traversal->tortoise_step = 0; + traversal->tortoise_power += 1; + } + traversal->tortoise_step += 1; } - if (traversal->tortoise_step - == ((lfs_size_t)1 << traversal->tortoise_power)) { - traversal->tortoise_mpair = lfsr_mdir_mpair(&traversal->mdir); - traversal->tortoise_mtree_block = traversal->mtraversal.branch.block; - traversal->tortoise_mtree_trunk = traversal->mtraversal.branch.trunk; - traversal->tortoise_step = 0; - traversal->tortoise_power += 1; - } - traversal->tortoise_step += 1; // new traversal? start with 0x{0,1} // @@ -5642,7 +5638,7 @@ static int lfsr_mtree_traversal_next(lfs_t *lfs, && rid == -1 && lfsr_tag_suptype(tag) == LFSR_TAG_STRUCT) { if (tag != LFSR_TAG_MDIR && tag != LFSR_TAG_BTREE) { - LFS_ERROR("Weird mstruct? 0x%"PRIx32, tag); + LFS_ERROR("Weird mstruct? (0x%"PRIx32")", tag); return LFS_ERR_CORRUPT; } @@ -5685,7 +5681,7 @@ static int lfsr_mtree_traversal_next(lfs_t *lfs, if (err) { if (err == LFS_ERR_CORRUPT) { LFS_ERROR("Corrupted rbyd during mtree traversal " - "(rbyd=0x%"PRIx32".%"PRIx32", 0x%08"PRIx32")", + "(0x%"PRIx32".%"PRIx32", 0x%08"PRIx32")", branch->block, branch->trunk, branch->crc); } return err; @@ -5698,7 +5694,7 @@ static int lfsr_mtree_traversal_next(lfs_t *lfs, // crc check and trunk check if (branch_.crc != branch->crc) { LFS_ERROR("Checksum mismatch during mtree traversal " - "(rbyd=0x%"PRIx32".%"PRIx32", " + "(0x%"PRIx32".%"PRIx32", " "0x%08"PRIx32" != 0x%08"PRIx32")", branch->block, branch->trunk, branch_.crc, branch->crc); @@ -5754,7 +5750,7 @@ static int lfsr_mtree_traversal_next(lfs_t *lfs, return 0; } else { - LFS_ERROR("Weird mtree entry? 0x%"PRIx32, tag); + LFS_ERROR("Weird mtree entry? (0x%"PRIx32")", tag); return LFS_ERR_CORRUPT; } } diff --git a/tests/test_mtree.toml b/tests/test_mtree.toml index 657920b7..856c07c2 100644 --- a/tests/test_mtree.toml +++ b/tests/test_mtree.toml @@ -3742,3 +3742,63 @@ code = ''' } ''' + +## Cycle detection? ## + +# test that our cycle detector at least works in common cases +[cases.test_mtree_traversal_mroot_cycle] +in = 'lfs.c' +code = ''' + lfs_t lfs; + lfsr_format(&lfs, cfg) => 0; + lfsr_mount(&lfs, cfg) => 0; + + uint8_t buffer[LFSR_MPAIR_DSIZE]; + lfs_ssize_t d = lfsr_mpair_todisk(&lfs, LFSR_MPAIR(0, 1), buffer); + assert(d >= 0); + lfsr_mdir_commit(&lfs, &lfs.mroot, &(lfs_ssize_t){-1}, LFSR_ATTRS( + LFSR_ATTR(-1, MROOT, 0, buffer, d))) => 0; + + // technically, cycle detection only needs to work when we're validating + lfsr_mtree_traversal_t traversal = LFSR_MTREE_TRAVERSAL_INIT( + LFSR_MTREE_TRAVERSAL_VALIDATE); + + for (lfs_block_t i = 0;; i++) { + // assert that we detect the cycle in a reasonable number of iterations + assert(i < 1024); + + lfs_size_t mid_; + lfsr_tag_t tag_; + lfsr_data_t data_; + int err = lfsr_mtree_traversal_next(&lfs, &traversal, + &mid_, &tag_, &data_); + assert(!err || err == LFS_ERR_CORRUPT); + if (err == LFS_ERR_CORRUPT) { + break; + } + + if (tag_ == LFSR_TAG_BTREE) { + lfsr_rbyd_t *branch = (lfsr_rbyd_t *)data_.buf.buffer; + printf("traversal: %d 0x%x btree 0x%x.%x\n", + mid_, + tag_, + branch->block, branch->trunk); + } else if (tag_ == LFSR_TAG_MDIR) { + lfsr_mdir_t *mdir = (lfsr_mdir_t*)data_.buf.buffer; + printf("traversal: %d 0x%x mdir 0x{%x,%x}\n", + mid_, + tag_, + mdir->rbyd.block, mdir->other_block); + } else { + // this shouldn't happen + printf("traversal: %d 0x%x %d\n", + mid_, + tag_, + lfsr_data_size(data_)); + assert(false); + } + } + + lfsr_unmount(&lfs) => 0; +''' +