fix(table): return error instead of fatalpanic when opening a corrupt sst (#2333)
**Description**
Fixes [#2201](https://github.com/dgraph-io/badger/issues/2201)
`db.Open()` crashes with a `runtime.fatalpanic` when an SSTable's footer
is corrupted (e.g. zeroed), instead of returning an error.
Looking at the stack trace of issue description, it looks like following
could be the reason for runtime termination,
An SST footer is laid out as `[index][indexLen][checksum][checksumLen]`,
with the two lengths stored as 4-byte big-endian values. When the footer
is zeroed, `checksumLen` and `indexLen` both read as `0`, so we read
empty checksum and index bytes. The empty protobuf checksum unmarshals
to `Sum = 0`, and the CRC32C of an empty index is also `0`, so the `0 ==
0` verification passes. The code then parses the empty index as
flatbuffers and reads an out-of-bounds offset into a slice, which
panics. `initBiggestAndSmallest` catches that panic, collects debug
info, then deliberately re-panics — and that re-panic escapes the
table-loading goroutine in `newLevelsController` (which has no
`recover()`), so the runtime terminates the process.
Changes:
- `table/table.go`: `initBiggestAndSmallest` returns an error (instead
of re-panicking) after collecting debug info, and `initIndex` validates
the footer's `checksumLen`/`indexLen`, rejecting `<= 0` (an empty
checksum/index false-passes the `0 == 0` checksum check and then panics
parsing empty flatbuffers) and `>= tableSize` (the length can't describe
a region as large as the whole file — it would make the footer's read
offset negative and slice out of bounds).
- `levels.go`: added a `recover()` in the table-loading goroutine as a
safety net for any panic that still escapes the table package.
**Checklist**
- [x] Code compiles correctly and linting passes locally
- [x] Tests added for new functionality, or regression tests for bug
fixes added as applicable
---------
Co-authored-by: Matthew McNeely <matthew.mcneely@gmail.com>
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
diff --git a/db_corrupt_test.go b/db_corrupt_test.go
new file mode 100644
index 0000000..04ac700
--- /dev/null
+++ b/db_corrupt_test.go
@@ -0,0 +1,105 @@
+/*
+ * SPDX-FileCopyrightText: © 2017-2025 Istari Digital, Inc.
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+package badger
+
+import (
+ "fmt"
+ "os"
+ "path/filepath"
+ "strings"
+ "testing"
+
+ "github.com/stretchr/testify/require"
+)
+
+// writeDBWithSST creates a DB, writes enough keys to flush an on-disk SSTable,
+// closes it, and returns the dir, the options used, and the first .sst path.
+func writeDBWithSST(t *testing.T) (dir string, opts Options, sstPath string) {
+ t.Helper()
+
+ dir, err := os.MkdirTemp("", "badger-test")
+ require.NoError(t, err)
+ opts = getTestOptions(dir)
+
+ db, err := Open(opts)
+ require.NoError(t, err)
+ for i := 0; i < 5000; i++ {
+ err := db.Update(func(txn *Txn) error {
+ return txn.Set(
+ []byte(fmt.Sprintf("key:%08d", i)),
+ []byte(fmt.Sprintf("value:%08d", i)))
+ })
+ require.NoError(t, err)
+ }
+ require.NoError(t, db.Close())
+
+ entries, err := os.ReadDir(dir)
+ require.NoError(t, err)
+ for _, e := range entries {
+ if strings.HasSuffix(e.Name(), ".sst") {
+ return dir, opts, filepath.Join(dir, e.Name())
+ }
+ }
+ t.Fatal("expected at least one SST on disk")
+ return dir, opts, ""
+}
+
+// TestOpenWithCorruptedFooterReturnsError mirrors the issue #2201 repro: the
+// footer/index region is zeroed, which must be rejected by the footer
+// validation and surfaced as a graceful Open error (not a fatalpanic).
+func TestOpenWithCorruptedFooterReturnsError(t *testing.T) {
+ dir, opts, sstPath := writeDBWithSST(t)
+ defer removeDir(dir)
+
+ data, err := os.ReadFile(sstPath)
+ require.NoError(t, err)
+ corruptSize := 300
+ if len(data) < corruptSize {
+ corruptSize = len(data)
+ }
+ for i := len(data) - corruptSize; i < len(data); i++ {
+ data[i] = 0
+ }
+ require.NoError(t, os.WriteFile(sstPath, data, 0644))
+
+ _, err = Open(opts)
+ require.Error(t, err)
+ require.Contains(t, err.Error(), "Data corrupted")
+}
+
+// TestOpenWithCorruptedBlockDataSucceeds corrupts only block data (the middle
+// of the file), leaving the footer and index intact. Block data is read
+// lazily, so Open must succeed rather than crash or error.
+func TestOpenWithCorruptedBlockDataSucceeds(t *testing.T) {
+ dir, opts, sstPath := writeDBWithSST(t)
+ defer removeDir(dir)
+
+ data, err := os.ReadFile(sstPath)
+ require.NoError(t, err)
+ mid := len(data) / 2
+ for i := mid; i < mid+100 && i < len(data); i++ {
+ data[i] = 0xFF
+ }
+ require.NoError(t, os.WriteFile(sstPath, data, 0644))
+
+ db, err := Open(opts)
+ require.NoError(t, err)
+ require.NoError(t, db.Close())
+}
+
+// TestOpenWithTinySSTableReturnsError truncates an SST below the 4-byte footer.
+// initIndex now rejects it with a footer-validation error, which must surface
+// as a graceful Open error (not a crash) through the table package.
+func TestOpenWithTinySSTableReturnsError(t *testing.T) {
+ dir, opts, sstPath := writeDBWithSST(t)
+ defer removeDir(dir)
+
+ require.NoError(t, os.Truncate(sstPath, 2))
+
+ _, err := Open(opts)
+ require.Error(t, err)
+ require.Contains(t, err.Error(), "Data corrupted")
+}
diff --git a/levels.go b/levels.go
index e951c8a..de4cfc0 100644
--- a/levels.go
+++ b/levels.go
@@ -14,6 +14,7 @@
"math"
"math/rand"
"os"
+ "runtime/debug"
"sort"
"strings"
"sync"
@@ -119,9 +120,21 @@
go func(fname string, tf TableManifest) {
var rerr error
defer func() {
+ if r := recover(); r != nil {
+ rerr = fmt.Errorf("error opening table %s: %v\n%s", fname, r, debug.Stack())
+ }
throttle.Done(rerr)
numOpened.Add(1)
}()
+ // tables is sized by opt.MaxLevels, and nothing upstream constrains
+ // the level recorded in the manifest, so reject an out-of-range level
+ // here rather than letting the append below panic.
+ if int(tf.Level) >= len(tables) {
+ rerr = fmt.Errorf(
+ "manifest records table %s at level %d, but MaxLevels is %d",
+ fname, tf.Level, len(tables))
+ return
+ }
dk, err := db.registry.DataKey(tf.KeyID)
if err != nil {
rerr = y.Wrapf(err, "Error while reading datakey")
@@ -149,9 +162,11 @@
return
}
- mu.Lock()
- tables[tf.Level] = append(tables[tf.Level], t)
- mu.Unlock()
+ func() {
+ mu.Lock()
+ defer mu.Unlock()
+ tables[tf.Level] = append(tables[tf.Level], t)
+ }()
}(fname, tf)
}
if err := throttle.Finish(); err != nil {
diff --git a/levels_test.go b/levels_test.go
index 39ab149..f20e592 100644
--- a/levels_test.go
+++ b/levels_test.go
@@ -1679,3 +1679,60 @@
require.Equal(t, ErrKeyNotFound, err, "deleted foo must not reappear after compaction")
require.NoError(t, db.Close())
}
+
+// TestOpenWithLevelBeyondMaxLevels covers a manifest that records a table at a
+// level at or beyond opt.MaxLevels. The tables slice in newLevelsController is
+// sized by MaxLevels, so the append used to panic with an index-out-of-range.
+// That panic is now recovered by the table-opening goroutine, and if the mutex
+// guarding tables were released without a defer the recover would leave it held
+// and every remaining goroutine would park on it, hanging Open forever with no
+// error. Assert instead that Open returns promptly with a usable message.
+func TestOpenWithLevelBeyondMaxLevels(t *testing.T) {
+ dir, err := os.MkdirTemp("", "badger-test")
+ require.NoError(t, err)
+ defer removeDir(dir)
+
+ // Build enough on-disk tables to exceed the size-3 throttle, so a leaked
+ // mutex would strand the goroutines behind it rather than surfacing early.
+ opts := getTestOptions(dir).WithNumCompactors(0)
+ opts.MemTableSize = 1 << 20
+ opts.BaseTableSize = 1 << 18
+ opts.ValueThreshold = 32
+
+ db, err := Open(opts)
+ require.NoError(t, err)
+ for i := 0; i < 40000; i++ {
+ require.NoError(t, db.Update(func(txn *Txn) error {
+ return txn.Set([]byte(fmt.Sprintf("key:%08d", i)), []byte(fmt.Sprintf("val:%08d", i)))
+ }))
+ }
+ require.NoError(t, db.Close())
+
+ // Rewrite the manifest so every table claims level 6.
+ mfFile, mf, err := helpOpenOrCreateManifestFile(dir, false, 0, 0, opts)
+ require.NoError(t, err)
+ var changes []*pb.ManifestChange
+ for id, tm := range mf.Tables {
+ changes = append(changes, newDeleteChange(id))
+ changes = append(changes, newCreateChange(id, 6, tm.KeyID, tm.Compression))
+ }
+ require.Greater(t, len(changes), 0)
+ require.NoError(t, mfFile.addChanges(changes, opts))
+ require.NoError(t, mfFile.close())
+
+ reopen := getTestOptions(dir).WithNumCompactors(0).WithMaxLevels(3)
+ reopen.MemTableSize = 1 << 20
+ reopen.BaseTableSize = 1 << 18
+ reopen.ValueThreshold = 32
+
+ errCh := make(chan error, 1)
+ go func() { _, e := Open(reopen); errCh <- e }()
+
+ select {
+ case err := <-errCh:
+ require.Error(t, err)
+ require.Contains(t, err.Error(), "MaxLevels")
+ case <-time.After(30 * time.Second):
+ t.Fatal("Open hung instead of returning an error")
+ }
+}
diff --git a/table/builder_test.go b/table/builder_test.go
index a9c1798..cb073a5 100644
--- a/table/builder_test.go
+++ b/table/builder_test.go
@@ -19,6 +19,7 @@
"github.com/dgraph-io/badger/v4/pb"
"github.com/dgraph-io/badger/v4/y"
"github.com/dgraph-io/ristretto/v2"
+ "github.com/dgraph-io/ristretto/v2/z"
)
func TestTableIndex(t *testing.T) {
@@ -133,14 +134,25 @@
opts := Options{BlockSize: 4 << 10, Compression: options.ZSTD}
tbl := buildTestTable(t, keyPrefix, 1000, opts)
defer func() { require.NoError(t, tbl.DecrRef()) }()
- mf := tbl.MmapFile
- t.Run("with correct decompression algo", func(t *testing.T) {
- _, err := OpenTable(mf, opts)
+
+ // OpenTable takes ownership of the mmap it is given, so each subtest must
+ // hand it a fresh mmap rather than reusing tbl.MmapFile.
+ openMmap := func() *z.MmapFile {
+ mf, err := z.OpenMmapFile(tbl.Filename(), os.O_RDONLY, 0)
require.NoError(t, err)
+ return mf
+ }
+
+ t.Run("with correct decompression algo", func(t *testing.T) {
+ mf := openMmap()
+ table, err := OpenTable(mf, opts)
+ require.NoError(t, err)
+ require.NoError(t, table.Close(-1))
})
t.Run("with incorrect decompression algo", func(t *testing.T) {
// Set incorrect compression algorithm.
opts.Compression = options.Snappy
+ mf := openMmap()
_, err := OpenTable(mf, opts)
require.Error(t, err)
})
diff --git a/table/initindex_test.go b/table/initindex_test.go
new file mode 100644
index 0000000..4bfae84
--- /dev/null
+++ b/table/initindex_test.go
@@ -0,0 +1,142 @@
+/*
+ * SPDX-FileCopyrightText: © 2017-2025 Istari Digital, Inc.
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+package table
+
+import (
+ "bytes"
+ "testing"
+
+ "github.com/stretchr/testify/require"
+ "google.golang.org/protobuf/proto"
+
+ "github.com/dgraph-io/badger/v4/pb"
+ "github.com/dgraph-io/badger/v4/y"
+ "github.com/dgraph-io/ristretto/v2/z"
+)
+
+// buildFooterData lays out an SST footer of the form:
+//
+// [index bytes][indexLen: 4 bytes][checksum bytes][checksumLen: 4 bytes]
+//
+// indexLen and checksumLen are written as BigEndian uint32 (matching
+// y.U32ToBytes/y.BytesToU32), independently of the actual index/checksum byte
+// lengths, so we can craft footers whose length fields disagree with reality.
+func buildFooterData(indexLen uint32, index []byte, checksumLen uint32, checksum []byte) []byte {
+ var b bytes.Buffer
+ b.Write(index)
+ b.Write(y.U32ToBytes(indexLen))
+ b.Write(checksum)
+ b.Write(y.U32ToBytes(checksumLen))
+ return b.Bytes()
+}
+
+// newTestTable returns an in-memory Table over data without going through
+// OpenTable/OpenInMemoryTable, so initIndex can be called directly.
+func newTestTable(data []byte) *Table {
+ return &Table{
+ MmapFile: &z.MmapFile{Data: data},
+ tableSize: len(data),
+ opt: &Options{},
+ }
+}
+
+func TestInitIndexFooterValidation(t *testing.T) {
+ // A non-empty, well-formed checksum so the checksumLen check and
+ // proto.Unmarshal both pass and we can reach the indexLen check.
+ validChecksum, err := proto.Marshal(&pb.Checksum{Sum: 1})
+ require.NoError(t, err)
+ checksumLen := uint32(len(validChecksum))
+
+ // The footer is read by walking backwards from EOF (readPos starts at
+ // tableSize), so each length must fit in the bytes remaining *before*
+ // readPos — not merely be smaller than the whole table. checksumLen == 0 is
+ // legal (a zero checksum marshals to no bytes), so only the bounds are
+ // checked for checksumLen; a zero indexLen is always corruption since a
+ // table always has at least one block.
+ tests := []struct {
+ name string
+ data []byte
+ wantErr string
+ }{
+ {
+ name: "tableSize<4",
+ data: make([]byte, 2),
+ wantErr: "invalid table size in footer. Data corrupted",
+ },
+ {
+ // 8-byte file since checksumLen and indexLen are both uint32.
+ // checksumLen field reads 8, exceeding the 4 bytes
+ // remaining before readPos.
+ name: "checksumLen=tableSize",
+ data: buildFooterData(0, nil, 8, nil),
+ wantErr: "invalid checksum length in footer. Data corrupted",
+ },
+ {
+ name: "checksumLen=tableSize-1",
+ data: buildFooterData(0, nil, 7, nil),
+ wantErr: "invalid checksum length in footer. Data corrupted",
+ },
+ {
+ name: "checksumLen>tableSize",
+ data: buildFooterData(0, nil, 100, nil),
+ wantErr: "invalid checksum length in footer. Data corrupted",
+ },
+ {
+ // checksumLen=0 is legal, so it must fall through to the empty
+ // index length related error.
+ name: "checksumLen=0 falls through to indexLen",
+ data: buildFooterData(0, nil, 0, nil),
+ wantErr: "invalid index length in footer. Data corrupted",
+ },
+ {
+ name: "indexLen=0",
+ data: buildFooterData(0, nil, checksumLen, validChecksum),
+ wantErr: "invalid index length in footer. Data corrupted",
+ },
+ {
+ // tableSize = [indexLen:4] + [checksum:len(validChecksum)] + [checksumLen:4].
+ name: "indexLen=tableSize",
+ data: buildFooterData(
+ uint32(4+len(validChecksum)+4), nil, checksumLen, validChecksum),
+ wantErr: "invalid index length in footer. Data corrupted",
+ },
+ {
+ name: "indexLen>tableSize",
+ data: buildFooterData(1000, nil, checksumLen, validChecksum),
+ wantErr: "invalid index length in footer. Data corrupted",
+ },
+ }
+
+ for _, tc := range tests {
+ t.Run(tc.name, func(t *testing.T) {
+ _, err := newTestTable(tc.data).initIndex()
+ require.Error(t, err)
+ require.Equal(t, tc.wantErr, err.Error())
+ })
+ }
+}
+
+// TestInitBiggestAndSmallestRecoversFlatbuffersPanic verifies the core of the
+// issue #2201 fix: when initIndex panics while parsing a corrupted index
+// (panic #1, "P1"), initBiggestAndSmallest's recover defer must convert that
+// panic into an error instead of re-panicking. The footer here is
+// self-consistent — a valid checksum over a garbage index — so every
+// validation check passes and the panic happens inside the flatbuffers parse,
+// exactly like the original issue (checksum false-pass → GetRootAsTableIndex).
+func TestInitBiggestAndSmallestRecoversFlatbuffersPanic(t *testing.T) {
+ garbageIndex := bytes.Repeat([]byte{0xFF}, 20)
+ checksum, err := proto.Marshal(&pb.Checksum{
+ Algo: pb.Checksum_CRC32C,
+ Sum: y.CalculateChecksum(garbageIndex, pb.Checksum_CRC32C),
+ })
+ require.NoError(t, err)
+ data := buildFooterData(
+ uint32(len(garbageIndex)), garbageIndex, uint32(len(checksum)), checksum)
+
+ err = newTestTable(data).initBiggestAndSmallest()
+ require.Error(t, err)
+ require.Contains(t, err.Error(), "initIndex crashed")
+}
diff --git a/table/table.go b/table/table.go
index f671333..7c5c5e6 100644
--- a/table/table.go
+++ b/table/table.go
@@ -269,6 +269,7 @@
// BlockSize is used to compute the approximate size of the decompressed
// block. It should not be zero if the table is compressed.
if opts.BlockSize == 0 && opts.Compression != options.None {
+ _ = mf.Close(-1)
return nil, errors.New("Block size cannot be zero")
}
fileInfo, err := mf.Fd.Stat()
@@ -295,6 +296,7 @@
t.ref.Store(1)
if err := t.initBiggestAndSmallest(); err != nil {
+ _ = mf.Close(-1)
return nil, y.Wrapf(err, "failed to initialize table")
}
@@ -331,14 +333,20 @@
return t, nil
}
-func (t *Table) initBiggestAndSmallest() error {
+func (t *Table) initBiggestAndSmallest() (err error) {
// This defer will help gathering debugging info in case initIndex crashes.
defer func() {
if r := recover(); r != nil {
- // Use defer for printing info because there may be an intermediate panic.
var debugBuf bytes.Buffer
+
+ // Best-effort debug collection: a panic here must not escape and
+ // re-trigger the fatalpanic we are trying to fix. Setting err inside
+ // this defer (rather than after the reads below) ensures whatever
+ // debug info was gathered so far is attached even if one of the
+ // reads panics.
defer func() {
- panic(fmt.Sprintf("%s\n== Recovered ==\n", debugBuf.String()))
+ _ = recover()
+ err = fmt.Errorf("initIndex crashed: %v\n%s", r, debugBuf.String())
}()
// Get the count of null bytes at the end of file. This is to make sure if there was an
@@ -378,15 +386,15 @@
indexLen := int(y.BytesToU32(buf))
fmt.Fprintf(&debugBuf, "indexLen: %d ", indexLen)
- // Read index.
- readPos -= t.indexLen
+ // Read index. Use the local indexLen read above so the debug output
+ // describes the same region it actually reads.
+ readPos -= indexLen
t.indexStart = readPos
- indexData := t.readNoFail(readPos, t.indexLen)
+ indexData := t.readNoFail(readPos, indexLen)
fmt.Fprintf(&debugBuf, "index: %v ", indexData)
}
}()
- var err error
var ko *fb.BlockOffset
if ko, err = t.initIndex(); err != nil {
return y.Wrapf(err, "failed to read index.")
@@ -420,11 +428,18 @@
readPos := t.tableSize
// Read checksum len from the last 4 bytes.
+ if readPos < 4 {
+ return nil, errors.New("invalid table size in footer. Data corrupted")
+ }
readPos -= 4
buf := t.readNoFail(readPos, 4)
checksumLen := int(y.BytesToU32(buf))
- if checksumLen < 0 {
- return nil, errors.New("checksum length less than zero. Data corrupted")
+ // checksumLen == 0 is legal (a zero checksum marshals to nothing), so only
+ // reject negative lengths and lengths that don't fit in the bytes remaining
+ // before readPos. The < 0 guard catches a uint32 value >= 2^31 wrapping to a
+ // negative int on 32-bit platforms.
+ if checksumLen < 0 || checksumLen > readPos {
+ return nil, errors.New("invalid checksum length in footer. Data corrupted")
}
// Read checksum.
@@ -436,9 +451,17 @@
}
// Read index size from the footer.
+ if readPos < 4 {
+ return nil, errors.New("invalid table size in footer. Data corrupted")
+ }
readPos -= 4
buf = t.readNoFail(readPos, 4)
t.indexLen = int(y.BytesToU32(buf))
+ // A table always has at least one block, so a zero indexLen is always
+ // corruption, and the index must fit in the bytes remaining before readPos.
+ if t.indexLen <= 0 || t.indexLen > readPos {
+ return nil, errors.New("invalid index length in footer. Data corrupted")
+ }
// Read index.
readPos -= t.indexLen
@@ -469,7 +492,9 @@
t.hasBloomFilter = len(index.BloomFilterBytes()) > 0
var bo fb.BlockOffset
- y.AssertTrue(index.Offsets(&bo, 0))
+ if !index.Offsets(&bo, 0) {
+ return nil, errors.New("failed to read block offset from index. Data corrupted")
+ }
return &bo, nil
}
