fix(simd_path): clamp resolve_ptrs iteration to buf.len() to prevent OOB panic#652
fix(simd_path): clamp resolve_ptrs iteration to buf.len() to prevent OOB panic#652elee7420-gif wants to merge 1 commit into
Conversation
resolve_ptrs() iterates self.indices.len() times over a fixed-size
[*const u8; 32] buffer with no guard. When a file path exceeds 512
bytes (32 chunks × 16 bytes), the loop accesses buf[32] and panics:
index out of bounds: the len is 32 but the index is 32
On macOS PATH_MAX is 1024, so any legitimately long path can trigger
this. Clamp count to buf.len() so pathological paths are truncated
gracefully instead of crashing.
Fixes an OOB panic found in pi-fff v0.9.6.
dmtrKovalenko
left a comment
dmtrKovalenko
left a comment
There was a problem hiding this comment.
wow thank you for the fix I need to spend some time debugging to understand the origin of the issue. Will merge today anyways
| @@ -103,7 +103,7 @@ impl ChunkedString { | |||
| arena: ArenaPtr, | |||
| buf: &'a mut [*const u8; 32], | |||
There was a problem hiding this comment.
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
There was a problem hiding this comment.
[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:
- Size the caller's buffer to
PATH_BUF_SIZE / SIMD_CHUNK_BYTES(256 on Linux, 64 on macOS) at every call site inscore.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. - Keep the 32-ptr buffer but reject/skip paths whose
byte_len > 512at index time soresolve_ptrsnever 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 🪿
There was a problem hiding this comment.
ooops this is a real mistake, we have to have a compile time buffer size per the platform limits not 512 bytes
|
I'm closing this PR as it doesn't really fix the issue, have opened a spearate one. (it is preserving your original commit) |
3 participants
Problem
ChunkedString::resolve_ptrs()iteratesself.indices.len()times over a fixed[*const u8; 32]buffer with no bounds guard. When a file path exceeds 512 bytes (32 chunks × 16 bytes), the loop accessesbuf[32]and panics:On macOS
PATH_MAXis 1024, so any legitimately long path can trigger this. The panic was found in the field withpi-fffv0.9.6.Fix
Clamp
counttobuf.len()so pathological paths are safely truncated instead of crashing. The caller already usesptrs.len()to control iteration, so a truncated slice is handled correctly downstream.This is a correctness fix — zero behavioral change for the common case (paths < 512 bytes).
Related
Discovered in the pi-fff integration. See the full diagnosis for details.