Fixed seek undefined behavior on signed integer overflow

In the previous implementation of lfs_file_seek, we calculated the new offset using signed arithmetic before checking for possible overflow/underflow conditions. This results in undefined behavior in C. Fortunately for us, littlefs is now limited to 31-bit file sizes for API reasons, so we don't have to be too clever here. Doing the arithmetic with unsigned integers and just checking if we're in a valid range afterwards should work. Found by m-kostrzewa and lucic71
Added some tests around seek integer overflow/underflow
2024-09-24 14:01:20 -05:00 · 2024-09-24 14:01:08 -05:00 · 2024-09-24 12:25:04 -05:00 · 2024-09-20 16:05:15 -05:00 · 2024-09-20 16:05:12 -05:00 · 2024-09-20 16:04:35 -05:00
10 changed files with 328 additions and 115 deletions
--- a/.github/workflows/release.yml
+++ b/.github/workflows/release.yml
@@ -20,7 +20,7 @@ jobs:
      github.event.workflow_run.head_sha == github.sha}}

    steps:
-      - uses: actions/checkout@v2
+      - uses: actions/checkout@v4
        with:
          ref: ${{github.event.workflow_run.head_sha}}
          # need workflow access since we push branches
@@ -30,26 +30,29 @@ jobs:
          fetch-depth: 0

      # try to get results from tests
-      - uses: dawidd6/action-download-artifact@v2
+      - uses: actions/download-artifact@v4
        continue-on-error: true
        with:
-          workflow: ${{github.event.workflow_run.name}}
-          run_id: ${{github.event.workflow_run.id}}
-          name: sizes
+          github-token: ${{secrets.GITHUB_TOKEN}}
+          run-id: ${{github.event.workflow_run.id}}
+          pattern: '{sizes,sizes-*}'
+          merge-multiple: true
          path: sizes
-      - uses: dawidd6/action-download-artifact@v2
+      - uses: actions/download-artifact@v4
        continue-on-error: true
        with:
-          workflow: ${{github.event.workflow_run.name}}
-          run_id: ${{github.event.workflow_run.id}}
-          name: cov
+          github-token: ${{secrets.GITHUB_TOKEN}}
+          run-id: ${{github.event.workflow_run.id}}
+          pattern: '{cov,cov-*}'
+          merge-multiple: true
          path: cov
-      - uses: dawidd6/action-download-artifact@v2
+      - uses: actions/download-artifact@v4
        continue-on-error: true
        with:
-          workflow: ${{github.event.workflow_run.name}}
-          run_id: ${{github.event.workflow_run.id}}
-          name: bench
+          github-token: ${{secrets.GITHUB_TOKEN}}
+          run-id: ${{github.event.workflow_run.id}}
+          pattern: '{bench,bench-*}'
+          merge-multiple: true
          path: bench

      - name: find-version
--- a/.github/workflows/status.yml
+++ b/.github/workflows/status.yml
@@ -13,12 +13,13 @@ jobs:
  status:
    runs-on: ubuntu-latest
    steps:
-      - uses: dawidd6/action-download-artifact@v2
+      - uses: actions/download-artifact@v4
        continue-on-error: true
        with:
-          workflow: ${{github.event.workflow_run.name}}
-          run_id: ${{github.event.workflow_run.id}}
-          name: status
+          github-token: ${{secrets.GITHUB_TOKEN}}
+          run-id: ${{github.event.workflow_run.id}}
+          pattern: '{status,status-*}'
+          merge-multiple: true
          path: status
      - name: update-status
        continue-on-error: true
@@ -67,12 +68,13 @@ jobs:

    steps:
      # generated comment?
-      - uses: dawidd6/action-download-artifact@v2
+      - uses: actions/download-artifact@v4
        continue-on-error: true
        with:
-          workflow: ${{github.event.workflow_run.name}}
-          run_id: ${{github.event.workflow_run.id}}
-          name: comment
+          github-token: ${{secrets.GITHUB_TOKEN}}
+          run-id: ${{github.event.workflow_run.id}}
+          pattern: '{comment,comment-*}'
+          merge-multiple: true
          path: comment
      - name: update-comment
        continue-on-error: true
--- a/.github/workflows/test.yml
+++ b/.github/workflows/test.yml
@@ -21,7 +21,7 @@ jobs:
        arch: [x86_64, thumb, mips, powerpc]

    steps:
-      - uses: actions/checkout@v2
+      - uses: actions/checkout@v4
      - name: install
        run: |
          # need a few things
@@ -235,9 +235,9 @@ jobs:

      # create size statuses
      - name: upload-sizes
-        uses: actions/upload-artifact@v2
+        uses: actions/upload-artifact@v4
        with:
-          name: sizes
+          name: sizes-${{matrix.arch}}
          path: sizes
      - name: status-sizes
        run: |
@@ -273,16 +273,17 @@ jobs:
            }' | tee status/$(basename $f .csv).json
          done
      - name: upload-status-sizes
-        uses: actions/upload-artifact@v2
+        if: ${{matrix.arch == 'x86_64'}}
+        uses: actions/upload-artifact@v4
        with:
-          name: status
+          name: status-sizes-${{matrix.arch}}
          path: status
          retention-days: 1

      # create cov statuses
      - name: upload-cov
        if: ${{matrix.arch == 'x86_64'}}
-        uses: actions/upload-artifact@v2
+        uses: actions/upload-artifact@v4
        with:
          name: cov
          path: cov
@@ -317,11 +318,11 @@ jobs:
              target_step: env.STEP,
            }' | tee status/$(basename $f .csv)-$s.json
          done
-      - name: upload-status-sizes
+      - name: upload-status-cov
        if: ${{matrix.arch == 'x86_64'}}
-        uses: actions/upload-artifact@v2
+        uses: actions/upload-artifact@v4
        with:
-          name: status
+          name: status-cov
          path: status
          retention-days: 1

@@ -336,7 +337,7 @@ jobs:
        pls: [1, 2]

    steps:
-      - uses: actions/checkout@v2
+      - uses: actions/checkout@v4
      - name: install
        run: |
          # need a few things
@@ -361,7 +362,7 @@ jobs:
  test-no-intrinsics:
    runs-on: ubuntu-latest
    steps:
-      - uses: actions/checkout@v2
+      - uses: actions/checkout@v4
      - name: install
        run: |
          # need a few things
@@ -378,7 +379,7 @@ jobs:
  test-multiversion:
    runs-on: ubuntu-latest
    steps:
-      - uses: actions/checkout@v2
+      - uses: actions/checkout@v4
      - name: install
        run: |
          # need a few things
@@ -395,7 +396,7 @@ jobs:
  test-lfs2_0:
    runs-on: ubuntu-latest
    steps:
-      - uses: actions/checkout@v2
+      - uses: actions/checkout@v4
      - name: install
        run: |
          # need a few things
@@ -414,7 +415,7 @@ jobs:
  test-valgrind:
    runs-on: ubuntu-latest
    steps:
-      - uses: actions/checkout@v2
+      - uses: actions/checkout@v4
      - name: install
        run: |
          # need a few things
@@ -436,7 +437,7 @@ jobs:
  test-clang:
    runs-on: ubuntu-latest
    steps:
-      - uses: actions/checkout@v2
+      - uses: actions/checkout@v4
      - name: install
        run: |
          # need a few things
@@ -459,7 +460,7 @@ jobs:
  bench:
    runs-on: ubuntu-latest
    steps:
-      - uses: actions/checkout@v2
+      - uses: actions/checkout@v4
      - name: install
        run: |
          # need a few things
@@ -491,7 +492,7 @@ jobs:

      # create bench statuses
      - name: upload-bench
-        uses: actions/upload-artifact@v2
+        uses: actions/upload-artifact@v4
        with:
          name: bench
          path: bench
@@ -525,9 +526,9 @@ jobs:
            }' | tee status/$(basename $f .csv)-$s.json
          done
      - name: upload-status-bench
-        uses: actions/upload-artifact@v2
+        uses: actions/upload-artifact@v4
        with:
-          name: status
+          name: status-bench
          path: status
          retention-days: 1

@@ -535,10 +536,10 @@ jobs:
  test-compat:
    runs-on: ubuntu-latest
    steps:
-      - uses: actions/checkout@v2
+      - uses: actions/checkout@v4
        if: ${{github.event_name == 'pull_request'}}
      # checkout the current pr target into lfsp
-      - uses: actions/checkout@v2
+      - uses: actions/checkout@v4
        if: ${{github.event_name == 'pull_request'}}
        with:
          ref: ${{github.event.pull_request.base.ref}}
@@ -572,7 +573,7 @@ jobs:
    runs-on: ubuntu-latest
    if: ${{!endsWith(github.ref, '-prefix')}}
    steps:
-      - uses: actions/checkout@v2
+      - uses: actions/checkout@v4
      - name: install
        run: |
          # need a few things
@@ -582,7 +583,7 @@ jobs:
          gcc --version
          python3 --version
          fusermount -V
-      - uses: actions/checkout@v2
+      - uses: actions/checkout@v4
        with:
          repository: littlefs-project/littlefs-fuse
          ref: v2
@@ -622,7 +623,7 @@ jobs:
    runs-on: ubuntu-latest
    if: ${{!endsWith(github.ref, '-prefix')}}
    steps:
-      - uses: actions/checkout@v2
+      - uses: actions/checkout@v4
      - name: install
        run: |
          # need a few things
@@ -632,12 +633,12 @@ jobs:
          gcc --version
          python3 --version
          fusermount -V
-      - uses: actions/checkout@v2
+      - uses: actions/checkout@v4
        with:
          repository: littlefs-project/littlefs-fuse
          ref: v2
          path: v2
-      - uses: actions/checkout@v2
+      - uses: actions/checkout@v4
        with:
          repository: littlefs-project/littlefs-fuse
          ref: v1
@@ -694,7 +695,7 @@ jobs:
    runs-on: ubuntu-latest
    needs: [test, bench]
    steps:
-      - uses: actions/checkout@v2
+      - uses: actions/checkout@v4
        if: ${{github.event_name == 'pull_request'}}
      - name: install
        if: ${{github.event_name == 'pull_request'}}
@@ -704,23 +705,26 @@ jobs:
          pip3 install toml
          gcc --version
          python3 --version
-      - uses: actions/download-artifact@v2
+      - uses: actions/download-artifact@v4
        if: ${{github.event_name == 'pull_request'}}
        continue-on-error: true
        with:
-          name: sizes
+          pattern: '{sizes,sizes-*}'
+          merge-multiple: true
          path: sizes
-      - uses: actions/download-artifact@v2
+      - uses: actions/download-artifact@v4
        if: ${{github.event_name == 'pull_request'}}
        continue-on-error: true
        with:
-          name: cov
+          pattern: '{cov,cov-*}'
+          merge-multiple: true
          path: cov
-      - uses: actions/download-artifact@v2
+      - uses: actions/download-artifact@v4
        if: ${{github.event_name == 'pull_request'}}
        continue-on-error: true
        with:
-          name: bench
+          pattern: '{bench,bench-*}'
+          merge-multiple: true
          path: bench

      # try to find results from tests
@@ -862,7 +866,7 @@ jobs:
            body: $comment,
          }' | tee comment/comment.json
      - name: upload-comment
-        uses: actions/upload-artifact@v2
+        uses: actions/upload-artifact@v4
        with:
          name: comment
          path: comment
--- a/README.md
+++ b/README.md
@@ -231,11 +231,25 @@ License Identifiers that are here available: http://spdx.org/licenses/
  to use littlefs in a Rust-friendly API, reaping the benefits of Rust's memory
  safety and other guarantees.

+- [nim-littlefs] - A Nim wrapper and API for littlefs. Includes a fuse
+  implementation based on [littlefs-fuse]
+
+- [chamelon] - A pure-OCaml implementation of (most of) littlefs, designed for
+  use with the MirageOS library operating system project. It is interoperable
+  with the reference implementation, with some caveats.
+
 - [littlefs-disk-img-viewer] - A memory-efficient web application for viewing
  littlefs disk images in your web browser.

- [mklfs] - A command line tool built by the [Lua RTOS] guys for making
-  littlefs images from a host PC. Supports Windows, Mac OS, and Linux.
+- [mklfs] - A command line tool for creating littlefs images. Used in the Lua
+  RTOS ecosystem.
+
+- [mklittlefs] - A command line tool for creating littlefs images. Used in the
+  ESP8266 and RP2040 ecosystem.
+
+- [pico-littlefs-usb] - An interface for littlefs that emulates a FAT12
+  filesystem over USB. Allows mounting littlefs on a host PC without additional
+  drivers.

 - [Mbed OS] - The easiest way to get started with littlefs is to jump into Mbed
  which already has block device drivers for most forms of embedded storage.
@@ -254,27 +268,21 @@ License Identifiers that are here available: http://spdx.org/licenses/
  for microcontroller-scale devices. Due to limitations of FAT it can't provide
  power-loss resilience, but it does allow easy interop with PCs.

- [chamelon] - A pure-OCaml implementation of (most of) littlefs, designed for
-  use with the MirageOS library operating system project. It is interoperable
-  with the reference implementation, with some caveats.
-
- [nim-littlefs] - A Nim wrapper and API for littlefs. Includes a fuse 
-  implementation based on [littlefs-fuse]
-
 [BSD-3-Clause]: https://spdx.org/licenses/BSD-3-Clause.html
-[littlefs-disk-img-viewer]: https://github.com/tniessen/littlefs-disk-img-viewer
 [littlefs-fuse]: https://github.com/geky/littlefs-fuse
 [FUSE]: https://github.com/libfuse/libfuse
 [littlefs-js]: https://github.com/geky/littlefs-js
 [littlefs-js-demo]:http://littlefs.geky.net/demo.html
+[littlefs-python]: https://pypi.org/project/littlefs-python/
+[littlefs2-rust]: https://crates.io/crates/littlefs2
+[nim-littlefs]: https://github.com/Graveflo/nim-littlefs
+[chamelon]: https://github.com/yomimono/chamelon
+[littlefs-disk-img-viewer]: https://github.com/tniessen/littlefs-disk-img-viewer
 [mklfs]: https://github.com/whitecatboard/Lua-RTOS-ESP32/tree/master/components/mklfs/src
-[Lua RTOS]: https://github.com/whitecatboard/Lua-RTOS-ESP32
+[mklittlefs]: https://github.com/earlephilhower/mklittlefs
+[pico-littlefs-usb]: https://github.com/oyama/pico-littlefs-usb
 [Mbed OS]: https://github.com/armmbed/mbed-os
 [LittleFileSystem]: https://os.mbed.com/docs/mbed-os/latest/apis/littlefilesystem.html
 [SPIFFS]: https://github.com/pellepl/spiffs
 [Dhara]: https://github.com/dlbeer/dhara
 [ChaN's FatFs]: http://elm-chan.org/fsw/ff/00index_e.html
-[littlefs-python]: https://pypi.org/project/littlefs-python/
-[littlefs2-rust]: https://crates.io/crates/littlefs2
-[chamelon]: https://github.com/yomimono/chamelon
-[nim-littlefs]: https://github.com/Graveflo/nim-littlefs
--- a/SPEC.md
+++ b/SPEC.md
@@ -441,9 +441,10 @@ Superblock fields:

 7. **Attr max (32-bits)** - Maximum size of file attributes in bytes.

-The superblock must always be the first entry (id 0) in a metadata pair as well
-as be the first entry written to the block. This means that the superblock
-entry can be read from a device using offsets alone.
+The superblock must always be the first entry (id 0) in the metadata pair, and
+the name tag must always be the first tag in the metadata pair. This makes it
+so that the magic string "littlefs" will always reside at offset=8 in a valid
+littlefs superblock.

 ---
 #### `0x2xx` LFS_TYPE_STRUCT
--- a/lfs.c
+++ b/lfs.c
@@ -688,7 +688,7 @@ static int lfs_alloc(lfs_t *lfs, lfs_block_t *block) {
        if (lfs->lookahead.ckpoint <= 0) {
            LFS_ERROR("No more free space 0x%"PRIx32,
                    (lfs->lookahead.start + lfs->lookahead.next)
-                        % lfs->cfg->block_count);
+                        % lfs->block_count);
            return LFS_ERR_NOSPC;
        }

@@ -2191,7 +2191,8 @@ static int lfs_dir_splittingcompact(lfs_t *lfs, lfs_mdir_t *dir,
                // we can do, we'll error later if we've become frozen
                LFS_WARN("Unable to expand superblock");
            } else {
-                end = begin;
+                // duplicate the superblock entry into the new superblock
+                end = 1;
            }
        }
    }
@@ -2358,7 +2359,9 @@ fixmlist:;

            while (d->id >= d->m.count && d->m.split) {
                // we split and id is on tail now
-                d->id -= d->m.count;
+                if (lfs_pair_cmp(d->m.tail, lfs->root) != 0) {
+                    d->id -= d->m.count;
+                }
                int err = lfs_dir_fetch(lfs, &d->m, d->m.tail);
                if (err) {
                    return err;
@@ -3661,22 +3664,16 @@ static lfs_ssize_t lfs_file_write_(lfs_t *lfs, lfs_file_t *file,
 static lfs_soff_t lfs_file_seek_(lfs_t *lfs, lfs_file_t *file,
        lfs_soff_t off, int whence) {
    // find new pos
+    //
+    // fortunately for us, littlefs is limited to 31-bit file sizes, so we
+    // don't have to worry too much about integer overflow
    lfs_off_t npos = file->pos;
    if (whence == LFS_SEEK_SET) {
        npos = off;
    } else if (whence == LFS_SEEK_CUR) {
-        if ((lfs_soff_t)file->pos + off < 0) {
-            return LFS_ERR_INVAL;
-        } else {
-            npos = file->pos + off;
-        }
+        npos = file->pos + (lfs_off_t)off;
    } else if (whence == LFS_SEEK_END) {
-        lfs_soff_t res = lfs_file_size_(lfs, file) + off;
-        if (res < 0) {
-            return LFS_ERR_INVAL;
-        } else {
-            npos = res;
-        }
+        npos = (lfs_off_t)lfs_file_size_(lfs, file) + (lfs_off_t)off;
    }

    if (npos > lfs->file_max) {
@@ -4466,6 +4463,7 @@ static int lfs_mount_(lfs_t *lfs, const struct lfs_config *cfg) {
            // found older minor version? set an in-device only bit in the
            // gstate so we know we need to rewrite the superblock before
            // the first write
+            bool needssuperblock = false;
            if (minor_version < lfs_fs_disk_version_minor(lfs)) {
                LFS_DEBUG("Found older minor version "
                        "v%"PRIu16".%"PRIu16" < v%"PRIu16".%"PRIu16,
@@ -4473,10 +4471,11 @@ static int lfs_mount_(lfs_t *lfs, const struct lfs_config *cfg) {
                        minor_version,
                        lfs_fs_disk_version_major(lfs),
                        lfs_fs_disk_version_minor(lfs));
-                // note this bit is reserved on disk, so fetching more gstate
-                // will not interfere here
-                lfs_fs_prepsuperblock(lfs, true);
+                needssuperblock = true;
            }
+            // note this bit is reserved on disk, so fetching more gstate
+            // will not interfere here
+            lfs_fs_prepsuperblock(lfs, needssuperblock);

            // check superblock configuration
            if (superblock.name_max) {
--- a/lfs.h
+++ b/lfs.h
@@ -59,7 +59,8 @@ typedef uint32_t lfs_block_t;
 #endif

 // Maximum size of custom attributes in bytes, may be redefined, but there is
-// no real benefit to using a smaller LFS_ATTR_MAX. Limited to <= 1022.
+// no real benefit to using a smaller LFS_ATTR_MAX. Limited to <= 1022. Stored
+// in superblock and must be respected by other littlefs drivers.
 #ifndef LFS_ATTR_MAX
 #define LFS_ATTR_MAX 1022
 #endif
@@ -203,7 +204,8 @@ struct lfs_config {
    // program sizes.
    lfs_size_t block_size;

-    // Number of erasable blocks on the device.
+    // Number of erasable blocks on the device. Defaults to block_count stored
+    // on disk when zero.
    lfs_size_t block_count;

    // Number of erase cycles before littlefs evicts metadata logs and moves
@@ -252,18 +254,18 @@ struct lfs_config {

    // Optional upper limit on length of file names in bytes. No downside for
    // larger names except the size of the info struct which is controlled by
-    // the LFS_NAME_MAX define. Defaults to LFS_NAME_MAX when zero. Stored in
-    // superblock and must be respected by other littlefs drivers.
+    // the LFS_NAME_MAX define. Defaults to LFS_NAME_MAX or name_max stored on
+    // disk when zero.
    lfs_size_t name_max;

    // Optional upper limit on files in bytes. No downside for larger files
-    // but must be <= LFS_FILE_MAX. Defaults to LFS_FILE_MAX when zero. Stored
-    // in superblock and must be respected by other littlefs drivers.
+    // but must be <= LFS_FILE_MAX. Defaults to LFS_FILE_MAX or file_max stored
+    // on disk when zero.
    lfs_size_t file_max;

    // Optional upper limit on custom attributes in bytes. No downside for
    // larger attributes size but must be <= LFS_ATTR_MAX. Defaults to
-    // LFS_ATTR_MAX when zero.
+    // LFS_ATTR_MAX or attr_max stored on disk when zero.
    lfs_size_t attr_max;

    // Optional upper limit on total space given to metadata pairs in bytes. On
--- a/tests/test_alloc.toml
+++ b/tests/test_alloc.toml
@@ -8,17 +8,22 @@ defines.FILES = 3
 defines.SIZE = '(((BLOCK_SIZE-8)*(BLOCK_COUNT-6)) / FILES)'
 defines.GC = [false, true]
 defines.COMPACT_THRESH = ['-1', '0', 'BLOCK_SIZE/2']
+defines.INFER_BC = [false, true]
 code = '''
    const char *names[] = {"bacon", "eggs", "pancakes"};
    lfs_file_t files[FILES];

    lfs_t lfs;
    lfs_format(&lfs, cfg) => 0;
-    lfs_mount(&lfs, cfg) => 0;
+    struct lfs_config cfg_ = *cfg;
+    if (INFER_BC) {
+        cfg_.block_count = 0;
+    }
+    lfs_mount(&lfs, &cfg_) => 0;
    lfs_mkdir(&lfs, "breakfast") => 0;
    lfs_unmount(&lfs) => 0;

-    lfs_mount(&lfs, cfg) => 0;
+    lfs_mount(&lfs, &cfg_) => 0;
    for (int n = 0; n < FILES; n++) {
        char path[1024];
        sprintf(path, "breakfast/%s", names[n]);
@@ -39,7 +44,7 @@ code = '''
    }
    lfs_unmount(&lfs) => 0;

-    lfs_mount(&lfs, cfg) => 0;
+    lfs_mount(&lfs, &cfg_) => 0;
    for (int n = 0; n < FILES; n++) {
        char path[1024];
        sprintf(path, "breakfast/%s", names[n]);
@@ -62,17 +67,22 @@ defines.FILES = 3
 defines.SIZE = '(((BLOCK_SIZE-8)*(BLOCK_COUNT-6)) / FILES)'
 defines.GC = [false, true]
 defines.COMPACT_THRESH = ['-1', '0', 'BLOCK_SIZE/2']
+defines.INFER_BC = [false, true]
 code = '''
    const char *names[] = {"bacon", "eggs", "pancakes"};

    lfs_t lfs;
    lfs_format(&lfs, cfg) => 0;
-    lfs_mount(&lfs, cfg) => 0;
+    struct lfs_config cfg_ = *cfg;
+    if (INFER_BC) {
+        cfg_.block_count = 0;
+    }
+    lfs_mount(&lfs, &cfg_) => 0;
    lfs_mkdir(&lfs, "breakfast") => 0;
    lfs_unmount(&lfs) => 0;

    for (int n = 0; n < FILES; n++) {
-        lfs_mount(&lfs, cfg) => 0;
+        lfs_mount(&lfs, &cfg_) => 0;
        char path[1024];
        sprintf(path, "breakfast/%s", names[n]);
        lfs_file_t file;
@@ -91,7 +101,7 @@ code = '''
        lfs_unmount(&lfs) => 0;
    }

-    lfs_mount(&lfs, cfg) => 0;
+    lfs_mount(&lfs, &cfg_) => 0;
    for (int n = 0; n < FILES; n++) {
        char path[1024];
        sprintf(path, "breakfast/%s", names[n]);
@@ -113,19 +123,24 @@ code = '''
 defines.FILES = 3
 defines.SIZE = '(((BLOCK_SIZE-8)*(BLOCK_COUNT-6)) / FILES)'
 defines.CYCLES = [1, 10]
+defines.INFER_BC = [false, true]
 code = '''
    const char *names[] = {"bacon", "eggs", "pancakes"};
    lfs_file_t files[FILES];

    lfs_t lfs;
    lfs_format(&lfs, cfg) => 0;
+    struct lfs_config cfg_ = *cfg;
+    if (INFER_BC) {
+        cfg_.block_count = 0;
+    }

    for (int c = 0; c < CYCLES; c++) {
-        lfs_mount(&lfs, cfg) => 0;
+        lfs_mount(&lfs, &cfg_) => 0;
        lfs_mkdir(&lfs, "breakfast") => 0;
        lfs_unmount(&lfs) => 0;

-        lfs_mount(&lfs, cfg) => 0;
+        lfs_mount(&lfs, &cfg_) => 0;
        for (int n = 0; n < FILES; n++) {
            char path[1024];
            sprintf(path, "breakfast/%s", names[n]);
@@ -143,7 +158,7 @@ code = '''
        }
        lfs_unmount(&lfs) => 0;

-        lfs_mount(&lfs, cfg) => 0;
+        lfs_mount(&lfs, &cfg_) => 0;
        for (int n = 0; n < FILES; n++) {
            char path[1024];
            sprintf(path, "breakfast/%s", names[n]);
@@ -159,7 +174,7 @@ code = '''
        }
        lfs_unmount(&lfs) => 0;

-        lfs_mount(&lfs, cfg) => 0;
+        lfs_mount(&lfs, &cfg_) => 0;
        for (int n = 0; n < FILES; n++) {
            char path[1024];
            sprintf(path, "breakfast/%s", names[n]);
@@ -175,19 +190,24 @@ code = '''
 defines.FILES = 3
 defines.SIZE = '(((BLOCK_SIZE-8)*(BLOCK_COUNT-6)) / FILES)'
 defines.CYCLES = [1, 10]
+defines.INFER_BC = [false, true]
 code = '''
    const char *names[] = {"bacon", "eggs", "pancakes"};

    lfs_t lfs;
    lfs_format(&lfs, cfg) => 0;
+    struct lfs_config cfg_ = *cfg;
+    if (INFER_BC) {
+        cfg_.block_count = 0;
+    }

    for (int c = 0; c < CYCLES; c++) {
-        lfs_mount(&lfs, cfg) => 0;
+        lfs_mount(&lfs, &cfg_) => 0;
        lfs_mkdir(&lfs, "breakfast") => 0;
        lfs_unmount(&lfs) => 0;

        for (int n = 0; n < FILES; n++) {
-            lfs_mount(&lfs, cfg) => 0;
+            lfs_mount(&lfs, &cfg_) => 0;
            char path[1024];
            sprintf(path, "breakfast/%s", names[n]);
            lfs_file_t file;
@@ -232,10 +252,15 @@ code = '''

 # exhaustion test
 [cases.test_alloc_exhaustion]
+defines.INFER_BC = [false, true]
 code = '''
    lfs_t lfs;
    lfs_format(&lfs, cfg) => 0;
-    lfs_mount(&lfs, cfg) => 0;
+    struct lfs_config cfg_ = *cfg;
+    if (INFER_BC) {
+        cfg_.block_count = 0;
+    }
+    lfs_mount(&lfs, &cfg_) => 0;
    lfs_file_t file;
    lfs_file_open(&lfs, &file, "exhaustion", LFS_O_WRONLY | LFS_O_CREAT);
    size_t size = strlen("exhaustion");
@@ -263,7 +288,7 @@ code = '''
    lfs_file_close(&lfs, &file) => 0;
    lfs_unmount(&lfs) => 0;

-    lfs_mount(&lfs, cfg) => 0;
+    lfs_mount(&lfs, &cfg_) => 0;
    lfs_file_open(&lfs, &file, "exhaustion", LFS_O_RDONLY);
    size = strlen("exhaustion");
    lfs_file_size(&lfs, &file) => size;
@@ -276,10 +301,15 @@ code = '''
 # exhaustion wraparound test
 [cases.test_alloc_exhaustion_wraparound]
 defines.SIZE = '(((BLOCK_SIZE-8)*(BLOCK_COUNT-4)) / 3)'
+defines.INFER_BC = [false, true]
 code = '''
    lfs_t lfs;
    lfs_format(&lfs, cfg) => 0;
-    lfs_mount(&lfs, cfg) => 0;
+    struct lfs_config cfg_ = *cfg;
+    if (INFER_BC) {
+        cfg_.block_count = 0;
+    }
+    lfs_mount(&lfs, &cfg_) => 0;

    lfs_file_t file;
    lfs_file_open(&lfs, &file, "padding", LFS_O_WRONLY | LFS_O_CREAT);
@@ -317,7 +347,7 @@ code = '''
    lfs_file_close(&lfs, &file) => 0;
    lfs_unmount(&lfs) => 0;

-    lfs_mount(&lfs, cfg) => 0;
+    lfs_mount(&lfs, &cfg_) => 0;
    lfs_file_open(&lfs, &file, "exhaustion", LFS_O_RDONLY);
    size = strlen("exhaustion");
    lfs_file_size(&lfs, &file) => size;
@@ -330,10 +360,15 @@ code = '''

 # dir exhaustion test
 [cases.test_alloc_dir_exhaustion]
+defines.INFER_BC = [false, true]
 code = '''
    lfs_t lfs;
    lfs_format(&lfs, cfg) => 0;
-    lfs_mount(&lfs, cfg) => 0;
+    struct lfs_config cfg_ = *cfg;
+    if (INFER_BC) {
+        cfg_.block_count = 0;
+    }
+    lfs_mount(&lfs, &cfg_) => 0;

    // find out max file size
    lfs_mkdir(&lfs, "exhaustiondir") => 0;
--- a/tests/test_seek.toml
+++ b/tests/test_seek.toml
@@ -405,3 +405,111 @@ code = '''
    lfs_file_close(&lfs, &file) => 0;
    lfs_unmount(&lfs) => 0;
 '''
+
+
+# test possible overflow/underflow conditions
+#
+# note these need -fsanitize=undefined to consistently detect
+# overflow/underflow conditions
+
+[cases.test_seek_filemax]
+code = '''
+    lfs_t lfs;
+    lfs_format(&lfs, cfg) => 0;
+    lfs_mount(&lfs, cfg) => 0;
+    lfs_file_t file;
+    lfs_file_open(&lfs, &file, "kitty",
+            LFS_O_WRONLY | LFS_O_CREAT | LFS_O_APPEND) => 0;
+    uint8_t buffer[1024];
+    strcpy((char*)buffer, "kittycatcat");
+    size_t size = strlen((char*)buffer);
+    lfs_file_write(&lfs, &file, buffer, size) => size;
+
+    // seek with LFS_SEEK_SET
+    lfs_file_seek(&lfs, &file, LFS_FILE_MAX, LFS_SEEK_SET) => LFS_FILE_MAX;
+
+    // seek with LFS_SEEK_CUR
+    lfs_file_seek(&lfs, &file, 0, LFS_SEEK_CUR) => LFS_FILE_MAX;
+
+    // the file hasn't changed size, so seek end takes us back to the offset=0
+    lfs_file_seek(&lfs, &file, +10, LFS_SEEK_END) => size+10;
+
+    lfs_file_close(&lfs, &file) => 0;
+    lfs_unmount(&lfs) => 0;
+'''
+
+[cases.test_seek_underflow]
+code = '''
+    lfs_t lfs;
+    lfs_format(&lfs, cfg) => 0;
+    lfs_mount(&lfs, cfg) => 0;
+    lfs_file_t file;
+    lfs_file_open(&lfs, &file, "kitty",
+            LFS_O_WRONLY | LFS_O_CREAT | LFS_O_APPEND) => 0;
+    uint8_t buffer[1024];
+    strcpy((char*)buffer, "kittycatcat");
+    size_t size = strlen((char*)buffer);
+    lfs_file_write(&lfs, &file, buffer, size) => size;
+
+    // underflow with LFS_SEEK_CUR, should error
+    lfs_file_seek(&lfs, &file, -(size+10), LFS_SEEK_CUR) => LFS_ERR_INVAL;
+    lfs_file_seek(&lfs, &file, -LFS_FILE_MAX, LFS_SEEK_CUR) => LFS_ERR_INVAL;
+    lfs_file_seek(&lfs, &file, -(size+LFS_FILE_MAX), LFS_SEEK_CUR)
+            => LFS_ERR_INVAL;
+
+    // underflow with LFS_SEEK_END, should error
+    lfs_file_seek(&lfs, &file, -(size+10), LFS_SEEK_END) => LFS_ERR_INVAL;
+    lfs_file_seek(&lfs, &file, -LFS_FILE_MAX, LFS_SEEK_END) => LFS_ERR_INVAL;
+    lfs_file_seek(&lfs, &file, -(size+LFS_FILE_MAX), LFS_SEEK_END)
+            => LFS_ERR_INVAL;
+
+    // file pointer should not have changed
+    lfs_file_tell(&lfs, &file) => size;
+
+    lfs_file_close(&lfs, &file) => 0;
+    lfs_unmount(&lfs) => 0;
+'''
+
+[cases.test_seek_overflow]
+code = '''
+    lfs_t lfs;
+    lfs_format(&lfs, cfg) => 0;
+    lfs_mount(&lfs, cfg) => 0;
+    lfs_file_t file;
+    lfs_file_open(&lfs, &file, "kitty",
+            LFS_O_WRONLY | LFS_O_CREAT | LFS_O_APPEND) => 0;
+    uint8_t buffer[1024];
+    strcpy((char*)buffer, "kittycatcat");
+    size_t size = strlen((char*)buffer);
+    lfs_file_write(&lfs, &file, buffer, size) => size;
+
+    // seek to LFS_FILE_MAX
+    lfs_file_seek(&lfs, &file, LFS_FILE_MAX, LFS_SEEK_SET) => LFS_FILE_MAX;
+
+    // overflow with LFS_SEEK_CUR, should error
+    lfs_file_seek(&lfs, &file, +10, LFS_SEEK_CUR) => LFS_ERR_INVAL;
+    lfs_file_seek(&lfs, &file, +LFS_FILE_MAX, LFS_SEEK_CUR) => LFS_ERR_INVAL;
+
+    // LFS_SEEK_SET/END don't care about the current file position, but we can
+    // still overflow with a large offset
+
+    // overflow with LFS_SEEK_SET, should error
+    lfs_file_seek(&lfs, &file,
+            +((uint32_t)LFS_FILE_MAX+10),
+            LFS_SEEK_SET) => LFS_ERR_INVAL;
+    lfs_file_seek(&lfs, &file,
+            +((uint32_t)LFS_FILE_MAX+(uint32_t)LFS_FILE_MAX),
+            LFS_SEEK_SET) => LFS_ERR_INVAL;
+
+    // overflow with LFS_SEEK_END, should error
+    lfs_file_seek(&lfs, &file, +(LFS_FILE_MAX-size+10), LFS_SEEK_END)
+            => LFS_ERR_INVAL;
+    lfs_file_seek(&lfs, &file, +(LFS_FILE_MAX-size+LFS_FILE_MAX), LFS_SEEK_END)
+            => LFS_ERR_INVAL;
+
+    // file pointer should not have changed
+    lfs_file_tell(&lfs, &file) => LFS_FILE_MAX;
+
+    lfs_file_close(&lfs, &file) => 0;
+    lfs_unmount(&lfs) => 0;
+'''
--- a/tests/test_superblocks.toml
+++ b/tests/test_superblocks.toml
@@ -14,6 +14,24 @@ code = '''
    lfs_unmount(&lfs) => 0;
 '''

+# make sure the magic string "littlefs" is always at offset=8
+[cases.test_superblocks_magic]
+code = '''
+    lfs_t lfs;
+    lfs_format(&lfs, cfg) => 0;
+
+    // check our magic string
+    //
+    // note if we lose power we may not have the magic string in both blocks!
+    // but we don't lose power in this test so we can assert the magic string
+    // is present in both
+    uint8_t magic[lfs_max(16, READ_SIZE)];
+    cfg->read(cfg, 0, 0, magic, lfs_max(16, READ_SIZE)) => 0;
+    assert(memcmp(&magic[8], "littlefs", 8) == 0);
+    cfg->read(cfg, 1, 0, magic, lfs_max(16, READ_SIZE)) => 0;
+    assert(memcmp(&magic[8], "littlefs", 8) == 0);
+'''
+
 # mount/unmount from interpretting a previous superblock block_count
 [cases.test_superblocks_mount_unknown_block_count]
 code = '''
@@ -28,7 +46,6 @@ code = '''
    lfs_unmount(&lfs) => 0;
 '''

-
 # reentrant format
 [cases.test_superblocks_reentrant_format]
 reentrant = true
@@ -135,6 +152,39 @@ code = '''
    lfs_unmount(&lfs) => 0;
 '''

+# make sure the magic string "littlefs" is always at offset=8
+[cases.test_superblocks_magic_expand]
+defines.BLOCK_CYCLES = [32, 33, 1]
+defines.N = [10, 100, 1000]
+code = '''
+    lfs_t lfs;
+    lfs_format(&lfs, cfg) => 0;
+    lfs_mount(&lfs, cfg) => 0;
+    for (int i = 0; i < N; i++) {
+        lfs_file_t file;
+        lfs_file_open(&lfs, &file, "dummy",
+                LFS_O_WRONLY | LFS_O_CREAT | LFS_O_EXCL) => 0;
+        lfs_file_close(&lfs, &file) => 0;
+        struct lfs_info info;
+        lfs_stat(&lfs, "dummy", &info) => 0;
+        assert(strcmp(info.name, "dummy") == 0);
+        assert(info.type == LFS_TYPE_REG);
+        lfs_remove(&lfs, "dummy") => 0;
+    }
+    lfs_unmount(&lfs) => 0;
+
+    // check our magic string
+    //
+    // note if we lose power we may not have the magic string in both blocks!
+    // but we don't lose power in this test so we can assert the magic string
+    // is present in both
+    uint8_t magic[lfs_max(16, READ_SIZE)];
+    cfg->read(cfg, 0, 0, magic, lfs_max(16, READ_SIZE)) => 0;
+    assert(memcmp(&magic[8], "littlefs", 8) == 0);
+    cfg->read(cfg, 1, 0, magic, lfs_max(16, READ_SIZE)) => 0;
+    assert(memcmp(&magic[8], "littlefs", 8) == 0);
+'''
+
 # expanding superblock with power cycle
 [cases.test_superblocks_expand_power_cycle]
 defines.BLOCK_CYCLES = [32, 33, 1]
@@ -221,6 +271,7 @@ code = '''
    lfs_unmount(&lfs) => 0;
 '''

+
 # mount with unknown block_count
 [cases.test_superblocks_unknown_blocks]
 code = '''
Author	SHA1	Message	Date
Christopher Haster	abaec45652	Fixed seek undefined behavior on signed integer overflow In the previous implementation of lfs_file_seek, we calculated the new offset using signed arithmetic before checking for possible overflow/underflow conditions. This results in undefined behavior in C. Fortunately for us, littlefs is now limited to 31-bit file sizes for API reasons, so we don't have to be too clever here. Doing the arithmetic with unsigned integers and just checking if we're in a valid range afterwards should work. Found by m-kostrzewa and lucic71	2024-09-24 14:01:20 -05:00
Christopher Haster	f1c430e779	Added some tests around seek integer overflow/underflow Original tests provided by m-kostrzewa, these identify signed overflow (undefined behavior) when compiled with -fsanitize=undefined.	2024-09-24 14:01:08 -05:00
Christopher Haster	b78afe2518	Merge pull request #1026 from yamt/update-gh-actions Update github actions to the latest versions	2024-09-24 12:25:04 -05:00
Christopher Haster	798073c2a7	gha: Dropped minor/patch version pinning of actions With GitHub forcibly deprecating old versions of actions, pinning the minor/patch version is more likely to cause breakage than not.	2024-09-20 16:05:15 -05:00
Christopher Haster	7db9e1663a	gha: Switched to standard da for cross-workflow downloads Looks like cross-workflow downloads has finally been added to the standard download-artifact action, so we might as well switch to it to reduce dependencies. dawidd6's version was also missing the merge-multiple feature which is necessary to work around breaking changes in download-artifact's v4 bump. Weirdly it needs GITHUB_TOKEN for some reason? Not sure why this couldn't be implicit.	2024-09-20 16:05:12 -05:00
Christopher Haster	2c4b262c35	gha: Merge artifacts on download Turns out major versions break things. Old behavior: Artifacts with same name are merged New behavior: Artifacts with same name error Using a pattern and merging on download should fix this at least on the job-side. Though I do wonder if we'll start running into artifact limit issues with the new way artifacts are handled...	2024-09-20 16:04:35 -05:00
YAMAMOTO Takashi	72a4b57f4e	gha: Make the artifact names unique	2024-09-19 17:26:49 -05:00
YAMAMOTO Takashi	6e7269890a	gha: Update github actions to the latest versions	2024-09-19 17:18:15 -05:00
Christopher Haster	d01280e649	Merge pull request #968 from littlefs-project/link-pico-littlefs-usb Add links to pico-littlefs-usb (FAT12 emulation) and mklittlefs	2024-04-29 16:21:49 -05:00
Christopher Haster	6e52140d51	Merge pull request #959 from littlefs-project/fix-expanded-magic Duplicate the superblock entry during superblock expansion, fix missing magic	2024-04-29 14:26:38 -05:00
Christopher Haster	0bbb8bc88b	Reorganized external project links a bit These were grouped up a bit better at one point, but that sort of drifted as new project were added: 1. Official repos (mainly littlefs-fuse) 2. Non-C reimplementations/wrappers 3. Utilities 4. Non-littlefs related projects Eventually, maybe when these move out of the README.md, these categories should probably be actually codified as headers or something.	2024-04-17 13:46:33 -05:00
Christopher Haster	78082336e7	Added a link to mklittlefs Implemented by earlephilhower, mklittlefs is a command line interface that seems to be used by the ESP8266 and RP2040 ecosystems. It deserves a mention. Also tweaked mklfs's description a bit.	2024-04-17 13:39:11 -05:00
Christopher Haster	8336ecd203	Added a link to pico-littlefs-usb (FAT12 emulation) Implemented by oyama, pico-littlefs-usb provides an easy interface to littlefs by emulating a FAT12 filesystem over USB. There are some tradeoffs to this, but being able to mount a littlefs device without installing additional drivers is very nice. Maybe in the future devices could provide both a FAT and raw endpoint for easy/advanced filesystem access.	2024-04-17 13:09:04 -05:00
Christopher Haster	68d28b5114	Merge pull request #966 from BrianPugh/fix-divide-by-zero-full-filesystem Fix DivideByZero exception when filesystem is completely full.	2024-04-17 12:38:22 -05:00
Christopher Haster	1bc14933b7	Tweaked on-disk config comments for consistency - Prefer "defaults to blablabla when zero" to hint that this is the default state when both explicitly set to zero and implicitly set to zero thanks to C's initializers. - Prefer "disk" when referencing something stored "on disk". Other terms can quickly get ambiguous. Except maybe "block device"...	2024-04-17 00:16:20 -05:00
Christopher Haster	01b6a47ea8	Extended test_alloc to test inferred block_count The block allocator is an area where inferred block counts (when cfg.block_count=0) are more likely to cause problems. As is shown by the recent divide-by-zero-exhaustion issue.	2024-04-17 00:04:56 -05:00
Brian Pugh	749a45650f	Fix DivideByZero exception when filesystem is completely full.	2024-04-16 20:32:12 -07:00
Christopher Haster	11b036cc6c	Prevented unnecessary superblock rewrites if old version in superblock chain Because multiple, out-of-date superblocks can exist in our superblock chain, we need to be careful to make sure newer superblock entries override older superblock entries. If we see an older on-disk minor version in the superblock chain, we were correctly overriding the on-disk minor version, but we were also leaving the "needs superblock" bit set in our consistency state. This isn't a hard-error, but would lead to a superblock rewrite every mount. The rewrite would make no progress, as the out-of-date version is effectively immutable at this point, and just waste prog cycles. This should fix that by clearing the "needs superblock" bit if we see a newer on-disk minor version.	2024-03-19 00:49:28 -05:00
Christopher Haster	25ee90fdf1	Clarified what is accessible at specific superblock offsets in SPEC.md It used to be the case that the entire superblock entry could be found at specific offsets, but this was only possible while the superblock entry was immutable. Now that the superblock entry is very mutable (block-count changes, lfs2.0 -> lfs2.1 version bumps, etc), the correct superblock entry may end up later in the metadata log. At the very least, the "littlefs" magic string is still immutable and at the specific offset offset=8. This is arguably the most useful fixed-offset item.	2024-03-19 00:49:28 -05:00
Christopher Haster	a60a986c9c	Duplicate the superblock entry during superblock expansion The documentation does not match the implementation here. The intended behavior of superblock expansion was to duplicate the current superblock entry into the new superblock: .--------. .--------. .\|littlefs\|->\|littlefs\| \|\|bs=4096 \| \|\|bs=4096 \| \|\|bc=256 \| \|\|bc=256 \| \|\|crc32 \| \|\|root dir\| \|\| \| \|\|crc32 \| \|'--------' \|'--------' '--------' '--------' The main benefit is that we can rely on the magic string "littlefs" always residing in blocks 0x{0,1}, even if the superblock chain has multiple superblocks. The downside is that earlier superblocks in the superblock chain may contain out-of-date configuration. This is a bit annoying, and risks hard-to-reach bugs, but in theory shouldn't break anything as long as the filesystem is aware of this. Unfortunately this was lost at some point during refactoring in the early v2-alpha work. A lot of code was moving around in this stage, so it's a bit hard to track down the change and if it was intentional. The result is superblock expansion creates a valid linked-list of superblocks, but only the last superblock contains a valid superblock entry: .--------. .--------. .\|crc32 \|->\|littlefs\| \|\| \| \|\|bs=4096 \| \|\| \| \|\|bc=256 \| \|\| \| \|\|root dir\| \|\| \| \|\|crc32 \| \|'--------' \|'--------' '--------' '--------' What's interesting is this isn't invalid as far as lfs_mount is concerned. lfs_mount is happy as long as a superblock entry exists anywhere in the superblock chain. This is good for compat flexibility, but is the main reason this has gone unnoticed for so long. --- With the benefit of more time to think about the problem, it may have been more preferable to copy only the "littlefs" magic string and NOT the superblock entry: .--------. .--------. .\|littlefs\|->\|littlefs\| \|\|crc32c \| \|\|bs=4096 \| \|\| \| \|\|bc=256 \| \|\| \| \|\|root dir\| \|\| \| \|\|crc32 \| \|'--------' \|'--------' '--------' '--------' This would allow for simple "littlefs" magic string checks without the risks associated with out-of-date superblock entries. Unfortunately the current implementation errors if it finds a "littlefs" magic string without an associated superblock entry, so such a change would not be compatible with old drivers. --- This commit tweaks superblock expansion to duplicate the superblock entry instead of simply moving it to the new superblock. And adds tests over the magic string "littlefs" both before and after superblock expansion. Found by rojer and Nikola Kosturski	2024-03-19 00:48:56 -05:00