fixes #158

makefile

@@ -17,11 +17,11 @@ builddir = build
 #############
 
 CC ?= gcc
-LD = ld
-GDB = gdb
-OBJCOPY = objcopy
-OBJDUMP = objdump
-MKDIR = mkdir -p
+LD ?= ld
+GDB ?= gdb
+OBJCOPY ?= objcopy
+OBJDUMP ?= objdump
+MKDIR ?= mkdir -p
 
 ###############
 #
@@ -43,6 +43,7 @@ CFILES_TEST = main.c \
 	test_bugreports.c \
 	testsuites.c \
 	testrunner.c
+CFLAGS += -D_SPIFFS_TEST
 endif
 include files.mk
 INCLUDE_DIRECTIVES = -I./${sourcedir} -I./${sourcedir}/default -I./${sourcedir}/test 

src/default/spiffs_config.h

@@ -17,6 +17,9 @@
 #include <string.h>
 #include <stddef.h>
 #include <unistd.h>
+#ifdef _SPIFFS_TEST
+#include "testrunner.h"
+#endif
 // ----------- >8 ------------
 
 // compile time switches

src/spiffs.h

@@ -60,6 +60,9 @@ extern "C" {
 #define SPIFFS_ERR_IX_MAP_MAPPED        -10038
 #define SPIFFS_ERR_IX_MAP_BAD_RANGE     -10039
 
+#define SPIFFS_ERR_SEEK_BOUNDS          -10040
+
+
 #define SPIFFS_ERR_INTERNAL             -10050
 
 #define SPIFFS_ERR_TEST                 -10100

src/spiffs_cache.c

@@ -137,7 +137,7 @@ s32_t spiffs_phys_rd(
 #endif
     cp->last_access = cache->last_access;
     u8_t *mem =  spiffs_get_cache_page(fs, cache, cp->ix);
-    memcpy(dst, &mem[SPIFFS_PADDR_TO_PAGE_OFFSET(fs, addr)], len);
+    _SPIFFS_MEMCPY(dst, &mem[SPIFFS_PADDR_TO_PAGE_OFFSET(fs, addr)], len);
   } else {
     if ((op & SPIFFS_OP_TYPE_MASK) == SPIFFS_OP_T_OBJ_LU2) {
       // for second layer lookup functions, we do not cache in order to prevent shredding
@@ -165,7 +165,7 @@ s32_t spiffs_phys_rd(
         res = res2;
       }
       u8_t *mem =  spiffs_get_cache_page(fs, cache, cp->ix);
-      memcpy(dst, &mem[SPIFFS_PADDR_TO_PAGE_OFFSET(fs, addr)], len);
+      _SPIFFS_MEMCPY(dst, &mem[SPIFFS_PADDR_TO_PAGE_OFFSET(fs, addr)], len);
     } else {
       // this will never happen, last resort for sake of symmetry
       s32_t res2 = SPIFFS_HAL_READ(fs, addr, len, dst);
@@ -203,7 +203,7 @@ s32_t spiffs_phys_wr(
     }
 
     u8_t *mem =  spiffs_get_cache_page(fs, cache, cp->ix);
-    memcpy(&mem[SPIFFS_PADDR_TO_PAGE_OFFSET(fs, addr)], src, len);
+    _SPIFFS_MEMCPY(&mem[SPIFFS_PADDR_TO_PAGE_OFFSET(fs, addr)], src, len);
 
     cache->last_access++;
     cp->last_access = cache->last_access;
@@ -302,7 +302,7 @@ void spiffs_cache_init(spiffs *fs) {
 
   cache.cpage_use_map = 0xffffffff;
   cache.cpage_use_mask = cache_mask;
-  memcpy(fs->cache, &cache, sizeof(spiffs_cache));
+  _SPIFFS_MEMCPY(fs->cache, &cache, sizeof(spiffs_cache));
 
   spiffs_cache *c = spiffs_get_cache(fs);
 

src/spiffs_hydrogen.c

@@ -86,7 +86,7 @@ s32_t SPIFFS_mount(spiffs *fs, spiffs_config *config, u8_t *work,
   SPIFFS_LOCK(fs);
   user_data = fs->user_data;
   memset(fs, 0, sizeof(spiffs));
-  memcpy(&fs->cfg, config, sizeof(spiffs_config));
+  _SPIFFS_MEMCPY(&fs->cfg, config, sizeof(spiffs_config));
   fs->user_data = user_data;
   fs->block_count = SPIFFS_CFG_PHYS_SZ(fs) / SPIFFS_CFG_LOG_BLOCK_SZ(fs);
   fs->work = &work[0];
@@ -543,7 +543,18 @@ s32_t SPIFFS_write(spiffs *fs, spiffs_file fh, void *buf, s32_t len) {
             offset, offset_in_cpage, len);
         spiffs_cache *cache = spiffs_get_cache(fs);
         u8_t *cpage_data = spiffs_get_cache_page(fs, cache, fd->cache_page->ix);
-        memcpy(&cpage_data[offset_in_cpage], buf, len);
+#ifdef _SPIFFS_TEST
+        {
+          intptr_t __a1 = (u8_t*)&cpage_data[offset_in_cpage]-(u8_t*)cache;
+          intptr_t __a2 = (u8_t*)&cpage_data[offset_in_cpage]+len-(u8_t*)cache;
+          intptr_t __b = sizeof(spiffs_cache) + cache->cpage_count * (sizeof(spiffs_cache_page) + SPIFFS_CFG_LOG_PAGE_SZ(fs));
+          if (__a1 > __b || __a2 > __b) {
+            printf("FATAL OOB: CACHE_WR: memcpy to cache buffer ixs:%4ld..%4ld of %4ld\n", __a1, __a2, __b);
+            ERREXIT();
+          }
+        }
+#endif
+        _SPIFFS_MEMCPY(&cpage_data[offset_in_cpage], buf, len);
         fd->cache_page->size = MAX(fd->cache_page->size, offset_in_cpage + len);
         fd->fdoffset += len;
         SPIFFS_UNLOCK(fs);
@@ -583,7 +594,7 @@ s32_t SPIFFS_write(spiffs *fs, spiffs_file fh, void *buf, s32_t len) {
 }
 
 s32_t SPIFFS_lseek(spiffs *fs, spiffs_file fh, s32_t offs, int whence) {
-  SPIFFS_API_DBG("%s "_SPIPRIfd " "_SPIPRIi " "_SPIPRIi "\n", __func__, fh, offs, whence);
+  SPIFFS_API_DBG("%s "_SPIPRIfd " "_SPIPRIi " %s\n", __func__, fh, offs, (const char* []){"SET","CUR","END","???"}[MIN(whence,3)]);
   SPIFFS_API_CHECK_CFG(fs);
   SPIFFS_API_CHECK_MOUNT(fs);
   SPIFFS_LOCK(fs);
@@ -598,19 +609,21 @@ s32_t SPIFFS_lseek(spiffs *fs, spiffs_file fh, s32_t offs, int whence) {
   spiffs_fflush_cache(fs, fh);
 #endif
 
-  s32_t fileSize = fd->size == SPIFFS_UNDEFINED_LEN ? 0 : fd->size;
+  s32_t file_size = fd->size == SPIFFS_UNDEFINED_LEN ? 0 : fd->size;
 
   switch (whence) {
   case SPIFFS_SEEK_CUR:
     offs = fd->fdoffset+offs;
     break;
   case SPIFFS_SEEK_END:
-    offs = fileSize + offs;
+    offs = file_size + offs;
     break;
   }
-
-  if (offs > fileSize) {
-    fd->fdoffset = fileSize;
+  if (offs < 0) {
+    SPIFFS_API_CHECK_RES_UNLOCK(fs, SPIFFS_ERR_SEEK_BOUNDS);
+  }
+  if (offs > file_size) {
+    fd->fdoffset = file_size;
     res = SPIFFS_ERR_END_OF_OBJECT;
   }
   SPIFFS_API_CHECK_RES_UNLOCK(fs, res);
@@ -730,7 +743,7 @@ static s32_t spiffs_stat_pix(spiffs *fs, spiffs_page_ix pix, spiffs_file fh, spi
   s->pix = pix;
   strncpy((char *)s->name, (char *)objix_hdr.name, SPIFFS_OBJ_NAME_LEN);
 #if SPIFFS_OBJ_META_LEN
-  memcpy(s->meta, objix_hdr.meta, SPIFFS_OBJ_META_LEN);
+  _SPIFFS_MEMCPY(s->meta, objix_hdr.meta, SPIFFS_OBJ_META_LEN);
 #endif
 
   return res;
@@ -1033,7 +1046,7 @@ static s32_t spiffs_read_dir_v(
     e->size = objix_hdr.size == SPIFFS_UNDEFINED_LEN ? 0 : objix_hdr.size;
     e->pix = pix;
 #if SPIFFS_OBJ_META_LEN
-  memcpy(e->meta, objix_hdr.meta, SPIFFS_OBJ_META_LEN);
+    _SPIFFS_MEMCPY(e->meta, objix_hdr.meta, SPIFFS_OBJ_META_LEN);
 #endif
     return SPIFFS_OK;
   }

src/spiffs_nucleus.c

@@ -270,7 +270,7 @@ s32_t spiffs_probe(
   s32_t res;
   u32_t paddr;
   spiffs dummy_fs; // create a dummy fs struct just to be able to use macros
-  memcpy(&dummy_fs.cfg, cfg, sizeof(spiffs_config));
+  _SPIFFS_MEMCPY(&dummy_fs.cfg, cfg, sizeof(spiffs_config));
   dummy_fs.block_count = 0;
 
   // Read three magics, as one block may be in an aborted erase state.
@@ -942,7 +942,7 @@ s32_t spiffs_object_create(
   strncpy((char*)oix_hdr.name, (const char*)name, SPIFFS_OBJ_NAME_LEN);
 #if SPIFFS_OBJ_META_LEN
   if (meta) {
-    memcpy(oix_hdr.meta, meta, SPIFFS_OBJ_META_LEN);
+    _SPIFFS_MEMCPY(oix_hdr.meta, meta, SPIFFS_OBJ_META_LEN);
   } else {
     memset(oix_hdr.meta, 0xff, SPIFFS_OBJ_META_LEN);
   }
@@ -1006,7 +1006,7 @@ s32_t spiffs_object_update_index_hdr(
   }
 #if SPIFFS_OBJ_META_LEN
   if (meta) {
-    memcpy(objix_hdr->meta, meta, SPIFFS_OBJ_META_LEN);
+    _SPIFFS_MEMCPY(objix_hdr->meta, meta, SPIFFS_OBJ_META_LEN);
   }
 #else
   (void) meta;
@@ -1298,7 +1298,7 @@ s32_t spiffs_object_append(spiffs_fd *fd, u32_t offset, u8_t *data, u32_t len) {
           SPIFFS_CHECK_RES(res);
           // quick "load" of new object index page
           memset(fs->work, 0xff, SPIFFS_CFG_LOG_PAGE_SZ(fs));
-          memcpy(fs->work, &p_hdr, sizeof(spiffs_page_header));
+          _SPIFFS_MEMCPY(fs->work, &p_hdr, sizeof(spiffs_page_header));
           spiffs_cb_object_event(fs, (spiffs_page_object_ix *)fs->work,
               SPIFFS_EV_IX_NEW, fd->obj_id, cur_objix_spix, cur_objix_pix, 0);
           SPIFFS_DBG("append: "_SPIPRIid" create objix page, "_SPIPRIpg":"_SPIPRIsp", written "_SPIPRIi"\n", fd->obj_id

src/spiffs_nucleus.h

@@ -803,4 +803,24 @@ s32_t spiffs_page_consistency_check(
 s32_t spiffs_object_index_consistency_check(
     spiffs *fs);
 
+// memcpy macro,
+// checked in test builds, otherwise plain memcpy (unless already defined)
+#ifdef _SPIFFS_TEST
+#define _SPIFFS_MEMCPY(__d, __s, __l) do { \
+    intptr_t __a1 = (intptr_t)((u8_t*)(__s)); \
+    intptr_t __a2 = (intptr_t)((u8_t*)(__s)+(__l)); \
+    intptr_t __b1 = (intptr_t)((u8_t*)(__d)); \
+    intptr_t __b2 = (intptr_t)((u8_t*)(__d)+(__l)); \
+    if (__a1 <= __b2 && __b1 <= __a2) { \
+      printf("FATAL OVERLAP: memcpy from %lx..%lx to %lx..%lx\n", __a1, __a2, __b1, __b2); \
+      ERREXIT(); \
+    } \
+    memcpy((__d),(__s),(__l)); \
+} while (0)
+#else
+#ifndef _SPIFFS_MEMCPY
+#define _SPIFFS_MEMCPY(__d, __s, __l) do{memcpy((__d),(__s),(__l));}while(0)
+#endif
+#endif //_SPIFFS_TEST
+
 #endif /* SPIFFS_NUCLEUS_H_ */

src/test/test_hydrogen.c

@@ -1589,6 +1589,33 @@ TEST(lseek_read) {
 TEST_END
 
 
+
+TEST(lseek_oob) {
+  int res;
+  spiffs_file fd;
+  char *fname = "seekfile";
+  int len = (FS_PURE_DATA_PAGES(FS) / 2) * SPIFFS_DATA_PAGE_SIZE(FS);
+
+  fd = SPIFFS_open(FS, fname, SPIFFS_TRUNC | SPIFFS_CREAT | SPIFFS_RDWR, 0);
+  TEST_CHECK(fd > 0);
+  u8_t *refbuf = malloc(len);
+  memrand(refbuf, len);
+  res = SPIFFS_write(FS, fd, refbuf, len);
+  TEST_CHECK(res >= 0);
+
+  int offs = 0;
+  res = SPIFFS_lseek(FS, fd, -1, SPIFFS_SEEK_SET);
+  TEST_CHECK_EQ(res, SPIFFS_ERR_SEEK_BOUNDS);
+  res = SPIFFS_lseek(FS, fd, len+1, SPIFFS_SEEK_SET);
+  TEST_CHECK_EQ(res, SPIFFS_ERR_END_OF_OBJECT);
+  free(refbuf);
+  SPIFFS_close(FS, fd);
+
+  return TEST_RES_OK;
+}
+TEST_END
+
+
 TEST(gc_quick)
 {
   char name[32];
@@ -2459,6 +2486,7 @@ SUITE_TESTS(hydrogen_tests)
   ADD_TEST(lseek_modification_append)
   ADD_TEST(lseek_modification_append_multi)
   ADD_TEST(lseek_read)
+  ADD_TEST(lseek_oob)
   ADD_TEST(gc_quick)
   ADD_TEST(write_small_file_chunks_1)
   ADD_TEST(write_small_files_chunks_1)

src/test/testrunner.h

@@ -155,7 +155,7 @@ int get_abort_on_error(void);
 int get_error_count(void);
 void inc_error_count(void);
 
-void add_suites();
+void add_suites(void);
 void test_init(void (*on_stop)(test *t));
 // returns 0 if all tests ok, -1 if any test failed, -2 on badness
 int run_tests(int argc, char **args);

src/test/testsuites.c

@@ -7,7 +7,7 @@
 
 #include "testrunner.h"
 
-void add_suites() {
+void add_suites(void) {
   //ADD_SUITE(dev_tests);
   ADD_SUITE(check_tests);
   ADD_SUITE(hydrogen_tests);