From 3c17c94b949f922e31b7b560abbdc8eefaeb90ec Mon Sep 17 00:00:00 2001 From: Christopher Haster Date: Wed, 28 Dec 2022 15:22:28 -0600 Subject: [PATCH] Added test_rbyd_large with better boundary conditions near end-of-block test_rbyd_large also doubles as a decent fuzz test, since it involves many more tags than the permutation testing can ever hit. --- lfs.c | 30 +++++++++++------- lfs.h | 2 ++ tests/test_rbyd.toml | 75 ++++++++++++++++++++++++++++++++++++++++---- 3 files changed, 89 insertions(+), 18 deletions(-) diff --git a/lfs.c b/lfs.c index 7f60b896..611f6728 100644 --- a/lfs.c +++ b/lfs.c @@ -938,11 +938,15 @@ static lfs_ssize_t lfs_rbyd_readtag(lfs_t *lfs, uint8_t buffer[2*4]; lfs_size_t i = 0; + // force leb decoding to overflow when truncated + memset(buffer, 0xff, 2*4); + // TODO allow different hint for lookup? bench this? does our hint work backwards? // TODO should lfs_bd_read allow a range for reads? int err = lfs_bd_read(lfs, pcache, rcache, hint, - block, off, &buffer, sizeof(buffer)); + block, off, &buffer, + lfs_min(sizeof(buffer), lfs->cfg->block_size-off)); if (err) { return err; } @@ -1215,8 +1219,14 @@ tryagain:; static int lfs_rbyd_prog(lfs_t *lfs, lfs_cache_t *pcache, lfs_cache_t *rcache, lfs_block_t block, lfs_off_t off, - const void *buffer, lfs_size_t size, - uint32_t *crc) { + const void *buffer, lfs_size_t size, uint32_t *crc) { + // check for out-of-bounds here + // TODO should we just move this to lfs_bd_prog? + // TODO actually should we just build crc into lfs_bd_prog as well? + if (off+size > lfs->cfg->block_size) { + return LFS_ERR_RANGE; + } + int err = lfs_bd_prog(lfs, pcache, rcache, false, block, off, buffer, size); @@ -1240,8 +1250,7 @@ static lfs_ssize_t lfs_rbyd_progtag(lfs_t *lfs, tag <<= 1; // make sure to include the parity of the current crc - uint32_t crc_ = *crc; - tag |= lfs_popc(crc_) & 1; + tag |= lfs_popc(*crc) & 1; // compress into pair of leb128s uint8_t buffer[2*4]; @@ -1266,9 +1275,6 @@ static lfs_ssize_t lfs_rbyd_progtag(lfs_t *lfs, return err; } - // crc - *crc = lfs_crc32c(crc_, buffer, i); - return i; } @@ -1639,7 +1645,7 @@ static int lfs_rbyd_commit(lfs_t *lfs, lfs_rbyd_t *rbyd, // = 9 bytes // const lfs_off_t aligned = lfs_alignup( - lfs_min(off + 1+1+4+4 + 1+4+4, lfs->cfg->block_size), + off + 1+1+4+4 + 1+4+4, lfs->cfg->prog_size); // space for fcrc? @@ -1711,9 +1717,9 @@ static int lfs_rbyd_commit(lfs_t *lfs, lfs_rbyd_t *rbyd, } lfs_tole32_(crc, &buffer[1+4]); - int err = lfs_bd_prog(lfs, - &lfs->pcache, &lfs->rcache, false, - block, off, buffer, 1+4+4); + int err = lfs_rbyd_prog(lfs, + &lfs->pcache, &lfs->rcache, + block, off, buffer, 1+4+4, NULL); if (err) { return err; } diff --git a/lfs.h b/lfs.h index 2558fb13..e4159f51 100644 --- a/lfs.h +++ b/lfs.h @@ -84,6 +84,8 @@ enum lfs_error { LFS_ERR_NOMEM = -12, // No more memory available LFS_ERR_NOATTR = -61, // No data/attr available LFS_ERR_NAMETOOLONG = -36, // File name too long + LFS_ERR_RANGE = -7, // Result out of range + // TODO should all overflow errors actually be corrupt errors? LFS_ERR_OVERFLOW = -75, // Value too large for defined data type }; diff --git a/tests/test_rbyd.toml b/tests/test_rbyd.toml index f92b7f68..b919cb19 100644 --- a/tests/test_rbyd.toml +++ b/tests/test_rbyd.toml @@ -1507,9 +1507,8 @@ code = ''' for (int j = 0; j < N; j++) { lfs_rbyd_commit(&lfs, &rbyd, - LFS_MKRATTR( - UATTR, perm[j]+1, 0, - &(uint32_t){0xaaaaaaaa}, 4, NULL)) => 0; + LFS_MKRATTR(UATTR, perm[j]+1, 0, &(uint32_t){0xaaaaaaaa}, 4, + NULL)) => 0; } lfs_rbyd_fetch(&lfs, &rbyd, rbyd.block, NULL) => 0; @@ -1539,6 +1538,71 @@ code = ''' } ''' +[cases.test_rbyd_large] +in = 'lfs.c' +# ORDER: +# 0 = in-order +# 1 = reverse-order +# 2 = random-order +defines.ORDER = [0, 1, 2] +code = ''' + lfs_t lfs; + lfs_init(&lfs, cfg) => 0; + + lfs_rbyd_t init_rbyd = { + .block = 0, + .trunk = 0, + .off = 0, + .rev = 1, + .crc = 0, + .count = 0, + .erased = true, + }; + lfs_rbyd_t rbyd; + lfs_off_t off; + lfs_size_t size; + + // create the rbyd tree + rbyd = init_rbyd; + lfs_bd_erase(&lfs, rbyd.block) => 0; + + // keep appending tags until we run out of space + // + // note, this will likely repeat tags, but that's ok + // + lfs_size_t count = 0; + uint32_t prng = 42; + for (lfs_size_t i = 0;; i++) { + uint8_t x + = (ORDER == 0) ? (uint8_t)i + : (ORDER == 1) ? (uint8_t)(((lfs_size_t)-1) - i) + : (uint8_t)TEST_PRNG(&prng); + int err = lfs_rbyd_commit(&lfs, &rbyd, + LFS_MKRATTR(UATTR, x, 0, &(uint32_t){0xaaaaaaaa}, 4, + NULL)); + // if we can't fit an fcrc, erased is set to false, but if we can, + // lfs_rbyd_commit may error later with LFS_ERR_RANGE + if (!rbyd.erased || err == LFS_ERR_RANGE) { + break; + } + assert(err == 0); + + count = i; + } + + // check that we can still lookup all the tags + prng = 42; + for (lfs_size_t i = 0; i < count; i++) { + uint8_t x + = (ORDER == 0) ? (uint8_t)i + : (ORDER == 1) ? (uint8_t)(((lfs_size_t)-1) - i) + : (uint8_t)TEST_PRNG(&prng); + lfs_rbyd_lookup(&lfs, &rbyd, + LFS_MKRTAG(UATTR, x, 0), &off, &size) + => LFS_MKRTAG(UATTR, x, 0); + } +''' + ### removal testing ### @@ -1879,9 +1943,8 @@ code = ''' for (int j = 0; j < N; j++) { lfs_rbyd_commit(&lfs, &rbyd, - LFS_MKRATTR( - UATTR, perm[j]+1, 0, - &(uint32_t){0xaaaaaaaa}, 4, NULL)) => 0; + LFS_MKRATTR(UATTR, perm[j]+1, 0, &(uint32_t){0xaaaaaaaa}, 4, + NULL)) => 0; } // copy block so we can reset after each remove