Skip to content

fix(simd_path): clamp resolve_ptrs iteration to buf.len() to prevent OOB panic #652

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
elee7420-gif wants to merge 1 commit into from
Closed

fix(simd_path): clamp resolve_ptrs iteration to buf.len() to prevent OOB panic #652

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion crates/fff-core/src/simd_path.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ impl ChunkedString {
arena: ArenaPtr,
buf: &'a mut [*const u8; 32],

@dmtrKovalenko dmtrKovalenko Jul 6, 2026

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

wait but this gives us max 32 * 128 bytes = 4096 bytes which is max file size ever available

how is it possible that the indicies array contains more elements

@dmtrKovalenko dmtrKovalenko Jul 6, 2026

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

@gustav-fff do you have any ideas?

@gustav-fff gustav-fff Jul 6, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

[triage-bot] DIRECTED:

Buffer holds 32 pointers, each pointing to one SimdChunk. SIMD_CHUNK_BYTES = 16 (crates/fff-core/src/simd_path.rs:7), not 128 — so 32 chunks cover only 32 * 16 = 512 bytes, not 4096.

Path capacity is PATH_BUF_SIZE (crates/fff-core/src/constants.rs:36-39):

  • Linux: libc::PATH_MAX = 4096 → up to 256 chunks
  • macOS: libc::PATH_MAX = 1024 → up to 64 chunks
  • Windows: 4096 → up to 256 chunks

ChunkedString::indices is a SmallVec<[u32; 4]> with no cap, so indices.len() can legitimately reach 64 (macOS) or 256 (Linux/Windows). Any path >512 bytes trips the OOB. The reporter's 500+ byte path on macOS is well within PATH_MAX but exceeds the pointer buffer.

The clamp in this PR is a truncation-based band-aid: paths >512 bytes get scored on their first 512 bytes only, which silently degrades fuzzy match quality for long paths (rare, but real on deeply-nested monorepos).

Proper fix options:

  1. Size the caller's buffer to PATH_BUF_SIZE / SIMD_CHUNK_BYTES (256 on Linux, 64 on macOS) at every call site in score.rs:35,63,69,176,202. Stack cost: 256 * 8 = 2 KiB per scoring frame — acceptable for a scoring hot path that already reserves large buffers.
  2. Keep the 32-ptr buffer but reject/skip paths whose byte_len > 512 at index time so resolve_ptrs never sees them. Cheaper but loses matching on legitimate long paths.

I'd lean (1) — a fixed [*const u8; 256] array in score.rs matches the actual PATH_BUF_SIZE invariant and needs no runtime check. Want me to push that instead of the truncation?

Honk-Honk 🪿

@dmtrKovalenko dmtrKovalenko Jul 6, 2026

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

ooops this is a real mistake, we have to have a compile time buffer size per the platform limits not 512 bytes

) -> &'a [*const u8] {
let count = self.indices.len();
let count = self.indices.len().min(buf.len());
let base = arena.as_ptr();
for (i, &idx) in self.indices.iter().enumerate() {
buf[i] = unsafe { base.add(idx as usize * SIMD_CHUNK_BYTES) };
Expand Down
Loading