Added a test for mtree cycle detection, limited cycle detection to mdirs
I intended to also add a test for cycles in the btree that backs the mtree (and eventually other btrees), but something really curious happened. It turns out it's actually really hard to create a btree cycle, even intentionally. This is because each CoW btree pointer includes the expected CRC of the branch's rbyd. To succesfully create a cycle that isn't trivially detected in a validating mtree traversal, you would somehow need to solve for a cyclic set of dependent CRCs that are still valid. I suspect this is slightly easier than a hash-based construction, due to the linear nature of CRCs, but still I think it's unreasonable to expect these sort of cycles to occur in the wild. Even with filesystem bugs. --- Note this isn't true for the mdirs, which are mutable so storing a checksum in the pointer isn't possible. For this reason, cycle detection is kept for mdirs during mtree traversal. This may not be strictly necessary for the mtree, but it needed for the mroot chain. Nonetheless, this does simplify things. Specifically it reduces the cycle detection's tortoise state to only mdir pairs.
This commit is contained in:
@@ -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;
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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;
|
||||
'''
|
||||
|
||||
|
||||
Reference in New Issue
Block a user