From f935fc0be6c6afe4101c7f249286d13408257e05 Mon Sep 17 00:00:00 2001 From: Christopher Haster Date: Tue, 10 Apr 2018 15:14:27 -0500 Subject: [PATCH] Fixed lookahead overflow and removed unbounded lookahead pointers As pointed out by davidefer, the lookahead pointer modular arithmetic does not work around integer overflow when the pointer size is not a multiple of the block count. To avoid overflow problems, the easy solution is to stop trying to work around integer overflows and keep the lookahead offset inside the block device. To make this work, the ack was modified into a resetable counter that is decremented every block allocation. As a plus, quite a bit of the allocation logic ended up simplified. --- lfs.c | 44 +++++++++++++++++++++++--------------------- lfs.h | 4 ++-- tests/test_alloc.sh | 9 ++++----- 3 files changed, 29 insertions(+), 28 deletions(-) diff --git a/lfs.c b/lfs.c index b810e51..931194b 100644 --- a/lfs.c +++ b/lfs.c @@ -270,8 +270,7 @@ int lfs_deorphan(lfs_t *lfs); static int lfs_alloc_lookahead(void *p, lfs_block_t block) { lfs_t *lfs = p; - lfs_block_t off = (((lfs_soff_t)(block - lfs->free.begin) - % (lfs_soff_t)(lfs->cfg->block_count)) + lfs_block_t off = ((block - lfs->free.off) + lfs->cfg->block_count) % lfs->cfg->block_count; if (off < lfs->free.size) { @@ -283,20 +282,22 @@ static int lfs_alloc_lookahead(void *p, lfs_block_t block) { static int lfs_alloc(lfs_t *lfs, lfs_block_t *block) { while (true) { - while (lfs->free.off != lfs->free.size) { - lfs_block_t off = lfs->free.off; - lfs->free.off += 1; + while (lfs->free.i != lfs->free.size) { + lfs_block_t off = lfs->free.i; + lfs->free.i += 1; + lfs->free.ack -= 1; if (!(lfs->free.buffer[off / 32] & (1U << (off % 32)))) { // found a free block - *block = (lfs->free.begin + off) % lfs->cfg->block_count; + *block = (lfs->free.off + off) % lfs->cfg->block_count; // eagerly find next off so an alloc ack can // discredit old lookahead blocks - while (lfs->free.off != lfs->free.size && - (lfs->free.buffer[lfs->free.off / 32] & - (1U << (lfs->free.off % 32)))) { - lfs->free.off += 1; + while (lfs->free.i != lfs->free.size && + (lfs->free.buffer[lfs->free.i / 32] + & (1U << (lfs->free.i % 32)))) { + lfs->free.i += 1; + lfs->free.ack -= 1; } return 0; @@ -304,15 +305,16 @@ static int lfs_alloc(lfs_t *lfs, lfs_block_t *block) { } // check if we have looked at all blocks since last ack - if (lfs->free.off == lfs->free.ack - lfs->free.begin) { - LFS_WARN("No more free space %d", lfs->free.off + lfs->free.begin); + //if (lfs->free.i == lfs->free.ack - lfs->free.off) { + if (lfs->free.ack == 0) { + LFS_WARN("No more free space %d", lfs->free.i + lfs->free.off); return LFS_ERR_NOSPC; } - lfs->free.begin += lfs->free.size; - lfs->free.size = lfs_min(lfs->cfg->lookahead, - lfs->free.ack - lfs->free.begin); - lfs->free.off = 0; + lfs->free.off = (lfs->free.off + lfs->free.size) + % lfs->cfg->block_count; + lfs->free.size = lfs_min(lfs->cfg->lookahead, lfs->free.ack); + lfs->free.i = 0; // find mask of free blocks from tree memset(lfs->free.buffer, 0, lfs->cfg->lookahead/8); @@ -324,7 +326,7 @@ static int lfs_alloc(lfs_t *lfs, lfs_block_t *block) { } static void lfs_alloc_ack(lfs_t *lfs) { - lfs->free.ack = lfs->free.begin+lfs->free.off + lfs->cfg->block_count; + lfs->free.ack = lfs->cfg->block_count; } @@ -2103,9 +2105,9 @@ int lfs_format(lfs_t *lfs, const struct lfs_config *cfg) { // create free lookahead memset(lfs->free.buffer, 0, lfs->cfg->lookahead/8); - lfs->free.begin = 0; - lfs->free.size = lfs_min(lfs->cfg->lookahead, lfs->cfg->block_count); lfs->free.off = 0; + lfs->free.size = lfs_min(lfs->cfg->lookahead, lfs->cfg->block_count); + lfs->free.i = 0; lfs_alloc_ack(lfs); // create superblock dir @@ -2182,9 +2184,9 @@ int lfs_mount(lfs_t *lfs, const struct lfs_config *cfg) { } // setup free lookahead - lfs->free.begin = 0; - lfs->free.size = 0; lfs->free.off = 0; + lfs->free.size = 0; + lfs->free.i = 0; lfs_alloc_ack(lfs); // load superblock diff --git a/lfs.h b/lfs.h index 1d65733..376776f 100644 --- a/lfs.h +++ b/lfs.h @@ -259,9 +259,9 @@ typedef struct lfs_superblock { } lfs_superblock_t; typedef struct lfs_free { - lfs_block_t begin; - lfs_block_t size; lfs_block_t off; + lfs_block_t size; + lfs_block_t i; lfs_block_t ack; uint32_t *buffer; } lfs_free_t; diff --git a/tests/test_alloc.sh b/tests/test_alloc.sh index 484f1cc..a2bce88 100755 --- a/tests/test_alloc.sh +++ b/tests/test_alloc.sh @@ -120,17 +120,16 @@ do tests/test.py << TEST lfs_mount(&lfs, &cfg) => 0; - // setup lookahead to almost overflow - lfs.free.begin = ((lfs_size_t)-1) - $SIZE/(2*cfg.block_size); - lfs.free.size = 0; - lfs.free.off = 0; +// // setup lookahead to almost overflow +// lfs.free.begin = ((lfs_size_t)-1) - 2*$SIZE; +// lfs.free.size = 0; +// lfs.free.off = 0; lfs_file_open(&lfs, &file[0], "overflow/$name", LFS_O_WRONLY | LFS_O_CREAT | LFS_O_APPEND) => 0; size = strlen("$name"); memcpy(buffer, "$name", size); for (int i = 0; i < $SIZE; i++) { - printf("%d\n", lfs.free.begin); lfs_file_write(&lfs, &file[0], buffer, size) => size; } lfs_file_close(&lfs, &file[0]) => 0;