fix(volume): avoid nil-deref when needle map loader errors (#9694) (#9697)

* fix(volume): avoid nil-deref when needle map loader errors

A corrupt .idx whose size is not a multiple of NeedleMapEntrySize sends
the read-only load path into NewSortedFileNeedleMap, which returns
(*SortedFileNeedleMap)(nil) when reverseWalkIndexFile rejects the file.
The multi-value assignment `v.nm, err = NewSortedFileNeedleMap(...)`
parks that typed-nil pointer in the v.nm NeedleMapper interface, so the
subsequent `v.nm != nil` guard still passes — and the post-load
MaxNeedleEnd structural check dispatches through the promoted mapMetric
accessor on a nil receiver, segfaulting the whole volume server at
load time.

Reset v.nm explicitly after every loader failure so the interface is
truly nil, and skip the MaxNeedleEnd check when err is non-nil since
the value would come from a partial walk anyway. NewLevelDbNeedleMap
has the same typed-nil-on-error shape and is fixed the same way.

* fix(volume): close indexFile when needle map load errors

Pre-fix the typed-nil v.nm path either leaked indexFile silently
(SortedFileNeedleMap.Close had a nil-receiver early return) or crashed
(LevelDbNeedleMap.Close had no such guard). With v.nm cleared to nil
on error, the defer cleanup no longer calls Close at all, so the
LoadCompactNeedleMap success-with-error path now also leaks indexFile.
Close indexFile explicitly on each loader error to keep ownership
balanced.

* trim comments
This commit is contained in:
Chris Lu
2026-05-26 16:56:49 -07:00
committed by GitHub
parent 4f17c6661a
commit 1e91a99f79
2 changed files with 57 additions and 2 deletions
+16 -2
View File
@@ -279,9 +279,14 @@ func (v *Volume) load(alsoLoadIndex bool, createDatIfMissing bool, needleMapKind
// (issue #8928). The check piggybacks on MaxNeedleEnd, which the load
// walks below populate without a second linear scan.
// Loaders can return a typed-nil pointer with err set; assigning that
// to v.nm yields a non-nil interface over a nil receiver. Clear v.nm
// and close indexFile so the defer cleanup keys off v.nm cleanly.
if v.noWriteOrDelete || v.noWriteCanDelete {
if v.nm, err = NewSortedFileNeedleMap(v.IndexFileName(), indexFile, v.Version()); err != nil {
glog.V(0).Infof("loading sorted db %s error: %v", v.FileName(".sdx"), err)
v.nm = nil
indexFile.Close()
}
} else {
switch needleMapKind {
@@ -293,6 +298,8 @@ func (v *Volume) load(alsoLoadIndex bool, createDatIfMissing bool, needleMapKind
glog.V(2).Infoln("loading memory index", v.FileName(".idx"), "to memory")
if v.nm, err = LoadCompactNeedleMap(indexFile, v.Version()); err != nil {
glog.V(0).Infof("loading index %s to memory error: %v", v.FileName(".idx"), err)
v.nm = nil
indexFile.Close()
}
}
case NeedleMapLevelDb:
@@ -309,6 +316,8 @@ func (v *Volume) load(alsoLoadIndex bool, createDatIfMissing bool, needleMapKind
glog.V(0).Infoln("loading leveldb index", v.FileName(".ldb"))
if v.nm, err = NewLevelDbNeedleMap(v.FileName(".ldb"), indexFile, opts, v.ldbTimeout, v.Version()); err != nil {
glog.V(0).Infof("loading leveldb %s error: %v", v.FileName(".ldb"), err)
v.nm = nil
indexFile.Close()
}
}
case NeedleMapLevelDbMedium:
@@ -325,6 +334,8 @@ func (v *Volume) load(alsoLoadIndex bool, createDatIfMissing bool, needleMapKind
glog.V(0).Infoln("loading leveldb medium index", v.FileName(".ldb"))
if v.nm, err = NewLevelDbNeedleMap(v.FileName(".ldb"), indexFile, opts, v.ldbTimeout, v.Version()); err != nil {
glog.V(0).Infof("loading leveldb %s error: %v", v.FileName(".ldb"), err)
v.nm = nil
indexFile.Close()
}
}
case NeedleMapLevelDbLarge:
@@ -341,6 +352,8 @@ func (v *Volume) load(alsoLoadIndex bool, createDatIfMissing bool, needleMapKind
glog.V(0).Infoln("loading leveldb large index", v.FileName(".ldb"))
if v.nm, err = NewLevelDbNeedleMap(v.FileName(".ldb"), indexFile, opts, v.ldbTimeout, v.Version()); err != nil {
glog.V(0).Infof("loading leveldb %s error: %v", v.FileName(".ldb"), err)
v.nm = nil
indexFile.Close()
}
}
}
@@ -351,8 +364,9 @@ func (v *Volume) load(alsoLoadIndex bool, createDatIfMissing bool, needleMapKind
// MaximumNeedleEnd, so this is just a numeric comparison — no extra
// disk I/O. A violation marks the volume read-only so a corrupt
// .idx left over from a crashed batched write does not silently
// power vacuum to drop reachable data. See issue #8928.
if !v.HasRemoteFile() && v.nm != nil && v.DataBackend != nil {
// power vacuum to drop reachable data. See issue #8928. err == nil
// guards against a partial-walk MaximumNeedleEnd.
if err == nil && !v.HasRemoteFile() && v.nm != nil && v.DataBackend != nil {
if datSize, _, statErr := v.DataBackend.GetStat(); statErr == nil && datSize > 0 {
if maxEnd := v.nm.MaxNeedleEnd(); maxEnd > datSize {
v.noWriteOrDelete = true
@@ -0,0 +1,41 @@
package storage
import (
"os"
"testing"
"github.com/seaweedfs/seaweedfs/weed/storage/needle"
"github.com/seaweedfs/seaweedfs/weed/storage/super_block"
)
// Corrupt .idx made NewSortedFileNeedleMap return a typed-nil into v.nm;
// the post-load MaxNeedleEnd check then segfaulted on the nil receiver.
func TestLoad_CorruptIdx_NoSegfault(t *testing.T) {
dir := t.TempDir()
v, err := NewVolume(dir, dir, "", 1, NeedleMapInMemory, &super_block.ReplicaPlacement{}, &needle.TTL{}, 0, needle.GetCurrentVersion(), 0, 0)
if err != nil {
t.Fatalf("create volume: %v", err)
}
if _, _, _, err := v.writeNeedle2(newRandomNeedle(1), true, false); err != nil {
t.Fatalf("seed write: %v", err)
}
v.PersistReadOnly(true) // reload goes through SortedFileNeedleMap
v.Close()
// Truncate .idx to a non-aligned size so the walk rejects it.
idxPath := VolumeFileName(dir, "", 1) + ".idx"
st, err := os.Stat(idxPath)
if err != nil {
t.Fatalf("stat idx: %v", err)
}
if err := os.Truncate(idxPath, st.Size()-1); err != nil {
t.Fatalf("truncate idx: %v", err)
}
// Pre-fix this panicked inside (*mapMetric).MaxNeedleEnd.
v2, err := NewVolume(dir, dir, "", 1, NeedleMapInMemory, &super_block.ReplicaPlacement{}, &needle.TTL{}, 0, needle.GetCurrentVersion(), 0, 0)
if err == nil {
v2.Close()
}
}