From 8cc82cf754a779a7f81a4a99d168c12da1951853 Mon Sep 17 00:00:00 2001 From: "Robert M. B" Date: Mon, 30 Oct 2017 15:15:40 +0100 Subject: [PATCH 1/8] Te function "spiffs_create_object" did use unitialized varaibles according to valgrind. --- src/spiffs_gc.c | 8 +++++--- src/spiffs_nucleus.c | 32 ++++++++++++++++++-------------- src/test/test_bugreports.c | 2 +- src/test/test_spiffs.c | 4 ++-- 4 files changed, 26 insertions(+), 20 deletions(-) diff --git a/src/spiffs_gc.c b/src/spiffs_gc.c index db1af4c..d46df26 100644 --- a/src/spiffs_gc.c +++ b/src/spiffs_gc.c @@ -107,9 +107,11 @@ s32_t spiffs_gc_quick( // Checks if garbage collecting is necessary. If so a candidate block is found, // cleansed and erased s32_t spiffs_gc_check( - spiffs *fs, - u32_t len) { - s32_t res; + spiffs *fs, + u32_t len + ) +{ + s32_t res = SPIFFS_OK; s32_t free_pages = (SPIFFS_PAGES_PER_BLOCK(fs) - SPIFFS_OBJ_LOOKUP_PAGES(fs)) * (fs->block_count-2) - fs->stats_p_allocated - fs->stats_p_deleted; diff --git a/src/spiffs_nucleus.c b/src/spiffs_nucleus.c index 6b052b0..c2b32fd 100644 --- a/src/spiffs_nucleus.c +++ b/src/spiffs_nucleus.c @@ -451,8 +451,9 @@ s32_t spiffs_obj_lu_find_free( spiffs_block_ix starting_block, int starting_lu_entry, spiffs_block_ix *block_ix, - int *lu_entry) { - s32_t res; + int *lu_entry) +{ + s32_t res = SPIFFS_OK; if (!fs->cleaning && fs->free_blocks < 2) { res = spiffs_gc_quick(fs, 0); if (res == SPIFFS_ERR_NO_DELETED_BLOCKS) { @@ -910,16 +911,18 @@ s32_t spiffs_page_delete( #if !SPIFFS_READ_ONLY // Create an object index header page with empty index and undefined length s32_t spiffs_object_create( - spiffs *fs, - spiffs_obj_id obj_id, - const u8_t name[], - const u8_t meta[], - spiffs_obj_type type, - spiffs_page_ix *objix_hdr_pix) { - s32_t res = SPIFFS_OK; - spiffs_block_ix bix; - spiffs_page_object_ix_header oix_hdr; - int entry; + spiffs * fs, + spiffs_obj_id obj_id, + const u8_t name[], + const u8_t meta[], + spiffs_obj_type type, + spiffs_page_ix * objix_hdr_pix + ) +{ + s32_t res = SPIFFS_OK; + spiffs_block_ix bix = 0; + spiffs_page_object_ix_header oix_hdr = {.p_hdr = {0}}; + int entry = 0; res = spiffs_gc_check(fs, SPIFFS_DATA_PAGE_SIZE(fs)); SPIFFS_CHECK_RES(res); @@ -939,12 +942,13 @@ s32_t spiffs_object_create( fs->stats_p_allocated++; // write empty object index page + size_t const len = MIN( strlen((const char *)name), sizeof(oix_hdr.name)); oix_hdr.p_hdr.obj_id = obj_id; oix_hdr.p_hdr.span_ix = 0; oix_hdr.p_hdr.flags = 0xff & ~(SPIFFS_PH_FLAG_FINAL | SPIFFS_PH_FLAG_INDEX | SPIFFS_PH_FLAG_USED); oix_hdr.type = type; oix_hdr.size = SPIFFS_UNDEFINED_LEN; // keep ones so we can update later without wasting this page - strncpy((char*)oix_hdr.name, (const char*)name, SPIFFS_OBJ_NAME_LEN); + strncpy((char*)oix_hdr.name, (const char*)name, len); #if SPIFFS_OBJ_META_LEN if (meta) { _SPIFFS_MEMCPY(oix_hdr.meta, meta, SPIFFS_OBJ_META_LEN); @@ -1051,7 +1055,7 @@ void spiffs_cb_object_event( #endif // update index caches in all file descriptors spiffs_obj_id obj_id = obj_id_raw & ~SPIFFS_OBJ_ID_IX_FLAG; - u32_t i; + u32_t i = 0; spiffs_fd *fds = (spiffs_fd *)fs->fd_space; SPIFFS_DBG(" CALLBACK %s obj_id:"_SPIPRIid" spix:"_SPIPRIsp" npix:"_SPIPRIpg" nsz:"_SPIPRIi"\n", (const char *[]){"UPD", "NEW", "DEL", "MOV", "HUP","???"}[MIN(ev,5)], obj_id_raw, spix, new_pix, new_size); diff --git a/src/test/test_bugreports.c b/src/test/test_bugreports.c index e9b59d1..7bb3186 100644 --- a/src/test/test_bugreports.c +++ b/src/test/test_bugreports.c @@ -628,7 +628,7 @@ static int run_fuzz_test(FILE *f, int maxfds, int debuglog) { for (i = 0; i < 8; i++) { char buff[128]; - sprintf(buff, "%dfile%d.xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxasdasdasdadxxxxxxxxxxxxxxxxxxx", i, i); + snprintf(buff, sizeof(buff), "%dfile%d.xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxasdasdasdadxxxxxxxxxxxxxxxxxxx", i, i); buff[9 + 2 * i] = 0; filename[i] = strdup(buff); } diff --git a/src/test/test_spiffs.c b/src/test/test_spiffs.c index 78ca447..1beac02 100644 --- a/src/test/test_spiffs.c +++ b/src/test/test_spiffs.c @@ -113,7 +113,7 @@ static int mkpath(const char *path, mode_t mode) { // // char *make_test_fname(const char *name) { - sprintf(_path, "%s/%s", TEST_PATH, name); + snprintf(_path, sizeof(_path), "%s/%s", TEST_PATH, name); return _path; } @@ -132,7 +132,7 @@ void clear_test_path() { if (dp != NULL) { while ((ep = readdir(dp))) { if (ep->d_name[0] != '.') { - sprintf(_path, "%s/%s", TEST_PATH, ep->d_name); + snprintf(_path, sizeof(_path), "%s/%s", TEST_PATH, ep->d_name); remove(_path); } } From c04d9ec471357aefb37d3e318909b2b1862ca4f5 Mon Sep 17 00:00:00 2001 From: Robert-B Date: Tue, 31 Oct 2017 10:14:46 +0100 Subject: [PATCH 2/8] add shell-script to run valground wit hsome common options. --- build/run_valgrind.sh | 3 +++ 1 file changed, 3 insertions(+) create mode 100755 build/run_valgrind.sh diff --git a/build/run_valgrind.sh b/build/run_valgrind.sh new file mode 100755 index 0000000..3082230 --- /dev/null +++ b/build/run_valgrind.sh @@ -0,0 +1,3 @@ +#!/bin/bash +valgrind --show-reachable=yes --track-origins=yes --leak-check=full ./linux_spiffs_test &> valgrind_output.txt + From e17a74999144e97c8e8dabfb2e618fb2a6047b85 Mon Sep 17 00:00:00 2001 From: Robert-B Date: Fri, 1 Dec 2017 11:18:43 +0100 Subject: [PATCH 3/8] cleaning up the error handling. two tests did fail. --- build/run_valgrind.sh | 2 +- src/spiffs_cache.c | 6 ++ src/spiffs_nucleus.c | 67 +++++++++---------- src/test/test_bugreports.c | 2 +- src/test/test_spiffs.c | 127 +++++++++++++++++++++++++++++++------ 5 files changed, 148 insertions(+), 56 deletions(-) diff --git a/build/run_valgrind.sh b/build/run_valgrind.sh index 3082230..803e293 100755 --- a/build/run_valgrind.sh +++ b/build/run_valgrind.sh @@ -1,3 +1,3 @@ #!/bin/bash -valgrind --show-reachable=yes --track-origins=yes --leak-check=full ./linux_spiffs_test &> valgrind_output.txt +valgrind -v --show-reachable=yes --track-origins=yes --leak-check=full ./linux_spiffs_test &> valgrind_output.txt diff --git a/src/spiffs_cache.c b/src/spiffs_cache.c index e7cd4b7..29adc02 100644 --- a/src/spiffs_cache.c +++ b/src/spiffs_cache.c @@ -17,6 +17,10 @@ static spiffs_cache_page *spiffs_cache_page_get(spiffs *fs, spiffs_page_ix pix) int i; for (i = 0; i < cache->cpage_count; i++) { spiffs_cache_page *cp = spiffs_get_cache_page_hdr(fs, cache, i); + if (NULL == cp) + { + return 0; + } if ((cache->cpage_use_map & (1<flags & SPIFFS_CACHE_FLAG_TYPE_WR) == 0 && cp->pix == pix ) { @@ -165,6 +169,7 @@ s32_t spiffs_phys_rd( if (res2 != SPIFFS_OK) { // honor read failure before possible write failure (bad idea?) res = res2; + return res2; } u8_t *mem = spiffs_get_cache_page(fs, cache, cp->ix); _SPIFFS_MEMCPY(dst, &mem[SPIFFS_PADDR_TO_PAGE_OFFSET(fs, addr)], len); @@ -174,6 +179,7 @@ s32_t spiffs_phys_rd( if (res2 != SPIFFS_OK) { // honor read failure before possible write failure (bad idea?) res = res2; + return res2; } } } diff --git a/src/spiffs_nucleus.c b/src/spiffs_nucleus.c index c2b32fd..8e7b82a 100644 --- a/src/spiffs_nucleus.c +++ b/src/spiffs_nucleus.c @@ -16,7 +16,7 @@ static s32_t spiffs_page_data_check(spiffs *fs, spiffs_fd *fd, spiffs_page_ix pi return SPIFFS_ERR_INDEX_REF_INVALID; } #if SPIFFS_PAGE_CHECK - spiffs_page_header ph; + spiffs_page_header ph = {0}; res = _spiffs_rd( fs, SPIFFS_OP_T_OBJ_DA | SPIFFS_OP_C_READ, fd->file_nbr, @@ -45,7 +45,7 @@ static s32_t spiffs_page_index_check(spiffs *fs, spiffs_fd *fd, spiffs_page_ix p return SPIFFS_ERR_INDEX_INVALID; } #if SPIFFS_PAGE_CHECK - spiffs_page_header ph; + spiffs_page_header ph = {0}; res = _spiffs_rd( fs, SPIFFS_OP_T_OBJ_IX | SPIFFS_OP_C_READ, fd->file_nbr, @@ -233,7 +233,8 @@ s32_t spiffs_erase_block( // here we ignore res, just try erasing the block while (size > 0) { SPIFFS_DBG("erase "_SPIPRIad":"_SPIPRIi"\n", addr, SPIFFS_CFG_PHYS_ERASE_SZ(fs)); - SPIFFS_HAL_ERASE(fs, addr, SPIFFS_CFG_PHYS_ERASE_SZ(fs)); + res = SPIFFS_HAL_ERASE(fs, addr, SPIFFS_CFG_PHYS_ERASE_SZ(fs)); + SPIFFS_CHECK_RES(res); addr += SPIFFS_CFG_PHYS_ERASE_SZ(fs); size -= SPIFFS_CFG_PHYS_ERASE_SZ(fs); @@ -275,10 +276,10 @@ s32_t spiffs_probe( // Read three magics, as one block may be in an aborted erase state. // At least two of these must contain magic and be in decreasing order. - spiffs_obj_id magic[3]; - spiffs_obj_id bix_count[3]; + spiffs_obj_id magic[3] = {0}; + spiffs_obj_id bix_count[3] = {0}; - spiffs_block_ix bix; + spiffs_block_ix bix = {0}; for (bix = 0; bix < 3; bix++) { paddr = SPIFFS_MAGIC_PADDR(&dummy_fs, bix); #if SPIFFS_HAL_CALLBACK_EXTRA @@ -347,8 +348,8 @@ static s32_t spiffs_obj_lu_scan_v( // Checks magic if enabled s32_t spiffs_obj_lu_scan( spiffs *fs) { - s32_t res; - spiffs_block_ix bix; + s32_t res = SPIFFS_OK; + spiffs_block_ix bix = {0}; int entry; #if SPIFFS_USE_MAGIC spiffs_block_ix unerased_bix = (spiffs_block_ix)-1; @@ -362,7 +363,7 @@ s32_t spiffs_obj_lu_scan( spiffs_obj_id erase_count_max = 0; while (bix < fs->block_count) { #if SPIFFS_USE_MAGIC - spiffs_obj_id magic; + spiffs_obj_id magic = 0; res = _spiffs_rd(fs, SPIFFS_OP_T_OBJ_LU2 | SPIFFS_OP_C_READ, 0, SPIFFS_MAGIC_PADDR(fs, bix) , @@ -379,7 +380,7 @@ s32_t spiffs_obj_lu_scan( } } #endif - spiffs_obj_id erase_count; + spiffs_obj_id erase_count = 0; res = _spiffs_rd(fs, SPIFFS_OP_T_OBJ_LU2 | SPIFFS_OP_C_READ, 0, SPIFFS_ERASE_COUNT_PADDR(fs, bix) , @@ -506,8 +507,8 @@ static s32_t spiffs_obj_lu_find_id_and_span_v( int ix_entry, const void *user_const_p, void *user_var_p) { - s32_t res; - spiffs_page_header ph; + s32_t res = SPIFFS_OK; + spiffs_page_header ph = {0}; spiffs_page_ix pix = SPIFFS_OBJ_LOOKUP_ENTRY_TO_PIX(fs, bix, ix_entry); res = _spiffs_rd(fs, 0, SPIFFS_OP_T_OBJ_LU2 | SPIFFS_OP_C_READ, SPIFFS_PAGE_TO_PADDR(fs, pix), sizeof(spiffs_page_header), (u8_t *)&ph); @@ -531,9 +532,9 @@ s32_t spiffs_obj_lu_find_id_and_span( spiffs_span_ix spix, spiffs_page_ix exclusion_pix, spiffs_page_ix *pix) { - s32_t res; - spiffs_block_ix bix; - int entry; + s32_t res = SPIFFS_OK; + spiffs_block_ix bix = {0}; + int entry = 0; res = spiffs_obj_lu_find_entry_visitor(fs, fs->cursor_block_ix, @@ -570,9 +571,9 @@ s32_t spiffs_obj_lu_find_id_and_span_by_phdr( spiffs_span_ix spix, spiffs_page_ix exclusion_pix, spiffs_page_ix *pix) { - s32_t res; - spiffs_block_ix bix; - int entry; + s32_t res = SPIFFS_OK; + spiffs_block_ix bix = {0}; + int entry = 0; res = spiffs_obj_lu_find_entry_visitor(fs, fs->cursor_block_ix, @@ -671,7 +672,7 @@ static s32_t spiffs_populate_ix_map_v( const void *user_const_p, void *user_var_p) { (void)user_const_p; - s32_t res; + s32_t res = SPIFFS_OK; spiffs_ix_map_populate_state *state = (spiffs_ix_map_populate_state *)user_var_p; spiffs_page_ix pix = SPIFFS_OBJ_LOOKUP_ENTRY_TO_PIX(fs, bix, ix_entry); @@ -711,7 +712,7 @@ static s32_t spiffs_populate_ix_map_v( // populates index map, from vector entry start to vector entry end, inclusive s32_t spiffs_populate_ix_map(spiffs *fs, spiffs_fd *fd, u32_t vec_entry_start, u32_t vec_entry_end) { - s32_t res; + s32_t res = SPIFFS_OK; spiffs_ix_map *map = fd->ix_map; spiffs_ix_map_populate_state state; vec_entry_start = MIN((u32_t)(map->end_spix - map->start_spix), vec_entry_start); @@ -761,8 +762,8 @@ s32_t spiffs_page_allocate_data( u8_t finalize, spiffs_page_ix *pix) { s32_t res = SPIFFS_OK; - spiffs_block_ix bix; - int entry; + spiffs_block_ix bix = {0}; + int entry= 0; // find free entry res = spiffs_obj_lu_find_free(fs, fs->free_cursor_block_ix, fs->free_cursor_obj_lu_entry, &bix, &entry); @@ -818,12 +819,12 @@ s32_t spiffs_page_move( spiffs_page_header *page_hdr, spiffs_page_ix src_pix, spiffs_page_ix *dst_pix) { - s32_t res; + s32_t res = SPIFFS_OK; u8_t was_final = 0; - spiffs_page_header *p_hdr; - spiffs_block_ix bix; - int entry; - spiffs_page_ix free_pix; + spiffs_page_header *p_hdr = NULL; + spiffs_block_ix bix = 0; + int entry = 0; + spiffs_page_ix free_pix = 0; // find free entry res = spiffs_obj_lu_find_free(fs, fs->free_cursor_block_ix, fs->free_cursor_obj_lu_entry, &bix, &entry); @@ -877,7 +878,7 @@ s32_t spiffs_page_move( s32_t spiffs_page_delete( spiffs *fs, spiffs_page_ix pix) { - s32_t res; + s32_t res = SPIFFS_OK; // mark deleted entry in source object lookup spiffs_obj_id d_obj_id = SPIFFS_OBJ_ID_DELETED; res = _spiffs_wr(fs, SPIFFS_OP_T_OBJ_LU | SPIFFS_OP_C_DELE, @@ -991,8 +992,8 @@ s32_t spiffs_object_update_index_hdr( u32_t size, spiffs_page_ix *new_pix) { s32_t res = SPIFFS_OK; - spiffs_page_object_ix_header *objix_hdr; - spiffs_page_ix new_objix_hdr_pix; + spiffs_page_object_ix_header *objix_hdr = NULL; + spiffs_page_ix new_objix_hdr_pix = 0; obj_id |= SPIFFS_OBJ_ID_IX_FLAG; @@ -1162,7 +1163,7 @@ s32_t spiffs_object_open_by_id( spiffs_flags flags, spiffs_mode mode) { s32_t res = SPIFFS_OK; - spiffs_page_ix pix; + spiffs_page_ix pix = 0; res = spiffs_obj_lu_find_id_and_span(fs, obj_id | SPIFFS_OBJ_ID_IX_FLAG, 0, 0, &pix); SPIFFS_CHECK_RES(res); @@ -1181,8 +1182,8 @@ s32_t spiffs_object_open_by_page( spiffs_mode mode) { (void)mode; s32_t res = SPIFFS_OK; - spiffs_page_object_ix_header oix_hdr; - spiffs_obj_id obj_id; + spiffs_page_object_ix_header oix_hdr = {.p_hdr = {0}}; + spiffs_obj_id obj_id = 0; res = _spiffs_rd(fs, SPIFFS_OP_T_OBJ_IX | SPIFFS_OP_C_READ, fd->file_nbr, SPIFFS_PAGE_TO_PADDR(fs, pix), sizeof(spiffs_page_object_ix_header), (u8_t *)&oix_hdr); diff --git a/src/test/test_bugreports.c b/src/test/test_bugreports.c index 7bb3186..5db8d12 100644 --- a/src/test/test_bugreports.c +++ b/src/test/test_bugreports.c @@ -1250,7 +1250,7 @@ SUITE_TESTS(bug_tests) ADD_TEST(fuzzer_found_1) ADD_TEST(fuzzer_found_2) ADD_TEST(fuzzer_found_3) - ADD_TEST(fuzzer_found_4) +// ADD_TEST(fuzzer_found_4) ADD_TEST(remove_release_fd_152) ADD_TEST(certain_file_size_fail_165) ADD_TEST_NON_DEFAULT(fuzzer_found_single_1) diff --git a/src/test/test_spiffs.c b/src/test/test_spiffs.c index 1beac02..16e28fb 100644 --- a/src/test/test_spiffs.c +++ b/src/test/test_spiffs.c @@ -18,6 +18,7 @@ #include "test_spiffs.h" +#include #include #include #include @@ -27,11 +28,13 @@ #define AREA(x) _area[(x) - addr_offset] -static u32_t _area_sz; +static u32_t _area_sz = 0; +static u32_t _start_address = 0; static unsigned char *_area = NULL; static u32_t addr_offset = 0; -static int *_erases; +static u32_t *_erases = NULL; +static u32_t erase_sz = 0; static char _path[256]; static u32_t bytes_rd = 0; static u32_t bytes_wr = 0; @@ -175,7 +178,7 @@ static s32_t _write( spiffs *fs, #endif u32_t addr, u32_t size, u8_t *src) { - int i; + u32_t i; //printf("wr %08x %i\n", addr, size); if (log_flash_ops) { bytes_wr += size; @@ -200,6 +203,11 @@ static s32_t _write( } for (i = 0; i < size; i++) { + if ((addr + i) >= _area_sz || (addr + i) < _start_address) + { + printf("imp. address (_write): 0x%x (size:0x%x)\r\n", addr+i, _area_sz); + return -11; + } #if !SPIFFS_NO_BLIND_WRITES if (((addr + i) & (SPIFFS_CFG_LOG_PAGE_SZ(&__fs)-1)) == offsetof(spiffs_page_header, flags)) { /* Blind flag writes are allowed. */ @@ -220,7 +228,19 @@ static s32_t _erase( #if SPIFFS_HAL_CALLBACK_EXTRA spiffs *fs, #endif - u32_t addr, u32_t size) { + u32_t addr, + u32_t size) +{ + if (addr < SPIFFS_CFG_PHYS_ADDR(&__fs)) { + printf("FATAL erase addr too low %08x < %08x\n", addr, SPIFFS_PHYS_ADDR); + ERREXIT(); + return -1; + } + if (addr + size > SPIFFS_CFG_PHYS_ADDR(&__fs) + SPIFFS_CFG_PHYS_SZ(&__fs)) { + printf("FATAL erase addr too high %08x + %08x > %08x\n", addr, size, SPIFFS_PHYS_ADDR + SPIFFS_FLASH_SIZE); + ERREXIT(); + return -1; + } if (addr & (SPIFFS_CFG_PHYS_ERASE_SZ(&__fs)-1)) { printf("trying to erase at addr %08x, out of boundary\n", addr); ERREXIT(); @@ -231,7 +251,24 @@ static s32_t _erase( ERREXIT(); return -1; } - _erases[(addr-SPIFFS_CFG_PHYS_ADDR(&__fs))/SPIFFS_CFG_PHYS_ERASE_SZ(&__fs)]++; + // exceeds the memory boundaries + uint32_t div = SPIFFS_CFG_PHYS_ERASE_SZ(&__fs); + if (!div) + { + return -2; + } + uint32_t offset = SPIFFS_CFG_PHYS_ADDR(&__fs)/div; + if (offset > addr) + { + return -3; + } + uint32_t new_addr = addr-offset; + if (new_addr>erase_sz) + { + return -4; + } + + _erases[(addr-offset)]++; memset(&AREA(addr), 0xff, size); return 0; } @@ -289,6 +326,11 @@ void dump_page(spiffs *fs, spiffs_page_ix p) { // obj lu page printf("OBJ_LU"); } else { + if (addr >= _area_sz || addr < _start_address) + { + printf("imp. address (dump_page) 0x%x (size:0x%x)\r\n", addr, _area_sz); + return; + } u32_t obj_id_addr = SPIFFS_BLOCK_TO_PADDR(fs, SPIFFS_BLOCK_FOR_PAGE(fs , p)) + SPIFFS_OBJ_LOOKUP_ENTRY_FOR_PAGE(fs, p) * sizeof(spiffs_obj_id); spiffs_obj_id obj_id = *((spiffs_obj_id *)&AREA(obj_id_addr)); @@ -319,22 +361,37 @@ void dump_page(spiffs *fs, spiffs_page_ix p) { } void area_write(u32_t addr, u8_t *buf, u32_t size) { - int i; + u32_t i; for (i = 0; i < size; i++) { + if ((addr+i) >= _area_sz /*|| (addr + i) < _start_address*/) + { + printf("imp. address (write): 0x%x (size:0x%x)", addr+i, _area_sz); + return; + } AREA(addr + i) = *buf++; } } void area_set(u32_t addr, u8_t d, u32_t size) { - int i; + u32_t i; for (i = 0; i < size; i++) { + if ((addr+i) >= _area_sz || (addr + i) < _start_address) + { + printf("imp. address (area_set): 0x%x (size:0x%x)", addr+i, _area_sz); + return; + } AREA(addr + i) = d; } } void area_read(u32_t addr, u8_t *buf, u32_t size) { - int i; + u32_t i; for (i = 0; i < size; i++) { + if ((addr+i) >= _area_sz || (addr + i) < _start_address) + { + printf("imp. address (area_read): 0x%x (size:0x%x)", addr+i, _area_sz); + return; + } *buf++ = AREA(addr + i); } } @@ -464,15 +521,26 @@ s32_t fs_mount_specific(u32_t phys_addr, u32_t phys_size, return SPIFFS_mount(&__fs, &c, _work, _fds, _fds_sz, _cache, _cache_sz, spiffs_check_cb_f); } -static void fs_create(u32_t spiflash_size, - u32_t phys_sector_size, - u32_t log_page_size, - u32_t descriptors, u32_t cache_pages) { +static void fs_create( + u32_t spiflash_size, + u32_t start_address, + u32_t phys_sector_size, + u32_t log_page_size, + u32_t descriptors, + u32_t cache_pages + ) +{ + _start_address = start_address; _area_sz = spiflash_size; _area = malloc(spiflash_size); ASSERT(_area != NULL, "testbench area could not be malloced"); - const u32_t erase_sz = sizeof(int) * (spiflash_size / phys_sector_size); + if (!phys_sector_size) + { + printf("FATAL: sector_size is zero!\r\n"); + return; + } + erase_sz = sizeof(int) * (spiflash_size / phys_sector_size); _erases = malloc(erase_sz); ASSERT(_erases != NULL, "testbench erase log could not be malloced"); memset(_erases, 0, erase_sz); @@ -496,25 +564,41 @@ static void fs_create(u32_t spiflash_size, } static void fs_free(void) { - if (_area) free(_area); + if (_area) + free(_area); _area = NULL; - if (_erases) free(_erases); + + if (_erases) + free(_erases); _erases = NULL; - if (_fds) free(_fds); + + if (_fds) + free(_fds); _fds = NULL; - if (_cache) free(_cache); + + if (_cache) + free(_cache); _cache = NULL; - if (_work) free(_work); + + if (_work) + free(_work); _work = NULL; } /** * addr_offset */ -void fs_reset_specific(u32_t addr_offset, u32_t phys_addr, u32_t phys_size, - u32_t phys_sector_size, - u32_t log_block_size, u32_t log_page_size) { +void fs_reset_specific( + u32_t addr_offset, + u32_t phys_addr, + u32_t phys_size, + u32_t phys_sector_size, + u32_t log_block_size, + u32_t log_page_size + ) +{ fs_create(phys_size + phys_addr - addr_offset, + phys_addr, phys_sector_size, log_page_size, DEFAULT_NUM_FD, @@ -568,6 +652,7 @@ void fs_mount_dump(char *fname, u32_t phys_sector_size, u32_t log_block_size, u32_t log_page_size) { fs_create(phys_size + phys_addr - addr_offset, + phys_addr, phys_sector_size, log_page_size, DEFAULT_NUM_FD, From a5d3feb71b40f60e678e57320bfdde5ae07e99b5 Mon Sep 17 00:00:00 2001 From: sensslen Date: Mon, 4 Dec 2017 14:49:18 +0100 Subject: [PATCH 4/8] stupid test for the erase block size. --- src/test/test_spiffs.c | 75 ++++++++++++++++++++---------------------- 1 file changed, 35 insertions(+), 40 deletions(-) diff --git a/src/test/test_spiffs.c b/src/test/test_spiffs.c index 16e28fb..7cf8e23 100644 --- a/src/test/test_spiffs.c +++ b/src/test/test_spiffs.c @@ -231,46 +231,41 @@ static s32_t _erase( u32_t addr, u32_t size) { - if (addr < SPIFFS_CFG_PHYS_ADDR(&__fs)) { - printf("FATAL erase addr too low %08x < %08x\n", addr, SPIFFS_PHYS_ADDR); - ERREXIT(); - return -1; - } - if (addr + size > SPIFFS_CFG_PHYS_ADDR(&__fs) + SPIFFS_CFG_PHYS_SZ(&__fs)) { - printf("FATAL erase addr too high %08x + %08x > %08x\n", addr, size, SPIFFS_PHYS_ADDR + SPIFFS_FLASH_SIZE); - ERREXIT(); - return -1; - } - if (addr & (SPIFFS_CFG_PHYS_ERASE_SZ(&__fs)-1)) { - printf("trying to erase at addr %08x, out of boundary\n", addr); - ERREXIT(); - return -1; - } - if (size & (SPIFFS_CFG_PHYS_ERASE_SZ(&__fs)-1)) { - printf("trying to erase at with size %08x, out of boundary\n", size); - ERREXIT(); - return -1; - } - // exceeds the memory boundaries - uint32_t div = SPIFFS_CFG_PHYS_ERASE_SZ(&__fs); - if (!div) - { - return -2; - } - uint32_t offset = SPIFFS_CFG_PHYS_ADDR(&__fs)/div; - if (offset > addr) - { - return -3; - } - uint32_t new_addr = addr-offset; - if (new_addr>erase_sz) - { - return -4; - } - - _erases[(addr-offset)]++; - memset(&AREA(addr), 0xff, size); - return 0; + if (addr < SPIFFS_CFG_PHYS_ADDR(&__fs)) { + printf("FATAL erase addr too low %08x < %08x\n", addr, + SPIFFS_PHYS_ADDR); + ERREXIT(); + return -1; + } + if (addr + size > SPIFFS_CFG_PHYS_ADDR(&__fs) + SPIFFS_CFG_PHYS_SZ(&__fs)) { + printf("FATAL erase addr too high %08x + %08x > %08x\n", addr, size, + SPIFFS_PHYS_ADDR + SPIFFS_FLASH_SIZE); + ERREXIT(); + return -1; + } + if (addr & (SPIFFS_CFG_PHYS_ERASE_SZ(&__fs) - 1)) { + printf("trying to erase at addr %08x, out of boundary\n", addr); + ERREXIT(); + return -1; + } + if (size & (SPIFFS_CFG_PHYS_ERASE_SZ(&__fs) - 1)) { + printf("trying to erase at with size %08x, out of boundary\n", size); + ERREXIT(); + return -1; + } + // exceeds the memory boundaries + uint32_t div = SPIFFS_CFG_PHYS_ERASE_SZ(&__fs); + if (!div) { + return -2; + } + uint32_t offset = SPIFFS_CFG_PHYS_ADDR(&__fs) / div; + if (offset > addr) { + return -3; + } + + _erases[(addr - offset)]++; + memset(&AREA(addr), 0xff, size); + return 0; } void hexdump_mem(u8_t *b, u32_t len) { From a28262668233e31f6ee1e3fce652a6d9faec66f1 Mon Sep 17 00:00:00 2001 From: sensslen Date: Mon, 4 Dec 2017 15:08:40 +0100 Subject: [PATCH 5/8] faulte erase address calculation. --- src/test/test_spiffs.c | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/src/test/test_spiffs.c b/src/test/test_spiffs.c index 7cf8e23..d5eeacf 100644 --- a/src/test/test_spiffs.c +++ b/src/test/test_spiffs.c @@ -258,12 +258,8 @@ static s32_t _erase( if (!div) { return -2; } - uint32_t offset = SPIFFS_CFG_PHYS_ADDR(&__fs) / div; - if (offset > addr) { - return -3; - } - _erases[(addr - offset)]++; + _erases[(addr-SPIFFS_CFG_PHYS_ADDR(&__fs))/div]++; memset(&AREA(addr), 0xff, size); return 0; } From 9088f67c710e3f4f6b77db754cea91c4ac99920f Mon Sep 17 00:00:00 2001 From: sensslen Date: Mon, 4 Dec 2017 15:17:16 +0100 Subject: [PATCH 6/8] show spiffs result. --- src/test/test_spiffs.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/test_spiffs.c b/src/test/test_spiffs.c index d5eeacf..1c7e6cd 100644 --- a/src/test/test_spiffs.c +++ b/src/test/test_spiffs.c @@ -746,7 +746,7 @@ int read_and_verify_fd(spiffs_file fd, char *name) { int read_len = MIN(s.size - offs, sizeof(buf_d)); res = SPIFFS_read(&__fs, fd, buf_d, read_len); if (res < 0) { - printf(" read_and_verify: could not read file %s offs:%i len:%i filelen:%i\n", name, offs, read_len, s.size); + printf(" read_and_verify: could not read file %s offs:%i len:%i filelen:%i (res:%d)\n", name, offs, read_len, s.size, res); return res; } int pres = read(pfd, buf_v, read_len); From 90c3bd9bedb4164de16b041b2d392928b99d5255 Mon Sep 17 00:00:00 2001 From: sensslen Date: Tue, 5 Dec 2017 06:56:55 +0100 Subject: [PATCH 7/8] re-enabling two test-cases. strncpy calls changed to use the sizeof operator. --- src/spiffs_nucleus.c | 7 +++---- src/test/test_bugreports.c | 4 ++-- 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/src/spiffs_nucleus.c b/src/spiffs_nucleus.c index 8e7b82a..0b062b7 100644 --- a/src/spiffs_nucleus.c +++ b/src/spiffs_nucleus.c @@ -349,7 +349,7 @@ static s32_t spiffs_obj_lu_scan_v( s32_t spiffs_obj_lu_scan( spiffs *fs) { s32_t res = SPIFFS_OK; - spiffs_block_ix bix = {0}; + spiffs_block_ix bix; int entry; #if SPIFFS_USE_MAGIC spiffs_block_ix unerased_bix = (spiffs_block_ix)-1; @@ -943,13 +943,12 @@ s32_t spiffs_object_create( fs->stats_p_allocated++; // write empty object index page - size_t const len = MIN( strlen((const char *)name), sizeof(oix_hdr.name)); oix_hdr.p_hdr.obj_id = obj_id; oix_hdr.p_hdr.span_ix = 0; oix_hdr.p_hdr.flags = 0xff & ~(SPIFFS_PH_FLAG_FINAL | SPIFFS_PH_FLAG_INDEX | SPIFFS_PH_FLAG_USED); oix_hdr.type = type; oix_hdr.size = SPIFFS_UNDEFINED_LEN; // keep ones so we can update later without wasting this page - strncpy((char*)oix_hdr.name, (const char*)name, len); + strncpy((char*)oix_hdr.name, (const char*)name, sizeof(oix_hdr.name)); #if SPIFFS_OBJ_META_LEN if (meta) { _SPIFFS_MEMCPY(oix_hdr.meta, meta, SPIFFS_OBJ_META_LEN); @@ -1012,7 +1011,7 @@ s32_t spiffs_object_update_index_hdr( // change name if (name) { - strncpy((char*)objix_hdr->name, (const char*)name, SPIFFS_OBJ_NAME_LEN); + strncpy((char*)objix_hdr->name, (const char*)name, sizeof(objix_hdr->name)); } #if SPIFFS_OBJ_META_LEN if (meta) { diff --git a/src/test/test_bugreports.c b/src/test/test_bugreports.c index 5db8d12..0301a8a 100644 --- a/src/test/test_bugreports.c +++ b/src/test/test_bugreports.c @@ -1245,12 +1245,12 @@ SUITE_TESTS(bug_tests) ADD_TEST(temporal_fd_cache) ADD_TEST(spiffs_145) ADD_TEST(seek_bug_148) - //ADD_TEST(small_free_space) + ADD_TEST(small_free_space) ADD_TEST(lots_of_overwrite) ADD_TEST(fuzzer_found_1) ADD_TEST(fuzzer_found_2) ADD_TEST(fuzzer_found_3) -// ADD_TEST(fuzzer_found_4) + ADD_TEST(fuzzer_found_4) ADD_TEST(remove_release_fd_152) ADD_TEST(certain_file_size_fail_165) ADD_TEST_NON_DEFAULT(fuzzer_found_single_1) From c6b3c3312a32fd79b6224713b6d7cec021ad545a Mon Sep 17 00:00:00 2001 From: sensslen Date: Fri, 12 Jan 2018 13:18:47 +0100 Subject: [PATCH 8/8] last changes. mainly initialisation and sanity checks. --- src/spiffs_cache.c | 2 +- src/spiffs_hydrogen.c | 17 +++++++++----- src/spiffs_nucleus.c | 9 +++++--- src/test/test_bugreports.c | 47 ++++++++++++++++++++++++++++++++++++++ src/test/test_spiffs.c | 13 ++++++++--- 5 files changed, 75 insertions(+), 13 deletions(-) diff --git a/src/spiffs_cache.c b/src/spiffs_cache.c index 29adc02..a8e79a5 100644 --- a/src/spiffs_cache.c +++ b/src/spiffs_cache.c @@ -283,7 +283,7 @@ void spiffs_cache_fd_release(spiffs *fs, spiffs_cache_page *cp) { } spiffs_cache_page_free(fs, cp->ix, 0); - cp->obj_id = 0; + cp->obj_id = SPIFFS_OBJ_ID_DELETED; } #endif diff --git a/src/spiffs_hydrogen.c b/src/spiffs_hydrogen.c index 235aaaa..d62b48e 100644 --- a/src/spiffs_hydrogen.c +++ b/src/spiffs_hydrogen.c @@ -194,7 +194,7 @@ s32_t SPIFFS_creat(spiffs *fs, const char *path, spiffs_mode mode) { SPIFFS_API_CHECK_RES(fs, SPIFFS_ERR_NAME_TOO_LONG); } SPIFFS_LOCK(fs); - spiffs_obj_id obj_id; + spiffs_obj_id obj_id = SPIFFS_OBJ_ID_DELETED; s32_t res; res = spiffs_obj_lu_find_free_obj_id(fs, &obj_id, (const u8_t*)path); @@ -245,7 +245,7 @@ spiffs_file SPIFFS_open(spiffs *fs, const char *path, spiffs_flags flags, spiffs if ((flags & SPIFFS_O_CREAT) && res == SPIFFS_ERR_NOT_FOUND) { #if !SPIFFS_READ_ONLY - spiffs_obj_id obj_id; + spiffs_obj_id obj_id = SPIFFS_OBJ_ID_DELETED; // no need to enter conflicting name here, already looked for it above res = spiffs_obj_lu_find_free_obj_id(fs, &obj_id, 0); if (res < SPIFFS_OK) { @@ -727,7 +727,7 @@ s32_t SPIFFS_fremove(spiffs *fs, spiffs_file fh) { static s32_t spiffs_stat_pix(spiffs *fs, spiffs_page_ix pix, spiffs_file fh, spiffs_stat *s) { (void)fh; spiffs_page_object_ix_header objix_hdr; - spiffs_obj_id obj_id; + spiffs_obj_id obj_id = SPIFFS_OBJ_ID_DELETED; s32_t res =_spiffs_rd(fs, SPIFFS_OP_T_OBJ_IX | SPIFFS_OP_C_READ, fh, SPIFFS_PAGE_TO_PADDR(fs, pix), sizeof(spiffs_page_object_ix_header), (u8_t *)&objix_hdr); SPIFFS_API_CHECK_RES(fs, res); @@ -1026,12 +1026,17 @@ static s32_t spiffs_read_dir_v( void *user_var_p) { (void)user_const_p; s32_t res; - spiffs_page_object_ix_header objix_hdr; + spiffs_page_object_ix_header objix_hdr = {.p_hdr = {0}}; if (obj_id == SPIFFS_OBJ_ID_FREE || obj_id == SPIFFS_OBJ_ID_DELETED || (obj_id & SPIFFS_OBJ_ID_IX_FLAG) == 0) { return SPIFFS_VIS_COUNTINUE; } + if (NULL == user_var_p) + { + return SPIFFS_VIS_END; + } + spiffs_page_ix pix = SPIFFS_OBJ_LOOKUP_ENTRY_TO_PIX(fs, bix, ix_entry); res = _spiffs_rd(fs, SPIFFS_OP_T_OBJ_LU2 | SPIFFS_OP_C_READ, 0, SPIFFS_PAGE_TO_PADDR(fs, pix), sizeof(spiffs_page_object_ix_header), (u8_t *)&objix_hdr); @@ -1062,8 +1067,8 @@ struct spiffs_dirent *SPIFFS_readdir(spiffs_DIR *d, struct spiffs_dirent *e) { } SPIFFS_LOCK(d->fs); - spiffs_block_ix bix; - int entry; + spiffs_block_ix bix = 0; + int entry = 0; s32_t res; struct spiffs_dirent *ret = 0; diff --git a/src/spiffs_nucleus.c b/src/spiffs_nucleus.c index 0b062b7..c2f3b89 100644 --- a/src/spiffs_nucleus.c +++ b/src/spiffs_nucleus.c @@ -1182,7 +1182,7 @@ s32_t spiffs_object_open_by_page( (void)mode; s32_t res = SPIFFS_OK; spiffs_page_object_ix_header oix_hdr = {.p_hdr = {0}}; - spiffs_obj_id obj_id = 0; + spiffs_obj_id obj_id = SPIFFS_OBJ_ID_DELETED; res = _spiffs_rd(fs, SPIFFS_OP_T_OBJ_IX | SPIFFS_OP_C_READ, fd->file_nbr, SPIFFS_PAGE_TO_PADDR(fs, pix), sizeof(spiffs_page_object_ix_header), (u8_t *)&oix_hdr); @@ -2137,7 +2137,7 @@ static s32_t spiffs_obj_lu_find_free_obj_id_compact_v(spiffs *fs, spiffs_obj_id s32_t spiffs_obj_lu_find_free_obj_id(spiffs *fs, spiffs_obj_id *obj_id, const u8_t *conflicting_name) { s32_t res = SPIFFS_OK; u32_t max_objects = (fs->block_count * SPIFFS_OBJ_LOOKUP_MAX_ENTRIES(fs)) / 2; - spiffs_free_obj_id_state state; + spiffs_free_obj_id_state state = {0}; spiffs_obj_id free_obj_id = SPIFFS_OBJ_ID_FREE; state.min_obj_id = 1; state.max_obj_id = max_objects + 1; @@ -2152,7 +2152,10 @@ s32_t spiffs_obj_lu_find_free_obj_id(spiffs *fs, spiffs_obj_id *obj_id, const u8 u32_t i, j; SPIFFS_DBG("free_obj_id: BITM min:"_SPIPRIid" max:"_SPIPRIid"\n", state.min_obj_id, state.max_obj_id); - memset(fs->work, 0, SPIFFS_CFG_LOG_PAGE_SZ(fs)); + if (fs->work != NULL) + { + memset(fs->work, 0, SPIFFS_CFG_LOG_PAGE_SZ(fs)); + } res = spiffs_obj_lu_find_entry_visitor(fs, 0, 0, 0, 0, spiffs_obj_lu_find_free_obj_id_bitmap_v, conflicting_name, &state.min_obj_id, 0, 0); if (res == SPIFFS_VIS_END) res = SPIFFS_OK; diff --git a/src/test/test_bugreports.c b/src/test/test_bugreports.c index 0301a8a..4ea1b83 100644 --- a/src/test/test_bugreports.c +++ b/src/test/test_bugreports.c @@ -1230,8 +1230,55 @@ TEST(certain_file_size_fail_165) { return TEST_RES_OK; } TEST_END +TEST(test_joel) +{ + char *file_name1 = "first_file"; + char *file_name2 = "second_file"; + char *file_name3 = "third_file"; + int size = 512; + int res; + spiffs_DIR dir; + struct spiffs_dirent e; + struct spiffs_dirent *pe = &e; + + // create a clean file system starting at address 0, 2 megabytes big, + // sector size 65536, block size 65536, page size 256 + fs_reset_specific(0, 0, 1024*1024*2, 65536, 65536, 256); + + res = test_create_and_write_file(file_name1, size, 1); + TEST_CHECK(res >= 0); + + res = test_create_and_write_file(file_name2, size, 1); + TEST_CHECK(res >= 0); + + res = test_create_and_write_file(file_name3, size, 1); + TEST_CHECK(res >= 0); + + SPIFFS_opendir(FS, "/", &dir); + while ((pe = SPIFFS_readdir(&dir, &e))) { + printf("%s [%04x] size:%i\n", pe->name, pe->obj_id, pe->size); + } + SPIFFS_closedir(&dir); + + res = SPIFFS_remove(FS, file_name2); + TEST_CHECK(res == SPIFFS_OK); + printf("deleted\n"); + + //This causes an error, I don't know why... + SPIFFS_opendir(FS, "/", &dir); + printf("open\n"); + pe = &e; + while ((pe = SPIFFS_readdir(&dir, pe))) { + printf("%s [%04x] size:%i\n", pe->name, pe->obj_id, pe->size); + } + SPIFFS_closedir(&dir); + + return TEST_RES_OK; +} +TEST_END SUITE_TESTS(bug_tests) + ADD_TEST(test_joel) ADD_TEST(nodemcu_full_fs_1) ADD_TEST(nodemcu_full_fs_2) ADD_TEST(magic_test) diff --git a/src/test/test_spiffs.c b/src/test/test_spiffs.c index 1c7e6cd..74d2870 100644 --- a/src/test/test_spiffs.c +++ b/src/test/test_spiffs.c @@ -35,7 +35,7 @@ static u32_t addr_offset = 0; static u32_t *_erases = NULL; static u32_t erase_sz = 0; -static char _path[256]; +static char _path[512]; static u32_t bytes_rd = 0; static u32_t bytes_wr = 0; static u32_t reads = 0; @@ -1103,8 +1103,15 @@ int run_file_config(int cfg_count, tfile_conf* cfgs, int max_runs, int max_concu res = SPIFFS_write(FS, tf->fd, buf, size); CHECK_RES(res); int pfd = open(make_test_fname(tf->name), O_APPEND | O_RDWR); - write(pfd, buf, size); - close(pfd); + if (pfd >= 0) + { + write(pfd, buf, size); + close(pfd); + } + else + { + printf("\nERROR opening file: %s\n", tf->name); + } free(buf); res = read_and_verify(tf->name); CHECK_RES(res);