mirror of
https://github.com/facebookresearch/faiss.git
synced 2026-10-12 02:13:36 +00:00
Fix IndexSVSVamana null-deref in deserialize_impl (#5029)
Summary: Pull Request resolved: https://github.com/facebookresearch/faiss/pull/5029 1. **Null pointer dereference in `deserialize_impl`**: `IndexSVSVamana::deserialize_impl()` and `IndexSVSFlat::deserialize_impl()` called `impl->load(...)` as a member function on a null `impl` pointer, causing a SIGSEGV. The `load` function is actually a static factory method (like `build` and `destroy`), so it should be called as `ClassName::load(&impl, ...)` instead. This matches the pattern used in `IndexSVSVamanaLeanVec::deserialize_training_data()` which correctly calls `svs_runtime::LeanVecTrainingData::load(&tdata, in)`. 2. **Added post-load validation**: Added `FAISS_THROW_IF_NOT_MSG(impl, ...)` after `load()` returns to ensure the impl pointer was actually populated. 3. **Added tests**: Added deserialization tests that provide invalid SVS stream data and verify the load fails with a FaissException rather than crashing. Reviewed By: mnorris11 Differential Revision: D99117347 fbshipit-source-id: 3165048d06dc5ab66fcf83ecba3a144605c5732f
This commit is contained in:
committed by
meta-codesync[bot]
parent
ab3da522a4
commit
4b8da6820e
@@ -107,11 +107,11 @@ void IndexSVSFlat::serialize_impl(std::ostream& out) const {
|
||||
void IndexSVSFlat::deserialize_impl(std::istream& in) {
|
||||
FAISS_THROW_IF_MSG(impl, "Cannot deserialize: SVS index already loaded.");
|
||||
auto metric = to_svs_metric(metric_type);
|
||||
auto status = impl->load(&impl, in, metric);
|
||||
auto status = svs_runtime::FlatIndex::load(&impl, in, metric);
|
||||
if (!status.ok()) {
|
||||
FAISS_THROW_MSG(status.message());
|
||||
}
|
||||
FAISS_THROW_IF_NOT(impl);
|
||||
FAISS_THROW_IF_NOT_MSG(impl, "Failed to load SVS Flat index.");
|
||||
}
|
||||
|
||||
} // namespace faiss
|
||||
|
||||
@@ -236,10 +236,12 @@ void IndexSVSVamana::deserialize_impl(std::istream& in) {
|
||||
FAISS_THROW_IF_MSG(impl, "Cannot deserialize: SVS index already loaded.");
|
||||
auto svs_metric = to_svs_metric(metric_type);
|
||||
auto svs_storage_kind = to_svs_storage_kind(storage_kind);
|
||||
auto status = impl->load(&impl, in, svs_metric, svs_storage_kind);
|
||||
auto status = svs_runtime::DynamicVamanaIndex::load(
|
||||
&impl, in, svs_metric, svs_storage_kind);
|
||||
if (!status.ok()) {
|
||||
FAISS_THROW_MSG(status.message());
|
||||
}
|
||||
FAISS_THROW_IF_NOT_MSG(impl, "Failed to load SVS Vamana index.");
|
||||
}
|
||||
|
||||
} // namespace faiss
|
||||
|
||||
@@ -2966,3 +2966,80 @@ TEST(ReadIndexDeserialize, IndexRQFastScanAQDimensionMismatch) {
|
||||
|
||||
expect_read_throws_with(buf, "does not match index d");
|
||||
}
|
||||
|
||||
// ============================================================
|
||||
// SVS fourcc rejection / deserialization safety (Group F: T262015608)
|
||||
// ============================================================
|
||||
|
||||
#ifdef FAISS_ENABLE_SVS
|
||||
|
||||
// When SVS is enabled, deserializing an SVS Vamana index with invalid SVS
|
||||
// stream data should throw a FaissException (from the SVS runtime load
|
||||
// failure) rather than crashing with a null-pointer dereference.
|
||||
// Previously, deserialize_impl called impl->load() on a null impl pointer.
|
||||
TEST(ReadIndexDeserialize, SVSVamanaInvalidStreamThrows) {
|
||||
std::vector<uint8_t> buf;
|
||||
push_fourcc(buf, "ISVD");
|
||||
push_index_header(buf, 8, 0);
|
||||
// SVS Vamana deserialization fields:
|
||||
push_val<size_t>(buf, 32); // graph_max_degree
|
||||
push_val<float>(buf, 1.2f); // alpha
|
||||
push_val<size_t>(buf, 10); // search_window_size
|
||||
push_val<size_t>(buf, 10); // search_buffer_capacity
|
||||
push_val<size_t>(buf, 64); // construction_window_size
|
||||
push_val<size_t>(buf, 750); // max_candidate_pool_size
|
||||
push_val<size_t>(buf, 28); // prune_to
|
||||
push_val<bool>(buf, false); // use_full_search_history
|
||||
push_val<int>(buf, 0); // storage_kind (SVS_Float16)
|
||||
push_val<bool>(buf, true); // initialized = true → triggers deserialize_impl
|
||||
// Provide garbage SVS stream data — load should fail gracefully.
|
||||
for (int i = 0; i < 256; i++) {
|
||||
push_val<uint8_t>(buf, 0);
|
||||
}
|
||||
|
||||
// Should throw from SVS runtime load failure, NOT crash with SIGSEGV.
|
||||
EXPECT_THROW(
|
||||
{
|
||||
auto reader = faiss::VectorIOReader();
|
||||
reader.data = buf;
|
||||
faiss::read_index(&reader);
|
||||
},
|
||||
faiss::FaissException);
|
||||
}
|
||||
|
||||
// Same test for SVS Flat index.
|
||||
TEST(ReadIndexDeserialize, SVSFlatInvalidStreamThrows) {
|
||||
std::vector<uint8_t> buf;
|
||||
push_fourcc(buf, "ISVF");
|
||||
push_index_header(buf, 8, 0);
|
||||
push_val<bool>(buf, true); // initialized = true → triggers deserialize_impl
|
||||
// Provide garbage SVS stream data.
|
||||
for (int i = 0; i < 256; i++) {
|
||||
push_val<uint8_t>(buf, 0);
|
||||
}
|
||||
|
||||
EXPECT_THROW(
|
||||
{
|
||||
auto reader = faiss::VectorIOReader();
|
||||
reader.data = buf;
|
||||
faiss::read_index(&reader);
|
||||
},
|
||||
faiss::FaissException);
|
||||
}
|
||||
|
||||
#else // !FAISS_ENABLE_SVS
|
||||
|
||||
// When SVS is not enabled, attempting to read an index with an SVS fourcc
|
||||
// should fail with an "unknown fourcc" error rather than crashing.
|
||||
TEST(ReadIndexDeserialize, SVSVamanaFourccRejected) {
|
||||
std::vector<uint8_t> buf;
|
||||
push_fourcc(buf, "ISVD");
|
||||
push_index_header(buf, 8, 0);
|
||||
for (int i = 0; i < 128; i++) {
|
||||
push_val<uint8_t>(buf, 0);
|
||||
}
|
||||
|
||||
expect_read_throws_with(buf, "fourcc");
|
||||
}
|
||||
|
||||
#endif // FAISS_ENABLE_SVS
|
||||
|
||||
Reference in New Issue
Block a user