fix(hnsw): reject build dims whose vector count overflows size_t#333
Merged
Conversation
ray_hnsw_build sized the copied vector block as n_nodes * dim * sizeof(float) with no overflow check. Dimensions whose product wraps size_t under-allocate the copy while the memcpy — and every later distance read (vectors + id*dim) — run past the buffer. Guard the product before any allocation, mirroring the per-layer neighbor guard in the loader, and reject overflowing dimensions. This hardens the public C API boundary; the in-tree (hnsw-build ...) path sizes vectors from an in-memory list and cannot reach the overflow, so it is defense-in-depth. Add a regression test driving an overflowing n_nodes/dim pair; with the guard removed it faults under ASan (stack-buffer-overflow at the copy).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What & why
ray_hnsw_buildsized the copied vector block asn_nodes * dim * sizeof(float)with no overflow check (
src/store/hnsw.c). Dimensions whose product wrapssize_tunder-allocate the copy, while the followingmemcpy— and every laterdistance read (
vectors + id*dim) during graph construction — runs past thebuffer.
The per-layer neighbor arrays in the loader already carry a divide-based
overflow guard; the build path had none. This adds the same guard at the top of
ray_hnsw_build, rejecting overflowing dimensions before any allocation:n_nodesanddimare already validated> 0at that point, so the divide issafe.
Scope / severity: this hardens the public C API boundary. The in-tree
(hnsw-build ...)builtin sizes its vectors from an in-memory list and cannotreach the overflow, so for the shipped language surface this is defense-in-depth
rather than a remotely reachable bug. It is the build-path companion to the
loader's untrusted-file vectors guard (submitted separately).
Test
embedding/hnsw_build_overflow_rejecteddrives an overflowingn_nodes/dimpair (chosen so the byte count wraps to 16) and asserts the build is refused.
Verified as a genuine regression test: with the guard removed, the debug ASan
build faults at the copy —
With the guard in place the call returns
NULLand the source is never read.Full suite under the debug ASan + UBSan build:
3632 of 3633 passed (1 skipped, 0 failed).Checklist
dev(notmaster)fix:)makebuilds cleanly (no new warnings)make testpasses; tests added/updated for behaviour changes