Reject null quantizer during IVF index deserialization (#5112) - #5112
Closed
scsiguy wants to merge 4 commits into
Closed
Reject null quantizer during IVF index deserialization (#5112)#5112scsiguy wants to merge 4 commits into
scsiguy wants to merge 4 commits into
Conversation
Contributor
|
@scsiguy has exported this pull request. If you are a Meta employee, you can view the originating Diff in D101236489. |
scsiguy
added a commit
to scsiguy/faiss
that referenced
this pull request
Apr 17, 2026
…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
scsiguy
added a commit
to scsiguy/faiss
that referenced
this pull request
Apr 17, 2026
…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
scsiguy
added a commit
to scsiguy/faiss
that referenced
this pull request
Apr 17, 2026
…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
scsiguy
added a commit
to scsiguy/faiss
that referenced
this pull request
Apr 17, 2026
…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
scsiguy
added a commit
to scsiguy/faiss
that referenced
this pull request
Apr 17, 2026
…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
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: Pull Request resolved: facebookresearch#5105 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. Differential Revision: D101008838
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. Differential Revision: D101031002
scsiguy
added a commit
to scsiguy/faiss
that referenced
this pull request
Apr 18, 2026
…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
scsiguy
added a commit
to scsiguy/faiss
that referenced
this pull request
Apr 18, 2026
…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
meta-codesync
Bot
changed the title
scsiguy
force-pushed
the
export-D101236489
branch
from
April 18, 2026 21:56
170e2ca to
28ace55
Compare
scsiguy
added a commit
to scsiguy/faiss
that referenced
this pull request
Apr 18, 2026
…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
scsiguy
added a commit
to scsiguy/faiss
that referenced
this pull request
Apr 18, 2026
…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
scsiguy
added a commit
to scsiguy/faiss
that referenced
this pull request
Apr 18, 2026
…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
…rch#5112) Summary: Pull Request resolved: facebookresearch#5112 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
scsiguy
force-pushed
the
export-D101236489
branch
from
April 18, 2026 21:59
28ace55 to
507cd25
Compare
Contributor
|
This pull request has been merged in edd6f3b. |
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 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