From 5167c2d8bf85816ebd7da71673eae99fb23ac255 Mon Sep 17 00:00:00 2001 From: Christopher Haster Date: Tue, 6 Dec 2022 00:43:10 -0600 Subject: [PATCH] Fixed handful of block_size-related bugs found by CI - 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. --- lfs.c | 19 ++++++++++++++----- tests/test_evil.toml | 5 +++-- tests/test_superblocks.toml | 3 +++ 3 files changed, 20 insertions(+), 7 deletions(-) diff --git a/lfs.c b/lfs.c index e46b9867..8ed8b2d9 100644 --- a/lfs.c +++ b/lfs.c @@ -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; @@ -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)) @@ -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); @@ -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 ////// @@ -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; } @@ -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; @@ -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, diff --git a/tests/test_evil.toml b/tests/test_evil.toml index 1dde8650..a15d670a 100644 --- a/tests/test_evil.toml +++ b/tests/test_evil.toml @@ -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 @@ -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 @@ -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; diff --git a/tests/test_superblocks.toml b/tests/test_superblocks.toml index 6ff25345..881dd098 100644 --- a/tests/test_superblocks.toml +++ b/tests/test_superblocks.toml @@ -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; @@ -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; @@ -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;