Skip to content

Commit

Permalink
Fixed handful of block_size-related bugs found by CI
Browse files Browse the repository at this point in the history
- Valgrind-detected memory leak in test_superblocks.

- Valgrind-detected uninitialized lfs->block_size in lfs_init caused by
  an unintended sed replacement.

- Said sed replacement also changed the implementation of the v1 portion
  of lfs_migrate, this is out of scope and caused migrate to fail, which
  fortunately CI noticed.

- We also need to copy the block_size/block_count configs in v1 so that
  low-level bd operations still work.

- le-conversion in test_evil now matters due to difference in
  LFS_ERR_CORRUPT/LFS_ERR_INVAL paths during lfs_mount. This is good
  because we're actually testing for loop detection not pointer
  out-of-bounds as intended.
  • Loading branch information
geky committed Dec 16, 2022
1 parent 9c23c90 commit f8c86ab
Show file tree
Hide file tree
Showing 3 changed files with 20 additions and 7 deletions.
19 changes: 14 additions & 5 deletions lfs.c
Original file line number Diff line number Diff line change
Expand Up @@ -4087,7 +4087,7 @@ static int lfs_init(lfs_t *lfs, const struct lfs_config *cfg) {
lfs->attr_max = LFS_ATTR_MAX;
}

LFS_ASSERT(lfs->cfg->metadata_max <= lfs->block_size);
LFS_ASSERT(lfs->cfg->metadata_max <= lfs->cfg->block_size);

// setup default state
lfs->root[0] = LFS_BLOCK_NULL;
Expand Down Expand Up @@ -4139,8 +4139,8 @@ static int lfs_rawformat(lfs_t *lfs, const struct lfs_config *cfg) {
if (!lfs->block_size) {
lfs->block_size = lfs->erase_size;
}
LFS_ASSERT(lfs->cfg->block_count != 0);
lfs->block_count = lfs->cfg->block_count;
LFS_ASSERT(lfs->block_count != 0);

// check that the block size is large enough to fit ctz pointers
LFS_ASSERT(4*lfs_npw2(0xffffffff / (lfs->block_size-2*4))
Expand Down Expand Up @@ -4846,6 +4846,7 @@ static int lfs_fs_rawstat(lfs_t *lfs, struct lfs_fsinfo *fsinfo) {
return 0;
}

#ifndef LFS_READONLY
int lfs_fs_rawgrow(lfs_t *lfs, lfs_size_t block_count) {
// shrinking is not supported
LFS_ASSERT(block_count >= lfs->block_count);
Expand Down Expand Up @@ -4882,6 +4883,7 @@ int lfs_fs_rawgrow(lfs_t *lfs, lfs_size_t block_count) {

return 0;
}
#endif

#ifdef LFS_MIGRATE
////// Migration from littelfs v1 below this //////
Expand Down Expand Up @@ -5057,7 +5059,7 @@ static int lfs1_dir_fetch(lfs_t *lfs,
}

if ((0x7fffffff & test.size) < sizeof(test)+4 ||
(0x7fffffff & test.size) > lfs->block_size) {
(0x7fffffff & test.size) > lfs->cfg->block_size) {
continue;
}

Expand Down Expand Up @@ -5245,6 +5247,13 @@ static int lfs1_mount(lfs_t *lfs, struct lfs1 *lfs1,
return err;
}

// setup block_size, v1 did not support block_size searching, so
// block_size and block_count are required
LFS_ASSERT(lfs->cfg->block_size != 0);
LFS_ASSERT(lfs->cfg->block_count != 0);
lfs->block_size = lfs->cfg->block_size;
lfs->block_count = lfs->cfg->block_count;

lfs->lfs1 = lfs1;
lfs->lfs1->root[0] = LFS_BLOCK_NULL;
lfs->lfs1->root[1] = LFS_BLOCK_NULL;
Expand Down Expand Up @@ -5491,8 +5500,8 @@ static int lfs_rawmigrate(lfs_t *lfs, const struct lfs_config *cfg) {

lfs_superblock_t superblock = {
.version = LFS_DISK_VERSION,
.block_size = lfs->block_size,
.block_count = lfs->block_count,
.block_size = lfs->cfg->block_size,
.block_count = lfs->cfg->block_count,
.name_max = lfs->name_max,
.file_max = lfs->file_max,
.attr_max = lfs->attr_max,
Expand Down
5 changes: 3 additions & 2 deletions tests/test_evil.toml
Original file line number Diff line number Diff line change
Expand Up @@ -237,7 +237,7 @@ code = '''
lfs_dir_fetch(&lfs, &mdir, (lfs_block_t[2]){0, 1}) => 0;
lfs_dir_commit(&lfs, &mdir, LFS_MKATTRS(
{LFS_MKTAG(LFS_TYPE_HARDTAIL, 0x3ff, 8),
(lfs_block_t[2]){0, 1}})) => 0;
(lfs_block_t[2]){lfs_tole32(0), lfs_tole32(1)}})) => 0;
lfs_deinit(&lfs) => 0;
// test that mount fails gracefully
Expand Down Expand Up @@ -268,7 +268,7 @@ code = '''
lfs_dir_fetch(&lfs, &mdir, pair) => 0;
lfs_dir_commit(&lfs, &mdir, LFS_MKATTRS(
{LFS_MKTAG(LFS_TYPE_HARDTAIL, 0x3ff, 8),
(lfs_block_t[2]){0, 1}})) => 0;
(lfs_block_t[2]){lfs_tole32(0), lfs_tole32(1)}})) => 0;
lfs_deinit(&lfs) => 0;
// test that mount fails gracefully
Expand Down Expand Up @@ -297,6 +297,7 @@ code = '''
lfs_pair_fromle32(pair);
// change tail-pointer to point to ourself
lfs_dir_fetch(&lfs, &mdir, pair) => 0;
lfs_pair_tole32(pair);
lfs_dir_commit(&lfs, &mdir, LFS_MKATTRS(
{LFS_MKTAG(LFS_TYPE_HARDTAIL, 0x3ff, 8), pair})) => 0;
lfs_deinit(&lfs) => 0;
Expand Down
3 changes: 3 additions & 0 deletions tests/test_superblocks.toml
Original file line number Diff line number Diff line change
Expand Up @@ -330,6 +330,7 @@ code = '''
{LFS_MKTAG(LFS_TYPE_SUPERBLOCK, 0, 8), "littlefs"},
{LFS_MKTAG(LFS_TYPE_INLINESTRUCT, 0, sizeof(superblock)),
&superblock})) => 0;
lfs_deinit(&lfs) => 0;
// known block_size/block_count
cfg->block_size = BLOCK_SIZE;
Expand Down Expand Up @@ -461,6 +462,7 @@ code = '''
{LFS_MKTAG(LFS_TYPE_SUPERBLOCK, 0, 8), "littlefs"},
{LFS_MKTAG(LFS_TYPE_INLINESTRUCT, 0, sizeof(superblock)),
&superblock})) => 0;
lfs_deinit(&lfs) => 0;
// known block_size/block_count
cfg->block_size = BLOCK_SIZE;
Expand Down Expand Up @@ -521,6 +523,7 @@ code = '''
{LFS_MKTAG(LFS_TYPE_SUPERBLOCK, 0, 8), "littlefs"},
{LFS_MKTAG(LFS_TYPE_INLINESTRUCT, 0, sizeof(superblock)),
&superblock})) => 0;
lfs_deinit(&lfs) => 0;
// known block_size/block_count
cfg->block_size = BLOCK_SIZE;
Expand Down

0 comments on commit f8c86ab

Please sign in to comment.