fix for indextool --apply-killlists crashes repeatedly with SIGSEGV #4837

[popalot2]

Fix for issue fix for indextool --apply-killlists crashes repeatedly with SIGSEGV #4837
Issue no longer happens after fix, indextool --apply-killlists works fine.
Fix created by AI agent and was debugged and verified against my actual files, I am not claiming this is the best fix or that it is production ready. Do your own due diligence.

The main index was created with lookup format version 67, while the delta uses version 71.

indextool --apply-killlists calls IndexFiles_c::CheckHeader() and then uses GetVersion():

manticoresearch/src/indextool.cpp:883

For JSON .sph headers, CheckHeader() previously returned success without reading index_format_version. The version therefore remained at its default—currently 71.

The v67 .spt was consequently opened as v71. Version 71 added an 8-byte UUID lookup offset:

manticoresearch/src/docidlookup.cpp:995

That shifted the checkpoint table by eight bytes. Every checkpoint was then interpreted as:

• offset → base DocID
• next checkpoint’s base DocID → data offset

This explains the apparently valid DocID combined with the impossible row ID 4076938452. It also reproduces the bad .spt pointer offset seen in GDB.

The atomic operation in DeadRowMap_c::Set() is only where the corrupted row ID finally causes an out-of-bounds access. killlist.h and the atomic operation are not the root cause.

Also, m_iCount = 36330127 for the dead map is a count of DWORD elements, not bytes. Its actual size is approximately 145 MB. Row ID 4076938452 addresses approximately 510 MB from its base.

Affected components

• indextool --apply-killlists: directly affected, especially when processing indexes with different JSON-header format versions.
• searchd: not affected by this specific bug. Normal index loading parses index_format_version correctly before constructing lookup readers.
• indexer: not affected; it writes the current format and does not use this broken version-discovery path.
• Index rotation calls CheckHeader(), but only to validate which header exists; it does not consume the stale version in the failing manner.
• indextool --check: passes because its checker independently parses the JSON header version correctly at manticoresearch/src/indexcheck.cpp:646.

Fix

IndexFiles_c::CheckHeader() now parses JSON headers and stores the actual index_format_version:

manticoresearch/src/indexfiles.cpp:267

It also validates that the version is supported. Therefore the v67 target lookup is read as v67 and the v71 delta as v71.

No reindexing of the .spt files is required. They are structurally valid.

However, because the failed run had the .spm dead-row map writable and memory-mapped, restore the target .spm from a pre-crash backup before rerunning if possible. The crashing write
itself was outside the mapping, but earlier incorrect in-range row IDs could theoretically have set wrong dead bits.
is IndexFiles_c::CheckHeader() only used in indextool? why this problem didn't happen in other tools?

• No. IndexFiles_c::CheckHeader() has two active callers:

1. indextool --apply-killlists
2. Searchd’s index-rotation file checker

The key difference is what callers do with the result.

Why indextool crashed

ApplyKilllists() uses both CheckHeader() and the version stored by it:

IndexFiles_c tIndexFiles (...);

if ( !tIndexFiles.CheckHeader() )
continue;

tIndex.m_uVersion = tIndexFiles.GetVersion();

manticoresearch/src/indextool.cpp:883

Before the fix, a JSON header caused CheckHeader() to return true without updating m_uVersion. It remained at the compiled-in current version, 71.

That incorrect version was passed directly into:

LookupReaderIterator_c ( lookupData, tIndex.m_uVersion );

The v67 lookup was therefore decoded using the v71 layout, producing the corrupt row ID.

Why searchd rotation did not hit it

The other caller is:

manticoresearch/src/index_rotator.cpp:20

It effectively does:

IndexFiles_c(sPath).CheckHeader()
IndexFiles_c(sPath).CheckHeader(".new")

Here IndexFiles_c is temporary. Rotation only needs the boolean result to determine whether the current and/or .new header exists and is acceptable. It does not call GetVersion() on that
object.

Therefore the stale version value was discarded immediately.

The fix still improves rotation because it now performs real JSON version validation, but rotation was not feeding that version into a lookup reader.

Why normal searchd operations worked

Searchd does not depend on IndexFiles_c::CheckHeader() to determine the loaded index version. Its normal index loader parses the complete JSON .sph header independently:

manticoresearch/src/sphinx.cpp:9767

It correctly executes:

m_uVersion = (DWORD)Int (
tBson.ChildByName ( "index_format_version" )
);

Later, searchd creates lookup readers using that correctly loaded m_uVersion.

Consequently, searchd can apply kills internally with KillByLookup() safely—the same underlying kill logic is used, but the supplied format version is correct.

Why indextool --check worked

The index checker has another independent JSON-header parser:

manticoresearch/src/indexcheck.cpp:646

It also reads index_format_version correctly. This is why both indexes passed --check: the files were valid, and the checker did not use the defective version-discovery path.

Why indexer was unaffected

indexer creates current-format indexes. It does not use this CheckHeader() → GetVersion() sequence to reopen mixed-version lookup files.

So the precise vulnerable pattern was:

JSON header
→ IndexFiles_c::CheckHeader()
→ GetVersion()
→ LookupReaderIterator_c

Among active callers, only indextool --apply-killlists used that complete sequence.

[popalot2]

I am not insisting this is the correct way to fix it - feel free to fix in anyway you like.
The PR is just an example, to demonstrate the place that causes the bug in #4837 and was used as a way to diagnose the crash and see if that was indeed the problem.