Github/littlefs
1
Fork 0
Code Actions Activity

tests: Accidentally found a couple buffer overruns

Browse Source 
- Off-by-one in test_btree_find_general[_sparse]_fuzz

  Because we can only create named btrees via splitting, these always
  start with one entry. If all operations are randomly selected to be
  splits, this can lead to an overflow of the sim buffer (sounds
  unlikely, but relatively easily for small N).

  The fix is to use a `for (lfs3_size_t i = 1; i < N; i++)` loop to
  account for the initial entry. Note we already use this in the
  test_btree_split_* tests.

  An alternative is allocating space for N+1 entries, but this seems
  unintuitive with N usually being associated with the upper bound on
  btree size.

- Off-by-one in our sim rename pattern

  When renaming, we don't bother to update sim_size, because after the
  rename the sim_size size will be unchanged. But this means the
  sim_size is out-of-date during the memmove that reinserts the renamed
  entry. Buffer overflow!

  To fix we just need to use sim_size-1 to account for the temporarily
  deleted entry.

  This is messy C code, so not surprised it went unnoticed, even though
  this pattern ended up in quite a few tests.

Found while running with HEAP=1. This was just intended to test HEAP=1,
but I guess the injected heap hooks result in a more fragile heap? They
increase all allocations by one word, and maybe this reduces alignment
padding? Not exactly sure.

But it's a good argument for maybe adding heap canaries in the future.
Previously we ran Valgrind on all tests, but it's unclear if this will
still be reasonable with the number of tests we have now.
This commit is contained in:
Christopher Haster
2026-01-27 11:42:03 -06:00
parent d37785cc9b
commit bc9562c64e
11 changed files with 95 additions and 95 deletions
Download Patch File Download Diff File Expand all files Collapse all files
tests/test_grow.toml
+10 -10
View File
@@ -712,7 +712,7 @@ code = '''
}
// then insert
memmove(&sim[k+1], &sim[k],
(sim_size-k)*sizeof(lfs3_size_t));
(sim_size-1-k)*sizeof(lfs3_size_t));
sim[k] = y;
}
break;
@@ -1139,9 +1139,9 @@ code = '''
}
// then insert
memmove(&sim[k+1], &sim[k],
(sim_size-k)*sizeof(lfs3_size_t));
(sim_size-1-k)*sizeof(lfs3_size_t));
memmove(&sim_prngs[k+1], &sim_prngs[k],
(sim_size-k)*sizeof(uint32_t));
(sim_size-1-k)*sizeof(uint32_t));
sim[k] = y;
sim_prngs[k] = wprng;
}
@@ -1611,11 +1611,11 @@ code = '''
}
// then insert
memmove(&sim[k+1], &sim[k],
(sim_size-k)*sizeof(lfs3_size_t));
(sim_size-1-k)*sizeof(lfs3_size_t));
memmove(&sim_prngs[k+1], &sim_prngs[k],
(sim_size-k)*sizeof(uint32_t));
(sim_size-1-k)*sizeof(uint32_t));
memmove(&sim_isstickys[k+1], &sim_isstickys[k],
(sim_size-k)*sizeof(bool));
(sim_size-1-k)*sizeof(bool));
sim[k] = y;
sim_prngs[k] = wprng;
sim_isstickys[k] = sticky;
@@ -2147,13 +2147,13 @@ code = '''
}
// then insert
memmove(&sim[k+1], &sim[k],
(sim_size-k)*sizeof(lfs3_size_t));
(sim_size-1-k)*sizeof(lfs3_size_t));
memmove(&sim_prngs[k+1], &sim_prngs[k],
(sim_size-k)*sizeof(uint32_t));
(sim_size-1-k)*sizeof(uint32_t));
memmove(&sim_isstickys[k+1], &sim_isstickys[k],
(sim_size-k)*sizeof(bool));
(sim_size-1-k)*sizeof(bool));
memmove(&sim_isdirs[k+1], &sim_isdirs[k],
(sim_size-k)*sizeof(bool));
(sim_size-1-k)*sizeof(bool));
sim[k] = y;
sim_prngs[k] = wprng;
sim_isstickys[k] = sticky;