Add is_trained check to IndexIVF search and range_search to prevent querying untrained indexes - #5114
Closed
scsiguy wants to merge 6 commits into
Closed
Add is_trained check to IndexIVF search and range_search to prevent querying untrained indexes#5114scsiguy wants to merge 6 commits into
scsiguy wants to merge 6 commits into
Conversation
…rch#5111) Summary: Add utility functions to FaissException.h for capturing and rethrowing exceptions across OpenMP parallel region boundaries, and migrate IndexIVF to use them. New helpers in FaissException.h: - omp_capture_exception(ex): Captures the current exception into a shared exception_ptr using #pragma omp critical. Only records the first exception. - omp_capture_exception(ex, cleanup): Overload with a per-thread cleanup callable (e.g. setting an interrupt flag) that runs inside the critical section. - omp_rethrow_if_exception(ex): Rethrows the captured exception on the main thread after the parallel region completes. The exception_ptr approach preserves the full exception during capture and preserves the original exception type on rethrow. IndexIVF migration (search, search_preassigned, range_search_preassigned): - Replace std::mutex + std::string exception_string with std::exception_ptr + omp_capture_exception/omp_rethrow_if_exception. - Replace bool interrupt with std::atomic<bool> (the previous non-atomic bool was read without synchronization in the omp for loop while being written inside a mutex-guarded catch block — undefined behavior). Use relaxed atomic loads for the interrupt short-circuit check since there is no way to fully close the race between interrupt being set and a thread being in the middle of computation. - Replace catch (const std::exception&) with catch (...) to capture all exception types. - Original exception type is now preserved on rethrow instead of being reformatted into a FaissException wrapping demangle_cpp_symbol + what(). - Replace InterruptCallback::is_interrupted() + manual flag set with InterruptCallback::check(). check() throws on interrupt, which is captured by omp_capture_exception and sets the interrupt flag via the cleanup lambda. This eliminates the separate post-region interrupt check — all error paths now flow through the unified exception capture/rethrow mechanism. - range_search_preassigned has no interrupt short-circuit check in its loops, so it uses omp_capture_exception(ex) without a cleanup lambda and has no interrupt variable. - Wrap all #pragma omp for loop bodies in try/catch blocks so exceptions are caught within the worksharing construct (required by the OpenMP specification). The outer try/catch at the #pragma omp parallel level remains as a safety net for code outside worksharing constructs. - Remove redundant try/catch from scan_one_list and scan_list_func lambdas since all call sites are now protected by the loop body try/catch. Differential Revision: D101233059
) Summary: Exceptions thrown inside OpenMP worksharing constructs in IndexFlatCodes::search call std::terminate because the OpenMP specification does not allow exceptions to escape parallel regions. This is triggered by corrupt serialized index data that causes allocation failures or assertion errors in GenericFlatCodesDistanceComputer, but can occur with any exception thrown during the search loop. Wrap the OpenMP parallel body in IndexFlatCodes::Run_search_with_decompress with per-iteration try/catch blocks that capture exceptions via std::exception_ptr and re-throw them on the main thread after the parallel region completes. The DC constructor is also wrapped separately since it runs before the worksharing loop. SingleResultHandler is moved from stack to heap allocation (std::unique_ptr) because the constructor is now inside a try/catch block. If a stack-allocated object's constructor throws, the object never comes into existence, but OpenMP requires all threads to participate in the worksharing loop that follows — any reference to the uninitialized object would be undefined behavior. With std::unique_ptr, the variable starts as nullptr before the try block, remains nullptr if construction fails, and the interrupt flag ensures all loop iterations skip via continue without dereferencing it. Reviewed By: mnorris11 Differential Revision: D101008838
…ookresearch#5106) Summary: Apply the same OpenMP exception-safety fix from IndexFlatCodes::search to IndexNNDescent::search and IndexNSG::search. Both methods have identical OMP structure: a parallel region constructs per-thread VisitedTable and DistanceComputer objects, then a worksharing loop calls the graph search function. Exceptions thrown inside either the constructor or the loop body (e.g. from corrupt deserialized graph state) call std::terminate because OpenMP does not allow exceptions to escape worksharing constructs. Wrap the per-thread setup and the per-iteration loop body in try/catch blocks that capture exceptions via std::exception_ptr and re-throw on the main thread after the parallel region completes. Potential exception sources in the OMP region: - VisitedTable(ntotal): std::bad_alloc if ntotal is corrupt/huge. ntotal is validated as >= 0 during deserialization but has no upper bound. - storage_distance_computer(storage): may throw from get_distance_computer() if the storage index has corrupt state. - nndescent.search() / nsg.search(): throws FaissException if the graph has not been built (has_built check), or std::bad_alloc from internal vector allocations with corrupt search_L. Reviewed By: mnorris11 Differential Revision: D101031002
…rch#5112) Summary: Add validation in read_ivf_header() to reject a null quantizer sub-index read from serialized data. The IVF deserialization reads the quantizer via read_index(), which returns nullptr when the stream contains the "null" fourcc. A null quantizer is fundamentally invalid for any IVF index type. Without this check, downstream code (e.g. initialize_IVFPQ_precomputed_table, IndexIVF::search) dereferences the null pointer. This single validation protects all IVF index types that share read_ivf_header: IndexIVFFlat, IndexIVFPQ, IndexIVFScalarQuantizer, IndexIVFAdditiveQuantizer, and others. Reviewed By: mnorris11 Differential Revision: D101236489
…ch (facebookresearch#5113) Summary: Add validation that IndexHNSW2Level (fourcc "IHN2") has storage of an appropriate type, both at deserialization time and at search time. IndexHNSW2Level::search() uses dynamic_cast to dispatch between Index2Layer and IndexIVFPQ storage types. When storage is null or a different type (e.g. IndexFlat from corrupt serialized data, or a programmatically misconfigured index), the dynamic_cast returns nullptr which is then unconditionally dereferenced, causing a segfault. Deserialization-time fix: After reading the HNSW storage sub-index for IHN2, validate that storage is non-null and is either Index2Layer or IndexIVFPQ. Search-time defense-in-depth: Add a FAISS_THROW_IF_NOT check on the dynamic_cast result in IndexHNSW2Level::search() before dereferencing. This protects against programmatically constructed indexes that bypass deserialization validation. Reviewed By: mnorris11 Differential Revision: D101243603
…uerying untrained indexes Summary: Add FAISS_THROW_IF_NOT(is_trained) to IndexIVF::search(), IndexIVF::search_preassigned(), IndexIVF::range_search(), and IndexIVF::range_search_preassigned(), mirroring the existing check in IndexScalarQuantizer::search(). This prevents querying untrained IVF indexes deserialized from corrupt data where the ScalarQuantizer trained vector is empty. The existing deserialization validation in read_ScalarQuantizer (D98924927) correctly allows untrained indexes (is_trained=false with empty trained) to be deserialized, since these are legitimately produced by index_factory before training. However, IndexIVF search methods lacked the is_trained guard that IndexScalarQuantizer::search() has, allowing a deserialized untrained IndexIVFScalarQuantizer to be queried, which causes null-deref in QuantizerTemplate when it indexes into the empty trained vector. Differential Revision: D101243973
Contributor
|
@scsiguy has exported this pull request. If you are a Meta employee, you can view the originating Diff in D101243973. |
scsiguy
added a commit
to scsiguy/faiss
that referenced
this pull request
Apr 17, 2026
…uerying untrained indexes (facebookresearch#5114) Summary: Add FAISS_THROW_IF_NOT(is_trained) to IndexIVF::search(), IndexIVF::search_preassigned(), IndexIVF::range_search(), and IndexIVF::range_search_preassigned(), mirroring the existing check in IndexScalarQuantizer::search(). This prevents querying untrained IVF indexes deserialized from corrupt data where the ScalarQuantizer trained vector is empty. The existing deserialization validation in read_ScalarQuantizer (D98924927) correctly allows untrained indexes (is_trained=false with empty trained) to be deserialized, since these are legitimately produced by index_factory before training. However, IndexIVF search methods lacked the is_trained guard that IndexScalarQuantizer::search() has, allowing a deserialized untrained IndexIVFScalarQuantizer to be queried, which causes null-deref in QuantizerTemplate when it indexes into the empty trained vector. Differential Revision: D101243973
scsiguy
added a commit
to scsiguy/faiss
that referenced
this pull request
Apr 17, 2026
…uerying untrained indexes (facebookresearch#5114) Summary: Add FAISS_THROW_IF_NOT(is_trained) to IndexIVF::search(), IndexIVF::search_preassigned(), IndexIVF::range_search(), and IndexIVF::range_search_preassigned(), mirroring the existing check in IndexScalarQuantizer::search(). This prevents querying untrained IVF indexes deserialized from corrupt data where the ScalarQuantizer trained vector is empty. The existing deserialization validation in read_ScalarQuantizer (D98924927) correctly allows untrained indexes (is_trained=false with empty trained) to be deserialized, since these are legitimately produced by index_factory before training. However, IndexIVF search methods lacked the is_trained guard that IndexScalarQuantizer::search() has, allowing a deserialized untrained IndexIVFScalarQuantizer to be queried, which causes null-deref in QuantizerTemplate when it indexes into the empty trained vector. Differential Revision: D101243973
scsiguy
added a commit
to scsiguy/faiss
that referenced
this pull request
Apr 17, 2026
…uerying untrained indexes (facebookresearch#5114) Summary: Add FAISS_THROW_IF_NOT(is_trained) to IndexIVF::search(), IndexIVF::search_preassigned(), IndexIVF::range_search(), and IndexIVF::range_search_preassigned(), mirroring the existing check in IndexScalarQuantizer::search(). This prevents querying untrained IVF indexes deserialized from corrupt data where the ScalarQuantizer trained vector is empty. The existing deserialization validation in read_ScalarQuantizer (D98924927) correctly allows untrained indexes (is_trained=false with empty trained) to be deserialized, since these are legitimately produced by index_factory before training. However, IndexIVF search methods lacked the is_trained guard that IndexScalarQuantizer::search() has, allowing a deserialized untrained IndexIVFScalarQuantizer to be queried, which causes null-deref in QuantizerTemplate when it indexes into the empty trained vector. Differential Revision: D101243973
scsiguy
added a commit
to scsiguy/faiss
that referenced
this pull request
Apr 18, 2026
…uerying untrained indexes (facebookresearch#5114) Summary: Add FAISS_THROW_IF_NOT(is_trained) to IndexIVF::search(), IndexIVF::search_preassigned(), IndexIVF::range_search(), and IndexIVF::range_search_preassigned(), mirroring the existing check in IndexScalarQuantizer::search(). This prevents querying untrained IVF indexes deserialized from corrupt data where the ScalarQuantizer trained vector is empty. The existing deserialization validation in read_ScalarQuantizer correctly allows untrained indexes (is_trained=false with empty trained) to be deserialized, since these are legitimately produced by index_factory before training. However, IndexIVF search methods lacked the is_trained guard that IndexScalarQuantizer::search() has, allowing a deserialized untrained IndexIVFScalarQuantizer to be queried, which causes null-deref in QuantizerTemplate when it indexes into the empty trained vector. Reviewed By: mnorris11 Differential Revision: D101243973
scsiguy
added a commit
to scsiguy/faiss
that referenced
this pull request
Apr 18, 2026
…uerying untrained indexes (facebookresearch#5114) Summary: Add FAISS_THROW_IF_NOT(is_trained) to IndexIVF::search(), IndexIVF::search_preassigned(), IndexIVF::range_search(), and IndexIVF::range_search_preassigned(), mirroring the existing check in IndexScalarQuantizer::search(). This prevents querying untrained IVF indexes deserialized from corrupt data where the ScalarQuantizer trained vector is empty. The existing deserialization validation in read_ScalarQuantizer correctly allows untrained indexes (is_trained=false with empty trained) to be deserialized, since these are legitimately produced by index_factory before training. However, IndexIVF search methods lacked the is_trained guard that IndexScalarQuantizer::search() has, allowing a deserialized untrained IndexIVFScalarQuantizer to be queried, which causes null-deref in QuantizerTemplate when it indexes into the empty trained vector. Reviewed By: mnorris11 Differential Revision: D101243973
scsiguy
added a commit
to scsiguy/faiss
that referenced
this pull request
Apr 18, 2026
…uerying untrained indexes (facebookresearch#5114) Summary: Add FAISS_THROW_IF_NOT(is_trained) to IndexIVF::search(), IndexIVF::search_preassigned(), IndexIVF::range_search(), and IndexIVF::range_search_preassigned(), mirroring the existing check in IndexScalarQuantizer::search(). This prevents querying untrained IVF indexes deserialized from corrupt data where the ScalarQuantizer trained vector is empty. The existing deserialization validation in read_ScalarQuantizer correctly allows untrained indexes (is_trained=false with empty trained) to be deserialized, since these are legitimately produced by index_factory before training. However, IndexIVF search methods lacked the is_trained guard that IndexScalarQuantizer::search() has, allowing a deserialized untrained IndexIVFScalarQuantizer to be queried, which causes null-deref in QuantizerTemplate when it indexes into the empty trained vector. Reviewed By: mnorris11 Differential Revision: D101243973
Contributor
|
This pull request has been merged in 6707eac. |
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
1 participant
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.
Summary:
Add FAISS_THROW_IF_NOT(is_trained) to IndexIVF::search(), IndexIVF::search_preassigned(), IndexIVF::range_search(), and IndexIVF::range_search_preassigned(), mirroring the existing check in IndexScalarQuantizer::search(). This prevents querying untrained IVF indexes deserialized from corrupt data where the ScalarQuantizer trained vector is empty.
The existing deserialization validation in read_ScalarQuantizer (D98924927) correctly allows untrained indexes (is_trained=false with empty trained) to be deserialized, since these are legitimately produced by index_factory before training. However, IndexIVF search methods lacked the is_trained guard that IndexScalarQuantizer::search() has, allowing a deserialized untrained IndexIVFScalarQuantizer to be queried, which causes null-deref in QuantizerTemplate when it indexes into the empty trained vector.
Differential Revision: D101243973