fix: avoid SIGBUS when reading non-std series segment files (influxda…

…ta#24509) Some series files which are smaller than the standard sizes cause SIGBUS in influx_inspect and influxd, because entry iteration walks onto mapped memory not backed by the the file. Avoid walking off the end of the file while iterating series entries in oddly sized files. closes influxdata#24508 Co-authored-by: Geoffrey Wossum <[email protected]>
chengshiwen · Aug 11, 2024 · 9645aa2 · 9645aa2
1 parent 23dc01c
commit 9645aa2
Show file tree

Hide file tree

Showing 5 changed files with 78 additions and 48 deletions.
diff --git a/cmd/influx_inspect/verify/seriesfile/verify.go b/cmd/influx_inspect/verify/seriesfile/verify.go
@@ -184,7 +184,7 @@ func (v Verify) VerifySegment(segmentPath string, ids map[uint64]IDData) (valid
 	v.Logger = v.Logger.With(zap.String("segment", segmentName))
 	v.Logger.Info("Verifying segment")
 
-	// Open up the segment and grab it's data.
+	// Open up the segment and grab its data.
 	segmentID, err := tsdb.ParseSeriesSegmentFilename(segmentName)
 	if err != nil {
 		return false, err
@@ -195,7 +195,8 @@ func (v Verify) VerifySegment(segmentPath string, ids map[uint64]IDData) (valid
 		return false, nil
 	}
 	defer segment.Close()
-	buf := newBuffer(segment.Data())
+	// Only walk the file as it exists, not the whole mapping which may be bigger than the file.
+	buf := newBuffer(segment.Data()[:segment.Size()])
 
 	defer func() {
 		if rec := recover(); rec != nil {

diff --git a/tsdb/series_file.go b/tsdb/series_file.go
@@ -366,9 +366,9 @@ func AppendSeriesKey(dst []byte, name []byte, tags models.Tags) []byte {
 }
 
 // ReadSeriesKey returns the series key from the beginning of the buffer.
-func ReadSeriesKey(data []byte) (key, remainder []byte) {
+func ReadSeriesKey(data []byte) (key []byte) {
 	sz, n := binary.Uvarint(data)
-	return data[:int(sz)+n], data[int(sz)+n:]
+	return data[:int(sz)+n]
 }
 
 func ReadSeriesKeyLen(data []byte) (sz int, remainder []byte) {

diff --git a/tsdb/series_partition.go b/tsdb/series_partition.go
@@ -508,7 +508,7 @@ func (p *SeriesPartition) seriesKeyByOffset(offset int64) []byte {
 			continue
 		}
 
-		key, _ := ReadSeriesKey(segment.Slice(pos + SeriesEntryHeaderSize))
+		key := ReadSeriesKey(segment.Slice(pos + SeriesEntryHeaderSize))
 		return key
 	}
 

diff --git a/tsdb/series_segment.go b/tsdb/series_segment.go
@@ -92,6 +92,12 @@ func CreateSeriesSegment(id uint16, path string) (*SeriesSegment, error) {
 // Open memory maps the data file at the file's path.
 func (s *SeriesSegment) Open() error {
 	if err := func() (err error) {
+		st, err := os.Stat(s.path)
+		if err != nil {
+			return fmt.Errorf("cannot stat %s: %w", s.path, err)
+		}
+		s.size = uint32(st.Size())
+
 		// Memory map file data.
 		if s.data, err = mmap.Map(s.path, int64(SeriesSegmentSize(s.id))); err != nil {
 			return err
@@ -120,14 +126,16 @@ func (s *SeriesSegment) Path() string { return s.path }
 // InitForWrite initializes a write handle for the segment.
 // This is only used for the last segment in the series file.
 func (s *SeriesSegment) InitForWrite() (err error) {
-	// Only calculcate segment data size if writing.
-	for s.size = uint32(SeriesSegmentHeaderSize); s.size < uint32(len(s.data)); {
-		flag, _, _, sz := ReadSeriesEntry(s.data[s.size:])
+	// Only recalculate segment data size if writing.
+	var size uint32
+	for size = uint32(SeriesSegmentHeaderSize); size < s.size; {
+		flag, _, _, sz := ReadSeriesEntry(s.data[size:s.size])
 		if !IsValidSeriesEntryFlag(flag) {
 			break
 		}
-		s.size += uint32(sz)
+		size += uint32(sz)
 	}
+	s.size = size
 
 	// Open file handler for writing & seek to end of data.
 	if s.file, err = os.OpenFile(s.path, os.O_WRONLY|os.O_CREATE, 0666); err != nil {
@@ -243,8 +251,8 @@ func (s *SeriesSegment) MaxSeriesID() uint64 {
 
 // ForEachEntry executes fn for every entry in the segment.
 func (s *SeriesSegment) ForEachEntry(fn func(flag uint8, id uint64, offset int64, key []byte) error) error {
-	for pos := uint32(SeriesSegmentHeaderSize); pos < uint32(len(s.data)); {
-		flag, id, key, sz := ReadSeriesEntry(s.data[pos:])
+	for pos := uint32(SeriesSegmentHeaderSize); pos < s.size; {
+		flag, id, key, sz := ReadSeriesEntry(s.data[pos:s.size])
 		if !IsValidSeriesEntryFlag(flag) {
 			break
 		}
@@ -337,7 +345,7 @@ func ReadSeriesKeyFromSegments(a []*SeriesSegment, offset int64) []byte {
 		return nil
 	}
 	buf := segment.Slice(pos)
-	key, _ := ReadSeriesKey(buf)
+	key := ReadSeriesKey(buf)
 	return key
 }
 
@@ -416,16 +424,22 @@ func (hdr *SeriesSegmentHeader) WriteTo(w io.Writer) (n int64, err error) {
 }
 
 func ReadSeriesEntry(data []byte) (flag uint8, id uint64, key []byte, sz int64) {
+	if len(data) <= 0 {
+		return 0, 0, nil, 1
+	}
 	// If flag byte is zero then no more entries exist.
 	flag, data = uint8(data[0]), data[1:]
 	if !IsValidSeriesEntryFlag(flag) {
 		return 0, 0, nil, 1
 	}
 
+	if len(data) < 8 {
+		return 0, 0, nil, 1
+	}
 	id, data = binary.BigEndian.Uint64(data), data[8:]
 	switch flag {
 	case SeriesEntryInsertFlag:
-		key, _ = ReadSeriesKey(data)
+		key = ReadSeriesKey(data)
 	}
 	return flag, id, key, int64(SeriesEntryHeaderSize + len(key))
 }

diff --git a/tsdb/series_segment_test.go b/tsdb/series_segment_test.go
@@ -4,6 +4,7 @@ import (
 	"bytes"
 	"os"
 	"path/filepath"
+	"strconv"
 	"testing"
 
 	"github.com/google/go-cmp/cmp"
@@ -142,45 +143,59 @@ func TestSeriesSegmentHeader(t *testing.T) {
 }
 
 func TestSeriesSegment_PartialWrite(t *testing.T) {
-	dir, cleanup := MustTempDir()
-	defer cleanup()
+	for extraSegs := uint64(2000); extraSegs < 4000; extraSegs++ {
+		func() {
+			dir, cleanup := MustTempDir()
+			defer cleanup()
 
-	// Create a new initial segment (4mb) and initialize for writing.
-	segment, err := tsdb.CreateSeriesSegment(0, filepath.Join(dir, "0000"))
-	if err != nil {
-		t.Fatal(err)
-	} else if err := segment.InitForWrite(); err != nil {
-		t.Fatal(err)
-	}
-	defer segment.Close()
+			// Create a new initial segment (4mb) and initialize for writing.
+			segment, err := tsdb.CreateSeriesSegment(0, filepath.Join(dir, "0000"))
+			if err != nil {
+				t.Fatal(err)
+			} else if err := segment.InitForWrite(); err != nil {
+				t.Fatal(err)
+			}
+			defer segment.Close()
 
-	// Write two entries.
-	if _, err := segment.WriteLogEntry(tsdb.AppendSeriesEntry(nil, tsdb.SeriesEntryInsertFlag, 1, tsdb.AppendSeriesKey(nil, []byte("A"), nil))); err != nil {
-		t.Fatal(err)
-	} else if _, err := segment.WriteLogEntry(tsdb.AppendSeriesEntry(nil, tsdb.SeriesEntryInsertFlag, 2, tsdb.AppendSeriesKey(nil, []byte("B"), nil))); err != nil {
-		t.Fatal(err)
-	}
-	sz := segment.Size()
-	entrySize := len(tsdb.AppendSeriesEntry(nil, tsdb.SeriesEntryInsertFlag, 2, tsdb.AppendSeriesKey(nil, []byte("B"), nil)))
+			// Write two entries.
+			if _, err := segment.WriteLogEntry(tsdb.AppendSeriesEntry(nil, tsdb.SeriesEntryInsertFlag, 1, tsdb.AppendSeriesKey(nil, []byte("A"), nil))); err != nil {
+				t.Fatal(err)
+			}
 
-	// Close segment.
-	if err := segment.Close(); err != nil {
-		t.Fatal(err)
-	}
+			// Adding intermediary segments in between "A" and "B" is to try and induce a SIGBUS
+			// when the file truncation backs over a page.
+			for i := uint64(0); i < extraSegs; i++ {
+				if _, err := segment.WriteLogEntry(tsdb.AppendSeriesEntry(nil, tsdb.SeriesEntryInsertFlag, 1+i, tsdb.AppendSeriesKey(nil, []byte(strconv.Itoa(int(i))), nil))); err != nil {
+					t.Fatal(err)
+				}
+			}
 
-	// Truncate at each point and reopen.
-	for i := entrySize; i > 0; i-- {
-		if err := os.Truncate(filepath.Join(dir, "0000"), sz-int64(entrySize-i)); err != nil {
-			t.Fatal(err)
-		}
-		segment := tsdb.NewSeriesSegment(0, filepath.Join(dir, "0000"))
-		if err := segment.Open(); err != nil {
-			t.Fatal(err)
-		} else if err := segment.InitForWrite(); err != nil {
-			t.Fatal(err)
-		} else if err := segment.Close(); err != nil {
-			t.Fatal(err)
-		}
+			if _, err := segment.WriteLogEntry(tsdb.AppendSeriesEntry(nil, tsdb.SeriesEntryInsertFlag, 2+extraSegs, tsdb.AppendSeriesKey(nil, []byte("B"), nil))); err != nil {
+				t.Fatal(err)
+			}
+			sz := segment.Size()
+			entrySize := len(tsdb.AppendSeriesEntry(nil, tsdb.SeriesEntryInsertFlag, 2+extraSegs, tsdb.AppendSeriesKey(nil, []byte("B"), nil)))
+
+			// Close segment.
+			if err := segment.Close(); err != nil {
+				t.Fatal(err)
+			}
+
+			// Truncate at each point and reopen.
+			for i := entrySize; i > 0; i-- {
+				if err := os.Truncate(filepath.Join(dir, "0000"), sz-int64(entrySize-i)); err != nil {
+					t.Fatal(err)
+				}
+				segment := tsdb.NewSeriesSegment(0, filepath.Join(dir, "0000"))
+				if err := segment.Open(); err != nil {
+					t.Fatal(err)
+				} else if err := segment.InitForWrite(); err != nil {
+					t.Fatal(err)
+				} else if err := segment.Close(); err != nil {
+					t.Fatal(err)
+				}
+			}
+		}()
 	}
 }