mirror of
https://github.com/facebookresearch/faiss.git
synced 2026-10-12 02:13:36 +00:00
Validate legacy IVFFlat per-list codes size against ids size (#5259)
Summary:
The legacy `IvFl`/`IvFL` on-disk inverted-list formats in `read_index_up` serialize per-list `ids` and `codes` as two independent `READVECTOR` calls with no cross-check between their lengths. A malformed file can produce a list where `codes[i].size()` does not equal `ids[i].size() * code_size`.
This matters at the CPU → GPU clone path: `GpuIndexIVFFlat::copyFrom` → `IVFBase::copyInvertedListsFrom` → `IVFBase::addEncodedVectorsToList_` derives its `memcpy` length from `list_size` (which derives from `ids.size()`) and copies that many bytes out of the codes buffer. When `codes[i]` is shorter than `ids[i].size() * code_size`, the `memcpy` reads past the heap allocation.
The current `ilar`/`IwFl` reader (`read_InvertedLists_up`) already validates this invariant via the `mul_no_overflow` + `FAISS_THROW_IF_NOT_FMT` pattern. This PR brings the legacy `IvFl`/`IvFL` branches in `read_index_up` to parity with that validation.
## Reproduction (pre-patch)
A malformed legacy IVFFlat file with `d=128`, `nlist=1`, `ids[0].size()=1`, `codes[0].size()=511` bytes driven through the public API path `faiss::read_index()` → `faiss::gpu::index_cpu_to_gpu()` produces (host AddressSanitizer):
```
ERROR: AddressSanitizer: heap-buffer-overflow on address 0x... at pc 0x...
READ of size 512 at 0x... thread T0
#0 memcpy
https://github.com/facebookresearch/faiss/issues/1 faiss::gpu::IVFBase::addEncodedVectorsToList_
https://github.com/facebookresearch/faiss/issues/2 faiss::gpu::IVFBase::copyInvertedListsFrom
https://github.com/facebookresearch/faiss/issues/3 faiss::gpu::GpuIndexIVFFlat::copyFrom
https://github.com/facebookresearch/faiss/issues/4 faiss::gpu::ToGpuCloner::clone_Index
https://github.com/facebookresearch/faiss/issues/5 faiss::gpu::index_cpu_to_gpu
0x... is located 0 bytes after 511-byte region
allocated by thread T0 here:
... faiss::MaybeOwnedVector<unsigned char>::resize
... faiss::read_index_up
```
The read is 512 bytes from a 511-byte heap allocation — boundary-minimal 1-byte over-read. The same shape fires deterministically across seeds with codes sizes 64..511 bytes (per-seed mapping available on request).
## After the patch
The same file now produces a clean throw at parse time, before the GPU clone path is reached:
```
Error in faiss::read_index_up(IOReader*, int)::<lambda(size_t)> at faiss/impl/index_read.cpp:1843:
Legacy IVFFlat inverted list 0: codes size 511 bytes does not match ids size 1 * code_size 512 = 512 bytes
```
The `memcpy` site in `IVFBase::addEncodedVectorsToList_` is never reached.
## Scope
The patched lane affects consumers who load a legacy `IvFl`/`IvFL`-format `.index` file **and** clone it to GPU via `faiss::gpu::index_cpu_to_gpu`. Pure CPU-side consumers of legacy files that don't exercise the GPU clone path were not demonstrated to hit this exact sink; this PR is the parity fix at the reader. The sink (`IVFBase::addEncodedVectorsToList_(idx_t, const void*, const idx_t*, idx_t)`) takes the codes as `const void*` with no length and physically cannot validate its own input — the reader is the only layer that knows both the buffer and its true size, which is why the fix lives there.
## Verification
Built and verified on the actual target hardware: NVIDIA GB10 (Blackwell, aarch64), driver 580.95.05, CUDA 13.0. Build flags `-fsanitize=address,undefined -fno-omit-frame-pointer -g -O1`. Harness invokes `faiss::read_index(IOReader*)` then `faiss::gpu::index_cpu_to_gpu(provider, device, index, opts)`, deterministic 3× repro of the original failure. Pre-patch: all 7 adversarial seeds (codes sizes 64, 128, 256, 320, 384, 511 bytes against expected 512; and one with `ids[0].size()=4`, codes=512 bytes against expected 2048) fire `heap-buffer-overflow READ`. Post-patch: all 7 fire the new `FAISS_THROW_IF_NOT_FMT` at parse time and the GPU clone path is never reached.
## Affected file
`faiss/impl/index_read.cpp` — 27 insertions, 0 removals. No interface change, no on-disk format change. Well-formed legacy files (where `codes.size() == ids.size() * code_size`, which the writer preserves) are unaffected.
## Follow-up (out of scope here)
The deeper fix — deprecating the legacy `IvFl`/`IvFL` format in favor of the validated current `IwFl` format — is a maintainer policy call rather than something to bundle in this PR. Happy to file a separate discussion issue if useful.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Pull Request resolved: https://github.com/facebookresearch/faiss/pull/5259
Reviewed By: alibeklfc
Differential Revision: D111114356
Pulled By: mnorris11
fbshipit-source-id: b246153be808e25ab72461bd87cc40549a95fbfc
This commit is contained in:
committed by
meta-codesync[bot]
parent
5c1afeed96
commit
79f59edcfc
@@ -1889,9 +1889,28 @@ std::unique_ptr<Index> read_index_up(IOReader* f, int io_flags) {
|
||||
ivfl->code_size = ivfl->d * sizeof(float);
|
||||
ArrayInvertedLists* ail = set_array_invlist(ivfl.get(), ids);
|
||||
|
||||
// Legacy IVF serialized ids and codes as separate vectors.
|
||||
// Check not required in default IVF, due to single sizes vector.
|
||||
auto validate_legacy_codes_size = [&](size_t i) {
|
||||
const size_t expected_codes_bytes = mul_no_overflow(
|
||||
ail->ids[i].size(),
|
||||
ivfl->code_size,
|
||||
"legacy IVFFlat inverted list codes");
|
||||
FAISS_THROW_IF_NOT_FMT(
|
||||
ail->codes[i].size() == expected_codes_bytes,
|
||||
"Legacy IVFFlat inverted list %zu: codes size %zu bytes "
|
||||
"does not match ids size %zu * code_size %zu = %zu bytes",
|
||||
i,
|
||||
ail->codes[i].size(),
|
||||
ail->ids[i].size(),
|
||||
(size_t)ivfl->code_size,
|
||||
expected_codes_bytes);
|
||||
};
|
||||
|
||||
if (h == fourcc("IvFL")) {
|
||||
for (size_t i = 0; i < ivfl->nlist; i++) {
|
||||
READVECTOR(ail->codes[i]);
|
||||
validate_legacy_codes_size(i);
|
||||
}
|
||||
} else { // old format
|
||||
for (size_t i = 0; i < ivfl->nlist; i++) {
|
||||
@@ -1899,6 +1918,7 @@ std::unique_ptr<Index> read_index_up(IOReader* f, int io_flags) {
|
||||
READVECTOR(vec);
|
||||
ail->codes[i].resize(vec.size() * sizeof(float));
|
||||
memcpy(ail->codes[i].data(), vec.data(), ail->codes[i].size());
|
||||
validate_legacy_codes_size(i);
|
||||
}
|
||||
}
|
||||
idx = std::move(ivfl);
|
||||
|
||||
Reference in New Issue
Block a user