Skip to content

Tests from overnight audit#64

Open
xarantolus wants to merge 13 commits into
mainfrom
audit/tests-only
Open

Tests from overnight audit#64
xarantolus wants to merge 13 commits into
mainfrom
audit/tests-only

Conversation

@xarantolus
Copy link
Copy Markdown
Contributor

@xarantolus xarantolus commented May 7, 2026

This adds a couple of tests that result in panics / undefined behaviour, which were found using Claude.

Test File What it shows
alloc_full_two_words bitset.rs BitAlloc::alloc(N*BITS_PER_WORD) panics — reachable from PFA
alloc_full_three_words bitset.rs Same on 3-word allocator
at2_mut_out_of_bounds_returns_none array.rs Vec::at2_mut UB: returns &mut T to uninit memory for OOB
at3_mut_out_of_bounds_returns_none array.rs Same for at3_mut
indexmap_get2_mut_oob_does_not_panic array.rs IndexMap::get2_mut panics on OOB, inconsistent with the rest of Get/GetMut
indexmap_get3_mut_oob_does_not_panic array.rs Same for get3_mut
bitreclaim_insert_with_failed_closure_does_not_leak array.rs Bit leak via user closure error
fixed_pool_alloc_beyond_n_returns_none pool.rs FixedPool OOB panic when WORDS*BITS_PER_WORD > N

Copilot AI review requested due to automatic review settings May 7, 2026 07:25
Copilot AI reviewed May 7, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR adds regression tests covering several panic/UB scenarios found during an audit, primarily around allocator exhaustion semantics and out-of-bounds mutable access.

Changes:

  • Added tests for BitAlloc::alloc allocating an exact multiple of BITS_PER_WORD (full-word allocations).
  • Added tests for Vec::at2_mut/at3_mut, IndexMap::get2_mut/get3_mut, and BitReclaimMap::insert_with error-path behavior.
  • Added a test demonstrating FixedPool can panic when WORDS*BITS_PER_WORD > N.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 5 comments.

File Description
src/types/pool.rs Adds a test that expects FixedPool::alloc to return None (not panic) when indices exceed N.
src/types/bitset.rs Adds tests for full-capacity allocations that currently hit an OOB access in allocation marking.
src/types/array.rs Adds tests demonstrating OOB/UB in multi-mutable access helpers and a leak in BitReclaimMap::insert_with.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/types/array.rs
Comment thread src/types/array.rs
Comment thread src/types/array.rs
Comment thread src/types/bitset.rs
Comment thread src/types/pool.rs
@xarantolus xarantolus requested review from F42J and thomasw04 May 8, 2026 21:50
xarantolus and others added 13 commits May 11, 2026 15:21
@xarantolus @claude
Add tests demonstrating 7 reachable bugs in data structures
2a4dd9d 
Tests added (all currently fail on the unfixed code):
- BitAlloc::alloc(N*BITS_PER_WORD) panics on word-aligned full allocation
  (alloc_full_two_words, alloc_full_three_words). Reachable from PFA when
  allocating all available pages.
- Vec::at2_mut / at3_mut return Some(&mut <uninit>) for out-of-bounds indices
  (UB) despite the doc-comment promising None for OOB.
- IndexMap::get2_mut / get3_mut panic on OOB indices via split_at_mut /
  array indexing, inconsistent with the rest of Get/GetMut returning None.
- BitReclaimMap::insert_with leaks BitAlloc bits on closure error. The
  closure is user-supplied; sustained errors exhaust the allocator.

No production code is changed; this commit only adds failing tests that
serve as regression reproducers for the issues.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@xarantolus @claude
Add FixedPool OOB regression test
3a1be1d 
FixedPool<T, N, WORDS> exposes N and WORDS as independent const generics,
but BitAlloc<WORDS> tracks WORDS*BITS_PER_WORD bits while blocks has only
N slots. Any user picking WORDS*BITS_PER_WORD > N hits an OOB panic on
the (N+1)-th alloc from `self.blocks[idx]` indexing past the end —
violating the allocator's "None on exhaustion" contract.

Currently no in-tree caller instantiates FixedPool, but the bug is real
for any future user.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@xarantolus @claude
Show returned value in at3_mut OOB assertion failure
cab2077 
Mirror the at2_mut format-string pattern: when at3_mut returns
Some(&mut <uninit>) for an out-of-bounds index, the failure message now
prints the garbage value, making the UB obvious.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@xarantolus @claude
Apply targeted fixes for the 8 failing tests
a9eb0b3 
- BitAlloc::alloc: guard last-word write against `len % BITS_PER_WORD == 0`
  (run ends on a word boundary, no trailing partial word).
- Vec::at2_mut / at3_mut: bounds-check each index against self.len before
  constructing &mut T; previously returned references into uninitialized
  memory for out-of-bounds indices.
- IndexMap::get2_mut / get3_mut: return None for out-of-bounds indices
  instead of panicking inside split_at_mut / array indexing.
- BitReclaimMap::insert_with: release the BitAlloc bit when the
  user-supplied closure returns an error.
- FixedPool::alloc: bounds-check the BitAlloc-returned index against N
  before indexing blocks; the const generics WORDS and N are independent
  and BitAlloc tracks WORDS * BITS_PER_WORD bits.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@xarantolus @claude
Simplify IndexMap::get3_mut to match original pointer-from-index pattern
f05e457 
Use `&mut self.data[idx] as *mut Option<V>` (released into a raw pointer)
instead of as_mut_ptr() + add(); now that bounds-checks gate the indexing,
the original pattern is cleaner and avoids unsafe pointer arithmetic.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@xarantolus
Formatting
f296325
@xarantolus @claude
Fix BitReclaimMap::insert bit-leak symmetric to insert_with
303510a 
When raw_insert rejects an idx that BitAlloc handed out (idx >= N because
BitAlloc<N> tracks N*BITS_PER_WORD bits while IndexMap<N> has only N slots),
release the bit so it can't accumulate. Mirrors the insert_with fix.

The new test probes BitAlloc directly: after a failed insert it expects
the next free bit to be 2 (the index that raw_insert rejected), confirming
the bit was returned to the allocator. Verified to fail before the fix
(BitAlloc returns Some(3), proving bit 2 is leaked).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@xarantolus @claude
Fix Vec::new_init to not leak inline clones on OOM
58c4151 
When length > N, the original code wrote N inline clones and only then
attempted to allocate the extra Box. If the allocation failed, vec.len
was still 0 → Drop called clear() which dropped nothing → the N clones
leaked.

Reorder so the Box allocation happens first (no inline clones written
on OOM), and bump vec.len after each individual write so a panicking
T::clone leaves Drop with a consistent state.

Verified via the new clones==drops invariant test (fails before fix
with clones=2, drops=1; passes after).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@xarantolus
Format
64f5bfd
@xarantolus @claude
Remove section banner comments from test modules
d4f471f 
Drop the `// -------- ... --------` group banners; test names already
describe what's being tested.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@xarantolus @claude
Memory subsystem audit: 7 bugs found and fixed
5395fdc 
Per `audit-memory-report.md`:
- BestFitAllocator: double-free creates free-list cycle (BLOCKER)
- PFA bitset: free() accepts sub-begin addresses (silent corruption)
- Region::contains: returns true for unplaced regions
- no-MMU unmap was a no-op (per-task BestFit leak, BLOCKER)
- no-MMU AddressSpace had no Drop (PFA pages leaked forever, BLOCKER)
- no-MMU virt/phys translation accepted out-of-range addresses
- HAL cortex-m ArmStack: byte/word unit confusion in bounds checks

12 new tests added (78 passing total, up from 66 on this branch).
5 Kani proofs validated locally, reverted before commit per cycle
convention.

The BitReclaimMap insert_with bit-leak fix from the prior cycle on
this branch is load-bearing for thread-memory correctness — flagged
in the report.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@xarantolus @claude
Adapt audit cycles to POSIX error renames
14df7f9 
Three sites still referenced the pre-POSIX error names after the
rebase onto current main:

- src/mem/vmm/nommu.rs::unmap: InvalidArgument -> EINVAL
- src/types/array.rs test: OutOfMemory -> ENOMEM
- machine/cortex-m/src/native/sched.rs::Stack::new: Error::default() ->
  PosixError::EINVAL

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@xarantolus @claude
Format
e0e5a9b 
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@xarantolus xarantolus force-pushed the audit/tests-only branch 2 times, most recently from c6d980b to e0e5a9b Compare May 11, 2026 15:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot Copilot left review comments

@thomasw04 thomasw04 Awaiting requested review from thomasw04

@F42J F42J Awaiting requested review from F42J

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@xarantolus @thomasw04 @F42J