facebookresearch/faiss
1
0
Fork 0
mirror of https://github.com/facebookresearch/faiss.git synced 2026-10-12 02:13:36 +00:00
Code Issues Packages Projects Releases Wiki Activity

Fix leak and unbounded lists[] in OnDisk invlist deserialization (#5560)

Browse Source 
Summary:
Pull Request resolved: https://github.com/facebookresearch/faiss/pull/5560

`OnDiskInvertedListsIOHook::read` allocates its `OnDiskInvertedLists` with a
bare `new` and then keeps reading. Any throw after that point leaks the whole
object -- including `od->lists`, which `READVECTOR` will happily size up to
`get_deserialization_vector_byte_limit()`. Throwing after that point is the
common case for malformed input: the `slots` and `filename` reads can fail, and
`do_mmap()` on an attacker-controlled filename fails essentially always.

This is what the `faiss_read_index_binary_fuzzer` health check has been tripping
over. The one CI stack that survived tpx truncation names
`OnDiskInvertedLists.cpp:722` (`vector<OnDiskOneList>::resize`) as the largest
live allocation, reached via `IBFf` -> `read_index` -> `IxRP` -> IVF ->
`read_InvertedLists`. Because the harness runs with `detect_leaks: 0`, the
leaked blocks accumulate across a 120 s session; peak RSS across the failing
runs climbed 978 -> 1142 -> 1249 -> 1815 -> 4356 MB against `-rss_limit_mb=4096`.
That is why the check reads as flaky (23.6%) rather than as a hard red, and why
the saved crash artifacts do not reproduce when replayed one at a time.

Switch both `read` and `read_ArrayInvertedLists` to hold the object in a
`std::unique_ptr` and `release()` on success. The public signature still returns
`InvertedLists*`, so this is a local scope guard only -- the same shape
`read_InvertedLists_up` in `index_read.cpp` already uses.

Also reject `lists.size() != nlist` at read time. Every `OnDiskInvertedLists`
accessor (`list_size`, `get_codes`, `get_ids`, ...) indexes `lists[list_no]`
with `list_no` bounded only against `nlist`, so a file whose `lists` vector is
shorter than its `nlist` is a straight out-of-bounds read. Every construction
path in the class keeps `lists.size() == nlist`, so this rejects only malformed
input.

Reviewed By: mnorris11

Differential Revision: D118487565

fbshipit-source-id: 6f01c937b24a2a98620e4ca2bde141674af1b19d
This commit is contained in:
Trang Nguyen
2026-09-14 21:23:31 -07:00
committed by meta-codesync[bot]
parent a74867cf56
commit fb2d57a312
2 changed files with 22 additions and 4 deletions
Download Patch File Download Diff File Expand all files Collapse all files
tests/test_read_index_deserialize.cpp
+12
View File
@@ -2260,6 +2260,18 @@ TEST(ReadIndexDeserialize, InvertedListOversizedEntry) {
set_deserialization_vector_byte_limit(old_limit);
}
TEST(ReadIndexDeserialize, OnDiskInvertedListsShortListsVectorRejected) {
std::vector<uint8_t> buf;
push_fourcc(buf, "IwFl");
push_ivf_header(buf, /*d=*/4);
push_fourcc(buf, "ilod");
push_val<size_t>(buf, 1); // nlist = 1
push_val<size_t>(buf, 16); // code_size = 16
push_val<size_t>(buf, 0); // lists: empty, inconsistent with nlist = 1
expect_read_throws_with(buf, "read 0 lists for nlist 1");
}
// -- Ixrq (IndexRaBitQ, single-bit) --
// qb=0 is valid for Ixrq: disables query quantization, uses raw fp32 values.
// See IndexRaBitQ.h comment on qb and RaBitQDistanceComputerNotQ.