Github/littlefs
1
Fork 0
Code Actions Activity

Fixed several issues with compat flag parsing

Browse Source 
- Fixed issue where some overflowed compat flags could end up ignored.

  A simple typo: incrementing by the unrelated d variable, meant we
  were skipping overflowed compat flags whenever the previous logic sets
  d > 1.

- Fixed issue where any zero padding was treated as overflowed compat
  flags.

  Note this hid the previous issue from our tests.

Added more tests to prevent a regression here. Letting bad compat flag
parsing through would be _very_ annoying in the future.

Code changes:

           code          stack          ctx
  before: 38148           2608          752
  after:  38084 (-0.2%)   2608 (+0.0%)  752 (+0.0%)
This commit is contained in:
Christopher Haster
2025-01-05 00:02:11 -06:00
parent 82d0ffb355
commit ab160d7feb
2 changed files with 124 additions and 9 deletions
Download Patch File Download Diff File Expand all files Collapse all files
lfs.c
+6 -6
View File
@@ -13471,16 +13471,16 @@ static int lfsr_data_readrcompat(lfs_t *lfs, lfsr_data_t *data,
//
// we don't really care about performance here
while (lfsr_data_size(*data) > 0) {
lfs_scmp_t cmp = lfsr_data_cmp(lfs, *data, (uint8_t[]){0}, 1);
if (cmp < 0) {
return cmp;
uint8_t b;
lfs_ssize_t d = lfsr_data_read(lfs, data, &b, 1);
if (d < 0) {
return d;
}
if (cmp != LFS_CMP_EQ) {
if (b != 0x00) {
*rcompat |= LFSR_RCOMPAT_OVERFLOW;
break;
}
*data = LFSR_DATA_SLICE(*data, d, -1);
}
return 0;
tests/test_mount.toml
+118 -3
View File
@@ -703,6 +703,8 @@ code = '''
# these are just a bit harder to detect
[cases.test_mount_incompat_rcompat_overflow]
defines.OVERFLOW = 72
defines.FLAG = 'range(72)'
in = 'lfs.c'
code = '''
// create a superblock
@@ -713,12 +715,16 @@ code = '''
//
// note we're messing around with internals to do this! this
// is not a user API
uint8_t overflow[OVERFLOW];
memset(overflow, 0, sizeof(overflow));
overflow[FLAG / 8] |= 1 << (FLAG % 8);
lfsr_mount(&lfs, LFS_M_RDWR, CFG) => 0;
lfsr_mdir_commit(&lfs, &lfs.mroot, LFSR_RATS(
LFSR_RAT_CAT(
LFSR_TAG_RCOMPAT, 0,
LFSR_DATA_RCOMPAT(LFSR_RCOMPAT_COMPAT),
LFSR_DATA_BUF("\x00\x00\x00\x00\x80", 5)))) => 0;
LFSR_DATA_BUF(overflow, sizeof(overflow))))) => 0;
lfsr_unmount(&lfs) => 0;
// mount should now fail
@@ -727,22 +733,31 @@ code = '''
'''
[cases.test_mount_incompat_wcompat_overflow]
defines.OVERFLOW = 72
defines.FLAG = 'range(72)'
in = 'lfs.c'
code = '''
// create a superblock
lfs_t lfs;
lfsr_format(&lfs, LFS_F_RDWR, CFG) => 0;
uint8_t flags[9] = {0};
flags[FLAG / 8] |= 1 << (FLAG % 8);
// set a really far wcompat flag
//
// note we're messing around with internals to do this! this
// is not a user API
uint8_t overflow[OVERFLOW];
memset(overflow, 0, sizeof(overflow));
overflow[FLAG / 8] |= 1 << (FLAG % 8);
lfsr_mount(&lfs, LFS_M_RDWR, CFG) => 0;
lfsr_mdir_commit(&lfs, &lfs.mroot, LFSR_RATS(
LFSR_RAT_CAT(
LFSR_TAG_WCOMPAT, 0,
LFSR_DATA_WCOMPAT(LFSR_WCOMPAT_COMPAT),
LFSR_DATA_BUF("\x00\x00\x00\x00\x80", 5)))) => 0;
LFSR_DATA_BUF(overflow, sizeof(overflow))))) => 0;
lfsr_unmount(&lfs) => 0;
// mount should now fail
@@ -754,6 +769,8 @@ code = '''
'''
[cases.test_mount_incompat_ocompat_overflow]
defines.OVERFLOW = 72
defines.FLAG = 'range(72)'
in = 'lfs.c'
code = '''
// create a superblock
@@ -764,12 +781,16 @@ code = '''
//
// note we're messing around with internals to do this! this
// is not a user API
uint8_t overflow[OVERFLOW];
memset(overflow, 0, sizeof(overflow));
overflow[FLAG / 8] |= 1 << (FLAG % 8);
lfsr_mount(&lfs, LFS_M_RDWR, CFG) => 0;
lfsr_mdir_commit(&lfs, &lfs.mroot, LFSR_RATS(
LFSR_RAT_CAT(
LFSR_TAG_OCOMPAT, 0,
LFSR_DATA_OCOMPAT(LFSR_OCOMPAT_COMPAT),
LFSR_DATA_BUF("\x00\x00\x00\x00\x80", 5)))) => 0;
LFSR_DATA_BUF(overflow, sizeof(overflow))))) => 0;
lfsr_unmount(&lfs) => 0;
// mount should _not_ fail, ocompat should always be ignored
@@ -779,6 +800,100 @@ code = '''
lfsr_unmount(&lfs) => 0;
'''
# but just appending zeros is _not_ an error
[cases.test_mount_incompat_rcompat_padding]
defines.OVERFLOW = 72
in = 'lfs.c'
code = '''
// create a superblock
lfs_t lfs;
lfsr_format(&lfs, LFS_F_RDWR, CFG) => 0;
// set a really far rcompat flag
//
// note we're messing around with internals to do this! this
// is not a user API
uint8_t overflow[OVERFLOW];
memset(overflow, 0, sizeof(overflow));
lfsr_mount(&lfs, LFS_M_RDWR, CFG) => 0;
lfsr_mdir_commit(&lfs, &lfs.mroot, LFSR_RATS(
LFSR_RAT_CAT(
LFSR_TAG_RCOMPAT, 0,
LFSR_DATA_RCOMPAT(LFSR_RCOMPAT_COMPAT),
LFSR_DATA_BUF(overflow, sizeof(overflow))))) => 0;
lfsr_unmount(&lfs) => 0;
// mount should _not_ fail, extra zeros should be ignored
lfsr_mount(&lfs, LFS_M_RDWR, CFG) => 0;
lfsr_unmount(&lfs) => 0;
lfsr_mount(&lfs, LFS_M_RDONLY, CFG) => 0;
lfsr_unmount(&lfs) => 0;
'''
[cases.test_mount_incompat_wcompat_padding]
defines.OVERFLOW = 72
in = 'lfs.c'
code = '''
// create a superblock
lfs_t lfs;
lfsr_format(&lfs, LFS_F_RDWR, CFG) => 0;
uint8_t flags[9] = {0};
flags[FLAG / 8] |= 1 << (FLAG % 8);
// set a really far wcompat flag
//
// note we're messing around with internals to do this! this
// is not a user API
uint8_t overflow[OVERFLOW];
memset(overflow, 0, sizeof(overflow));
lfsr_mount(&lfs, LFS_M_RDWR, CFG) => 0;
lfsr_mdir_commit(&lfs, &lfs.mroot, LFSR_RATS(
LFSR_RAT_CAT(
LFSR_TAG_WCOMPAT, 0,
LFSR_DATA_WCOMPAT(LFSR_WCOMPAT_COMPAT),
LFSR_DATA_BUF(overflow, sizeof(overflow))))) => 0;
lfsr_unmount(&lfs) => 0;
// mount should _not_ fail, extra zeros should be ignored
lfsr_mount(&lfs, LFS_M_RDWR, CFG) => 0;
lfsr_unmount(&lfs) => 0;
lfsr_mount(&lfs, LFS_M_RDONLY, CFG) => 0;
lfsr_unmount(&lfs) => 0;
'''
[cases.test_mount_incompat_ocompat_padding]
defines.OVERFLOW = 72
in = 'lfs.c'
code = '''
// create a superblock
lfs_t lfs;
lfsr_format(&lfs, LFS_F_RDWR, CFG) => 0;
// set a really far ocompat flag
//
// note we're messing around with internals to do this! this
// is not a user API
uint8_t overflow[OVERFLOW];
memset(overflow, 0, sizeof(overflow));
lfsr_mount(&lfs, LFS_M_RDWR, CFG) => 0;
lfsr_mdir_commit(&lfs, &lfs.mroot, LFSR_RATS(
LFSR_RAT_CAT(
LFSR_TAG_OCOMPAT, 0,
LFSR_DATA_OCOMPAT(LFSR_OCOMPAT_COMPAT),
LFSR_DATA_BUF(overflow, sizeof(overflow))))) => 0;
lfsr_unmount(&lfs) => 0;
// mount should _not_ fail, extra zeros should be ignored
lfsr_mount(&lfs, LFS_M_RDWR, CFG) => 0;
lfsr_unmount(&lfs) => 0;
lfsr_mount(&lfs, LFS_M_RDONLY, CFG) => 0;
lfsr_unmount(&lfs) => 0;
'''
# test that we fail to mount incompatible block sizes
[cases.test_mount_incompat_block_size]
defines.INC_BLOCK_SIZE = ['BLOCK_SIZE/2', 'BLOCK_SIZE*2']