Skip to content

Commit

Permalink
Added "evil" tests and detecion/recovery from bad pointers and infini…
Browse files Browse the repository at this point in the history
…te loops

These two features have been much requested by users, and have even had
several PRs proposed to fix these in several cases. Before this, these
error conditions usually were caught by internal asserts, however
asserts prevented users from implementing their own workarounds.

It's taken me a while to provide/accept a useful recovery mechanism
(returning LFS_ERR_CORRUPT instead of asserting) because my original thinking
was that these error conditions only occur due to bugs in the filesystem, and
these bugs should be fixed properly.

While I still think this is mostly true, the point has been made clear
that being able to recover from these conditions is definitely worth the
code cost. Hopefully this new behaviour helps the longevity of devices
even if the storage code fails.

Another, less important, reason I didn't want to accept fixes for these
situations was the lack of tests that prove the code's value. This has
been fixed with the new testing framework thanks to the additional of
"internal tests" which can call C static functions and really take
advantage of the internal information of the filesystem.
  • Loading branch information
geky committed Mar 20, 2020
1 parent cb26157 commit 4677421
Show file tree
Hide file tree
Showing 2 changed files with 326 additions and 6 deletions.
44 changes: 38 additions & 6 deletions lfs.c
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,8 @@ static int lfs_bd_read(lfs_t *lfs,
lfs_block_t block, lfs_off_t off,
void *buffer, lfs_size_t size) {
uint8_t *data = buffer;
if ((off+size > lfs->cfg->block_size) || (block == LFS_BLOCK_NULL)) {
if (block >= lfs->cfg->block_count ||
off+size > lfs->cfg->block_size) {
return LFS_ERR_CORRUPT;
}

Expand Down Expand Up @@ -172,7 +173,7 @@ static int lfs_bd_prog(lfs_t *lfs,
lfs_block_t block, lfs_off_t off,
const void *buffer, lfs_size_t size) {
const uint8_t *data = buffer;
LFS_ASSERT(block != LFS_BLOCK_NULL);
LFS_ASSERT(block == LFS_BLOCK_INLINE || block < lfs->cfg->block_count);
LFS_ASSERT(off + size <= lfs->cfg->block_size);

while (size > 0) {
Expand Down Expand Up @@ -747,6 +748,12 @@ static lfs_stag_t lfs_dir_fetchmatch(lfs_t *lfs,
// scanning the entire directory
lfs_stag_t besttag = -1;

// if either block address is invalid we return LFS_ERR_CORRUPT here,
// otherwise later writes to the pair could fail
if (pair[0] >= lfs->cfg->block_count || pair[1] >= lfs->cfg->block_count) {
return LFS_ERR_CORRUPT;
}

// find the block with the most recent revision
uint32_t revs[2] = {0, 0};
int r = 0;
Expand Down Expand Up @@ -2196,7 +2203,6 @@ static int lfs_ctz_find(lfs_t *lfs,
return err;
}

LFS_ASSERT(head >= 2 && head <= lfs->cfg->block_count);
current -= 1 << skip;
}

Expand All @@ -2216,7 +2222,6 @@ static int lfs_ctz_extend(lfs_t *lfs,
if (err) {
return err;
}
LFS_ASSERT(nblock >= 2 && nblock <= lfs->cfg->block_count);

{
err = lfs_bd_erase(lfs, nblock);
Expand Down Expand Up @@ -2289,8 +2294,6 @@ static int lfs_ctz_extend(lfs_t *lfs,
return err;
}
}

LFS_ASSERT(nhead >= 2 && nhead <= lfs->cfg->block_count);
}

*block = nblock;
Expand Down Expand Up @@ -3661,7 +3664,15 @@ int lfs_mount(lfs_t *lfs, const struct lfs_config *cfg) {

// scan directory blocks for superblock and any global updates
lfs_mdir_t dir = {.tail = {0, 1}};
lfs_block_t cycle = 0;
while (!lfs_pair_isnull(dir.tail)) {
if (cycle >= lfs->cfg->block_count/2) {
// loop detected
err = LFS_ERR_CORRUPT;
goto cleanup;
}
cycle += 1;

// fetch next block in tail list
lfs_stag_t tag = lfs_dir_fetchmatch(lfs, &dir, dir.tail,
LFS_MKTAG(0x7ff, 0x3ff, 0),
Expand Down Expand Up @@ -3803,7 +3814,14 @@ int lfs_fs_traverseraw(lfs_t *lfs,
}
#endif

lfs_block_t cycle = 0;
while (!lfs_pair_isnull(dir.tail)) {
if (cycle >= lfs->cfg->block_count/2) {
// loop detected
return LFS_ERR_CORRUPT;
}
cycle += 1;

for (int i = 0; i < 2; i++) {
int err = cb(data, dir.tail[i]);
if (err) {
Expand Down Expand Up @@ -3887,7 +3905,14 @@ static int lfs_fs_pred(lfs_t *lfs,
// iterate over all directory directory entries
pdir->tail[0] = 0;
pdir->tail[1] = 1;
lfs_block_t cycle = 0;
while (!lfs_pair_isnull(pdir->tail)) {
if (cycle >= lfs->cfg->block_count/2) {
// loop detected
return LFS_ERR_CORRUPT;
}
cycle += 1;

if (lfs_pair_cmp(pdir->tail, pair) == 0) {
return 0;
}
Expand Down Expand Up @@ -3930,7 +3955,14 @@ static lfs_stag_t lfs_fs_parent(lfs_t *lfs, const lfs_block_t pair[2],
// use fetchmatch with callback to find pairs
parent->tail[0] = 0;
parent->tail[1] = 1;
lfs_block_t cycle = 0;
while (!lfs_pair_isnull(parent->tail)) {
if (cycle >= lfs->cfg->block_count/2) {
// loop detected
return LFS_ERR_CORRUPT;
}
cycle += 1;

lfs_stag_t tag = lfs_dir_fetchmatch(lfs, parent, parent->tail,
LFS_MKTAG(0x7ff, 0, 0x3ff),
LFS_MKTAG(LFS_TYPE_DIRSTRUCT, 0, 8),
Expand Down
Loading

0 comments on commit 4677421

Please sign in to comment.