Repository: cilium/ebpf
Pull request #530 (2021-12-17 11:09:23)

Adopt Go 1.18 fuzzing and fix btf crashes
commit 83a63d0faabbe471a09d073fd29a656663745722
Author: Lorenz Bauer <lmb@cloudflare.com>
Date:   Fri Dec 17 10:57:25 2021 +0000

    btf: fix missing error return in parseFuncInfos, parseLineInfos
    
    Found by fuzzing!

commit 8e15fa985e29c3fc6b979757dd73c30bcf0ee4ef
Author: Lorenz Bauer <lmb@cloudflare.com>
Date:   Fri Dec 17 10:49:13 2021 +0000

    btf: move loadExtInfos to ext_info.go

commit e1971f6a55822cb44e66df3db6d5c2d818783610
Author: Lorenz Bauer <lmb@cloudflare.com>
Date:   Fri Dec 17 11:06:56 2021 +0000

    elf, btf: make fuzzing 1.18 ready
    
    Migrate the existing (and bitrotted) fuzz harness to the built
    in fuzzing support available from Go 1.18.
    
    This makes some refactoring of BTF necessary, since it's currently not
    easy to parse ext infos on their own.


lmb: You can try this by running `go1.18beta1 test -fuzz=Spec -run XXX -v` in the project root.
ti-mo: I've found another crasher in extinfos, let's not merge this yet.

Patch:
diff --git a/elf_reader_fuzz.go b/elf_reader_fuzz.go
deleted file mode 100644
index 5f4e0a0..0000000
--- a/elf_reader_fuzz.go
+++ /dev/null
@@ -1,22 +0,0 @@
-//go:build gofuzz
-// +build gofuzz
-
-// Use with https://github.com/dvyukov/go-fuzz
-
-package ebpf
-
-import "bytes"
-
-func FuzzLoadCollectionSpec(data []byte) int {
-	spec, err := LoadCollectionSpecFromReader(bytes.NewReader(data))
-	if err != nil {
-		if spec != nil {
-			panic("spec is not nil")
-		}
-		return 0
-	}
-	if spec == nil {
-		panic("spec is nil")
-	}
-	return 1
-}
diff --git a/internal/btf/btf.go b/internal/btf/btf.go
index 83af0ea..4b88559 100644
--- a/internal/btf/btf.go
+++ b/internal/btf/btf.go
@@ -156,68 +156,32 @@ func loadSpecFromELF(file *internal.SafeELFFile, variableOffsets map[variable]ui
 		return spec, nil
 	}
 
-	if err := spec.loadExtInfos(btfExtSection); err != nil {
-		return nil, fmt.Errorf("can't parse ext info: %w", err)
-	}
-
-	return spec, nil
-}
-
-// loadExtInfos parses the .BTF.ext section and populates spec's funcInfo,
-// lineInfo and coreRelos fields using the contents.
-func (spec *Spec) loadExtInfos(sec *elf.Section) error {
-	// Open unbuffered section reader. binary.Read() calls io.ReadFull on
-	// the header structs, resulting in one syscall per header.
-	r := sec.Open()
-
-	extHeader, err := parseBTFExtHeader(r, spec.byteOrder)
-	if err != nil {
-		return fmt.Errorf("parsing BTF extension header: %w", err)
-	}
-
-	coreHeader, err := parseBTFExtCoreHeader(r, spec.byteOrder, extHeader)
-	if err != nil {
-		return fmt.Errorf("parsing BTF CO-RE header: %w", err)
+	if btfExtSection.ReaderAt == nil {
+		return nil, fmt.Errorf("compressed ext_info is not supported")
 	}
 
-	buf := internal.NewBufferedSectionReader(sec, extHeader.funcInfoStart(), int64(extHeader.FuncInfoLen))
-	funcInfos, err := parseFuncInfos(buf, spec.byteOrder, spec.strings)
+	extInfo, err := loadExtInfos(btfExtSection, file.ByteOrder, spec.strings)
 	if err != nil {
-		return fmt.Errorf("parsing BTF function info: %w", err)
-	}
-
-	buf = internal.NewBufferedSectionReader(sec, extHeader.lineInfoStart(), int64(extHeader.LineInfoLen))
-	lineInfos, err := parseLineInfos(buf, spec.byteOrder, spec.strings)
-	if err != nil {
-		return fmt.Errorf("parsing BTF line info: %w", err)
-	}
-
-	relos := make(map[string]CoreRelos)
-	if coreHeader != nil && coreHeader.CoreReloOff > 0 && coreHeader.CoreReloLen > 0 {
-		buf = internal.NewBufferedSectionReader(sec, extHeader.coreReloStart(coreHeader), int64(coreHeader.CoreReloLen))
-		relos, err = parseCoreRelos(buf, spec.byteOrder, spec.strings)
-		if err != nil {
-			return fmt.Errorf("parsing CO-RE relocation info: %w", err)
-		}
+		return nil, fmt.Errorf("can't parse ext info: %w", err)
 	}
 
-	if err := spec.splitExtInfos(funcInfos, lineInfos, relos); err != nil {
-		return fmt.Errorf("linking funcInfos and lineInfos: %w", err)
+	if err := spec.splitExtInfos(extInfo); err != nil {
+		return nil, fmt.Errorf("linking funcInfos and lineInfos: %w", err)
 	}
 
-	return nil
+	return spec, nil
 }
 
 // splitExtInfos takes FuncInfos, LineInfos and CoreRelos indexed by section and
 // transforms them to be indexed by function. Retrieves function names from
 // the BTF spec.
-func (spec *Spec) splitExtInfos(sfi map[string]FuncInfos, sli map[string]LineInfos, scr map[string]CoreRelos) error {
+func (spec *Spec) splitExtInfos(info *extInfo) error {
 
 	ofi := make(map[string]FuncInfo)
 	oli := make(map[string]LineInfos)
 	ocr := make(map[string]CoreRelos)
 
-	for secName, secFuncs := range sfi {
+	for secName, secFuncs := range info.funcInfos {
 		// Collect functions from each section and organize them by name.
 		for _, fi := range secFuncs {
 			name, err := fi.Name(spec)
@@ -235,7 +199,7 @@ func (spec *Spec) splitExtInfos(sfi map[string]FuncInfos, sli map[string]LineInf
 		}
 
 		// Attribute LineInfo records to their respective functions, if any.
-		if lines := sli[secName]; lines != nil {
+		if lines := info.lineInfos[secName]; lines != nil {
 			for _, li := range lines {
 				fi := secFuncs.funcForOffset(li.InsnOff)
 				if fi == nil {
@@ -256,7 +220,7 @@ func (spec *Spec) splitExtInfos(sfi map[string]FuncInfos, sli map[string]LineInf
 		}
 
 		// Attribute CO-RE relocations to their respective functions, if any.
-		if relos := scr[secName]; relos != nil {
+		if relos := info.relos[secName]; relos != nil {
 			for _, r := range relos {
 				fi := secFuncs.funcForOffset(r.insnOff)
 				if fi == nil {
diff --git a/internal/btf/ext_info.go b/internal/btf/ext_info.go
index 5e05a3f..c4da1e4 100644
--- a/internal/btf/ext_info.go
+++ b/internal/btf/ext_info.go
@@ -5,11 +5,60 @@ import (
 	"errors"
 	"fmt"
 	"io"
+	"math"
 
 	"github.com/cilium/ebpf/asm"
 	"github.com/cilium/ebpf/internal"
 )
 
+// extInfo contains extended program metadata.
+//
+// It is indexed per section.
+type extInfo struct {
+	funcInfos map[string]FuncInfos
+	lineInfos map[string]LineInfos
+	relos     map[string]CoreRelos
+}
+
+// loadExtInfos parses the .BTF.ext section into its constituent parts.
+func loadExtInfos(r io.ReaderAt, bo binary.ByteOrder, strings stringTable) (*extInfo, error) {
+	// Open unbuffered section reader. binary.Read() calls io.ReadFull on
+	// the header structs, resulting in one syscall per header.
+	headerRd := io.NewSectionReader(r, 0, math.MaxInt64)
+	extHeader, err := parseBTFExtHeader(headerRd, bo)
+	if err != nil {
+		return nil, fmt.Errorf("parsing BTF extension header: %w", err)
+	}
+
+	coreHeader, err := parseBTFExtCoreHeader(headerRd, bo, extHeader)
+	if err != nil {
+		return nil, fmt.Errorf("parsing BTF CO-RE header: %w", err)
+	}
+
+	buf := internal.NewBufferedSectionReader(r, extHeader.funcInfoStart(), int64(extHeader.FuncInfoLen))
+	funcInfos, err := parseFuncInfos(buf, bo, strings)
+	if err != nil {
+		return nil, fmt.Errorf("parsing BTF function info: %w", err)
+	}
+
+	buf = internal.NewBufferedSectionReader(r, extHeader.lineInfoStart(), int64(extHeader.LineInfoLen))
+	lineInfos, err := parseLineInfos(buf, bo, strings)
+	if err != nil {
+		return nil, fmt.Errorf("parsing BTF line info: %w", err)
+	}
+
+	relos := make(map[string]CoreRelos)
+	if coreHeader != nil && coreHeader.CoreReloOff > 0 && coreHeader.CoreReloLen > 0 {
+		buf = internal.NewBufferedSectionReader(r, extHeader.coreReloStart(coreHeader), int64(coreHeader.CoreReloLen))
+		relos, err = parseCoreRelos(buf, bo, strings)
+		if err != nil {
+			return nil, fmt.Errorf("parsing CO-RE relocation info: %w", err)
+		}
+	}
+
+	return &extInfo{funcInfos, lineInfos, relos}, nil
+}
+
 // btfExtHeader is found at the start of the .BTF.ext section.
 type btfExtHeader struct {
 	Magic   uint16
@@ -227,6 +276,9 @@ func parseFuncInfos(r io.Reader, bo binary.ByteOrder, strings stringTable) (map[
 		if errors.Is(err, io.EOF) {
 			return result, nil
 		}
+		if err != nil {
+			return nil, err
+		}
 
 		records, err := parseFuncInfoRecords(r, bo, recordSize, infoHeader.NumInfo)
 		if err != nil {
@@ -317,6 +369,9 @@ func parseLineInfos(r io.Reader, bo binary.ByteOrder, strings stringTable) (map[
 		if errors.Is(err, io.EOF) {
 			return result, nil
 		}
+		if err != nil {
+			return nil, err
+		}
 
 		records, err := parseLineInfoRecords(r, bo, recordSize, infoHeader.NumInfo)
 		if err != nil {
@@ -391,6 +446,9 @@ func parseCoreRelos(r io.Reader, bo binary.ByteOrder, strings stringTable) (map[
 		if errors.Is(err, io.EOF) {
 			return result, nil
 		}
+		if err != nil {
+			return nil, err
+		}
 
 		records, err := parseCoreReloRecords(r, bo, recordSize, infoHeader.NumInfo, strings)
 		if err != nil {
diff --git a/internal/btf/fuzz.go b/internal/btf/fuzz.go
deleted file mode 100644
index 220b285..0000000
--- a/internal/btf/fuzz.go
+++ /dev/null
@@ -1,50 +0,0 @@
-//go:build gofuzz
-// +build gofuzz
-
-// Use with https://github.com/dvyukov/go-fuzz
-
-package btf
-
-import (
-	"bytes"
-	"encoding/binary"
-
-	"github.com/cilium/ebpf/internal"
-)
-
-func FuzzSpec(data []byte) int {
-	if len(data) < binary.Size(btfHeader{}) {
-		return -1
-	}
-
-	spec, err := loadNakedSpec(bytes.NewReader(data), internal.NativeEndian, nil, nil)
-	if err != nil {
-		if spec != nil {
-			panic("spec is not nil")
-		}
-		return 0
-	}
-	if spec == nil {
-		panic("spec is nil")
-	}
-	return 1
-}
-
-func FuzzExtInfo(data []byte) int {
-	if len(data) < binary.Size(btfExtHeader{}) {
-		return -1
-	}
-
-	table := stringTable("\x00foo\x00barfoo\x00")
-	info, err := parseExtInfo(bytes.NewReader(data), internal.NativeEndian, table)
-	if err != nil {
-		if info != nil {
-			panic("info is not nil")
-		}
-		return 0
-	}
-	if info == nil {
-		panic("info is nil")
-	}
-	return 1
-}


Test patch:
diff --git a/fuzz_test.go b/fuzz_test.go
new file mode 100644
index 0000000..dc32c78
--- /dev/null
+++ b/fuzz_test.go
@@ -0,0 +1,28 @@
+//go:build go1.18
+// +build go1.18
+
+package ebpf
+
+import (
+	"bytes"
+	"debug/elf"
+	"testing"
+)
+
+func FuzzLoadCollectionSpec(f *testing.F) {
+	f.Add([]byte(elf.ELFMAG))
+	f.Fuzz(func(t *testing.T, data []byte) {
+		if len(data) < len(elf.ELFMAG) {
+			t.Skip("input can't be valid ELF")
+		}
+
+		spec, err := LoadCollectionSpecFromReader(bytes.NewReader(data))
+		if err != nil {
+			if spec != nil {
+				t.Fatal("spec is not nil")
+			}
+		} else if spec == nil {
+			t.Fatal("spec is nil")
+		}
+	})
+}
diff --git a/internal/btf/fuzz_test.go b/internal/btf/fuzz_test.go
new file mode 100644
index 0000000..7c979ff
--- /dev/null
+++ b/internal/btf/fuzz_test.go
@@ -0,0 +1,68 @@
+//go:build go1.18
+// +build go1.18
+
+package btf
+
+import (
+	"bytes"
+	"encoding/binary"
+	"testing"
+
+	"github.com/cilium/ebpf/internal"
+)
+
+func FuzzSpec(f *testing.F) {
+	var buf bytes.Buffer
+	err := binary.Write(&buf, internal.NativeEndian, &btfHeader{
+		Magic:   btfMagic,
+		Version: 1,
+		HdrLen:  uint32(binary.Size(btfHeader{})),
+	})
+	if err != nil {
+		f.Fatal(err)
+	}
+	f.Add(buf.Bytes())
+	f.Fuzz(func(t *testing.T, data []byte) {
+		if len(data) < binary.Size(btfHeader{}) {
+			t.Skip("data is too short")
+		}
+
+		spec, err := loadRawSpec(bytes.NewReader(data), internal.NativeEndian, nil, nil)
+		if err != nil {
+			if spec != nil {
+				t.Fatal("spec is not nil")
+			}
+		} else if spec == nil {
+			t.Fatal("spec is nil")
+		}
+	})
+}
+
+func FuzzExtInfo(f *testing.F) {
+	var buf bytes.Buffer
+	err := binary.Write(&buf, internal.NativeEndian, &btfExtHeader{
+		Magic:   btfMagic,
+		Version: 1,
+		HdrLen:  uint32(binary.Size(btfExtHeader{})),
+	})
+	if err != nil {
+		f.Fatal(err)
+	}
+	f.Add(buf.Bytes(), []byte("\x00foo\x00barfoo\x00"))
+
+	f.Fuzz(func(t *testing.T, data, strings []byte) {
+		if len(data) < binary.Size(btfExtHeader{}) {
+			t.Skip("data is too short")
+		}
+
+		table := stringTable(strings)
+		info, err := loadExtInfos(bytes.NewReader(data), internal.NativeEndian, table)
+		if err != nil {
+			if info != nil {
+				t.Fatal("info is not nil")
+			}
+		} else if info == nil {
+			t.Fatal("info is nil")
+		}
+	})
+}
diff --git a/internal/btf/testdata/fuzz/FuzzExtInfo/50a33736610b4a0945179db4c8a88e8247b05fbb25f50ed81e5393baf29bc5bc b/internal/btf/testdata/fuzz/FuzzExtInfo/50a33736610b4a0945179db4c8a88e8247b05fbb25f50ed81e5393baf29bc5bc
new file mode 100644
index 0000000..fb5bb62
--- /dev/null
+++ b/internal/btf/testdata/fuzz/FuzzExtInfo/50a33736610b4a0945179db4c8a88e8247b05fbb25f50ed81e5393baf29bc5bc
@@ -0,0 +1,3 @@
+go test fuzz v1
+[]byte("\x9f\xeb\x01\x00\x1c\x00\x00\x00\x00\x00\x00\x00000\x10\x00\x00\x00\x000000\xf3\xff\xff\xff0\x00\x00\x00")
+[]byte("0")
diff --git a/internal/btf/testdata/fuzz/FuzzExtInfo/72534f53bd90cb52a017013499b11511535c1295bf0e22f856148c02454c323e b/internal/btf/testdata/fuzz/FuzzExtInfo/72534f53bd90cb52a017013499b11511535c1295bf0e22f856148c02454c323e
new file mode 100644
index 0000000..e7ab3fb
--- /dev/null
+++ b/internal/btf/testdata/fuzz/FuzzExtInfo/72534f53bd90cb52a017013499b11511535c1295bf0e22f856148c02454c323e
@@ -0,0 +1,3 @@
+go test fuzz v1
+[]byte("\x9f\xeb\x01\x00\x18\x00\x00\x00\x00\x00\x00\x000000000000000\x00\x00\x000000")
+[]byte("0")
diff --git a/internal/btf/testdata/fuzz/FuzzExtInfo/a87a26efa64ed50b598ae8e333301d57d5f234527730f042d68ccc736e90c9fa b/internal/btf/testdata/fuzz/FuzzExtInfo/a87a26efa64ed50b598ae8e333301d57d5f234527730f042d68ccc736e90c9fa
new file mode 100644
index 0000000..783bbb5
--- /dev/null
+++ b/internal/btf/testdata/fuzz/FuzzExtInfo/a87a26efa64ed50b598ae8e333301d57d5f234527730f042d68ccc736e90c9fa
@@ -0,0 +1,3 @@
+go test fuzz v1
+[]byte("\x9f\xeb\x01\x00\x1c\x00\x00\x00\x00\x00\x00\x000000\xe8\xff\xff\xff000000000\x00\x00\x00")
+[]byte("0")