diff --git a/lfs.c b/lfs.c index d35d5d6d..933f69c7 100644 --- a/lfs.c +++ b/lfs.c @@ -3664,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) { diff --git a/tests/test_seek.toml b/tests/test_seek.toml index 9b3768d7..33fb5785 100644 --- 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; +'''