Repository: cilium/ebpf
Pull request #861 (2022-11-20 16:10:07)

btf: fix some split BTF shortcomings
Fix some outright bugs and some annoyances with split BTF support. It's still not entirely great, but this is better than nothing.

Requirement for #859.

lmb: API changes:
```
--- old.txt	2022-11-20 16:21:35.946436516 +0000
+++ new.txt	2022-11-20 16:21:29.014452875 +0000
@@ -21,6 +21,9 @@

 FUNCTIONS

+func ClearCachedKernelSpec()
+    ClearCachedKernelSpec flushes any cached kernel type information.
+
 func LoadSpecAndExtInfosFromReader(rd io.ReaderAt) (*Spec, *ExtInfos, error)
     LoadSpecAndExtInfosFromReader reads from an ELF.

@@ -270,11 +273,9 @@
 func (h *Handle) Info() (*HandleInfo, error)
     Info returns metadata about the handle.

-func (h *Handle) Spec(base *Spec) (*Spec, error)
+func (h *Handle) Spec() (*Spec, error)
     Spec parses the kernel BTF into Go types.

-    base is used to decode split BTF and may be nil.
-
 type HandleInfo struct {
 	// ID of this handle in the kernel. The ID is only valid as long as the
 	// associated handle is kept alive.
```

Patch:
diff --git a/btf/btf.go b/btf/btf.go
index a11bc03..6716fe7 100644
--- a/btf/btf.go
+++ b/btf/btf.go
@@ -11,6 +11,7 @@ import (
 	"math"
 	"os"
 	"reflect"
+	"sync"
 
 	"github.com/cilium/ebpf/internal"
 	"github.com/cilium/ebpf/internal/sys"
@@ -34,9 +35,9 @@ type Spec struct {
 	// Data from .BTF.
 	strings *stringTable
 
-	// All types contained by the spec. For the base type, the position of
-	// a type in the slice is its ID.
-	types types
+	// All types contained by the spec, not including types from the base in
+	// case the spec was parsed from split BTF.
+	types []Type
 
 	// Type IDs indexed by type.
 	typeIDs map[Type]TypeID
@@ -263,20 +264,67 @@ func indexTypes(types []Type, typeIDOffset TypeID) (map[Type]TypeID, map[essenti
 // Defaults to /sys/kernel/btf/vmlinux and falls back to scanning the file system
 // for vmlinux ELFs. Returns an error wrapping ErrNotSupported if BTF is not enabled.
 func LoadKernelSpec() (*Spec, error) {
+	spec, _, err := kernelSpec()
+	return spec, err
+}
+
+var kernelBTF struct {
+	sync.RWMutex
+	spec *Spec
+	// True if the spec was read from an ELF instead of raw BTF in /sys.
+	fallback bool
+}
+
+// FlushKernelSpec removes any cached kernel type information.
+func FlushKernelSpec() {
+	kernelBTF.Lock()
+	defer kernelBTF.Unlock()
+
+	kernelBTF.spec, kernelBTF.fallback = nil, false
+}
+
+func kernelSpec() (*Spec, bool, error) {
+	kernelBTF.RLock()
+	spec, fallback := kernelBTF.spec, kernelBTF.fallback
+	kernelBTF.RUnlock()
+
+	if spec == nil {
+		kernelBTF.Lock()
+		defer kernelBTF.Unlock()
+
+		spec, fallback = kernelBTF.spec, kernelBTF.fallback
+	}
+
+	if spec != nil {
+		return spec.Copy(), fallback, nil
+	}
+
+	spec, fallback, err := loadKernelSpec()
+	if err != nil {
+		return nil, false, err
+	}
+
+	kernelBTF.spec, kernelBTF.fallback = spec, fallback
+	return spec.Copy(), fallback, nil
+}
+
+func loadKernelSpec() (_ *Spec, fallback bool, _ error) {
 	fh, err := os.Open("/sys/kernel/btf/vmlinux")
 	if err == nil {
 		defer fh.Close()
 
-		return loadRawSpec(fh, internal.NativeEndian, nil, nil)
+		spec, err := loadRawSpec(fh, internal.NativeEndian, nil, nil)
+		return spec, false, err
 	}
 
 	file, err := findVMLinux()
 	if err != nil {
-		return nil, err
+		return nil, false, err
 	}
 	defer file.Close()
 
-	return loadSpecFromELF(file)
+	spec, err := loadSpecFromELF(file)
+	return spec, true, err
 }
 
 // findVMLinux scans multiple well-known paths for vmlinux kernel images.
@@ -421,11 +469,7 @@ func fixupDatasec(types []Type, sectionSizes map[string]uint32, offsets map[symb
 func (s *Spec) Copy() *Spec {
 	types := copyTypes(s.types, nil)
 
-	typeIDOffset := TypeID(0)
-	if len(s.types) != 0 {
-		typeIDOffset = s.typeIDs[s.types[0]]
-	}
-	typeIDs, typesByName := indexTypes(types, typeIDOffset)
+	typeIDs, typesByName := indexTypes(types, s.firstTypeID())
 
 	// NB: Other parts of spec are not copied since they are immutable.
 	return &Spec{
@@ -452,7 +496,14 @@ func (sw sliceWriter) Write(p []byte) (int, error) {
 // Returns an error wrapping ErrNotFound if a Type with the given ID
 // does not exist in the Spec.
 func (s *Spec) TypeByID(id TypeID) (Type, error) {
-	return s.types.ByID(id)
+	firstID := s.firstTypeID()
+	lastID := firstID + TypeID(len(s.types))
+
+	if id < firstID || id >= lastID {
+		return nil, fmt.Errorf("expected type ID between %d and %d, got %d: %w", firstID, lastID, id, ErrNotFound)
+	}
+
+	return s.types[id-firstID], nil
 }
 
 // TypeID returns the ID for a given Type.
@@ -574,6 +625,14 @@ func (s *Spec) TypeByName(name string, typ interface{}) error {
 	return nil
 }
 
+// firstTypeID returns the first type ID or zero.
+func (s *Spec) firstTypeID() TypeID {
+	if len(s.types) > 0 {
+		return s.typeIDs[s.types[0]]
+	}
+	return 0
+}
+
 // LoadSplitSpecFromReader loads split BTF from a reader.
 //
 // Types from base are used to resolve references in the split BTF.
@@ -606,138 +665,6 @@ func (iter *TypesIterator) Next() bool {
 	return true
 }
 
-// Handle is a reference to BTF loaded into the kernel.
-type Handle struct {
-	fd *sys.FD
-
-	// Size of the raw BTF in bytes.
-	size uint32
-}
-
-// NewHandle loads BTF into the kernel.
-//
-// Returns ErrNotSupported if BTF is not supported.
-func NewHandle(spec *Spec) (*Handle, error) {
-	if spec.byteOrder != nil && spec.byteOrder != internal.NativeEndian {
-		return nil, fmt.Errorf("can't load %s BTF on %s", spec.byteOrder, internal.NativeEndian)
-	}
-
-	enc := newEncoder(kernelEncoderOptions, newStringTableBuilderFromTable(spec.strings))
-
-	for _, typ := range spec.types {
-		_, err := enc.Add(typ)
-		if err != nil {
-			return nil, fmt.Errorf("add %s: %w", typ, err)
-		}
-	}
-
-	btf, err := enc.Encode()
-	if err != nil {
-		return nil, fmt.Errorf("marshal BTF: %w", err)
-	}
-
-	return newHandleFromRawBTF(btf)
-}
-
-func newHandleFromRawBTF(btf []byte) (*Handle, error) {
-	if uint64(len(btf)) > math.MaxUint32 {
-		return nil, errors.New("BTF exceeds the maximum size")
-	}
-
-	attr := &sys.BtfLoadAttr{
-		Btf:     sys.NewSlicePointer(btf),
-		BtfSize: uint32(len(btf)),
-	}
-
-	fd, err := sys.BtfLoad(attr)
-	if err == nil {
-		return &Handle{fd, attr.BtfSize}, nil
-	}
-
-	if err := haveBTF(); err != nil {
-		return nil, err
-	}
-
-	logBuf := make([]byte, 64*1024)
-	attr.BtfLogBuf = sys.NewSlicePointer(logBuf)
-	attr.BtfLogSize = uint32(len(logBuf))
-	attr.BtfLogLevel = 1
-
-	// Up until at least kernel 6.0, the BTF verifier does not return ENOSPC
-	// if there are other verification errors. ENOSPC is only returned when
-	// the BTF blob is correct, a log was requested, and the provided buffer
-	// is too small.
-	_, ve := sys.BtfLoad(attr)
-	return nil, internal.ErrorWithLog("load btf", err, logBuf, errors.Is(ve, unix.ENOSPC))
-}
-
-// NewHandleFromID returns the BTF handle for a given id.
-//
-// Prefer calling [ebpf.Program.Handle] or [ebpf.Map.Handle] if possible.
-//
-// Returns ErrNotExist, if there is no BTF with the given id.
-//
-// Requires CAP_SYS_ADMIN.
-func NewHandleFromID(id ID) (*Handle, error) {
-	fd, err := sys.BtfGetFdById(&sys.BtfGetFdByIdAttr{
-		Id: uint32(id),
-	})
-	if err != nil {
-		return nil, fmt.Errorf("get FD for ID %d: %w", id, err)
-	}
-
-	info, err := newHandleInfoFromFD(fd)
-	if err != nil {
-		_ = fd.Close()
-		return nil, err
-	}
-
-	return &Handle{fd, info.size}, nil
-}
-
-// Spec parses the kernel BTF into Go types.
-//
-// base is used to decode split BTF and may be nil.
-func (h *Handle) Spec(base *Spec) (*Spec, error) {
-	var btfInfo sys.BtfInfo
-	btfBuffer := make([]byte, h.size)
-	btfInfo.Btf, btfInfo.BtfSize = sys.NewSlicePointerLen(btfBuffer)
-
-	if err := sys.ObjInfo(h.fd, &btfInfo); err != nil {
-		return nil, err
-	}
-
-	var baseTypes types
-	var baseStrings *stringTable
-	if base != nil {
-		baseTypes = base.types
-		baseStrings = base.strings
-	}
-
-	return loadRawSpec(bytes.NewReader(btfBuffer), internal.NativeEndian, baseTypes, baseStrings)
-}
-
-// Close destroys the handle.
-//
-// Subsequent calls to FD will return an invalid value.
-func (h *Handle) Close() error {
-	if h == nil {
-		return nil
-	}
-
-	return h.fd.Close()
-}
-
-// FD returns the file descriptor for the handle.
-func (h *Handle) FD() int {
-	return h.fd.Int()
-}
-
-// Info returns metadata about the handle.
-func (h *Handle) Info() (*HandleInfo, error) {
-	return newHandleInfoFromFD(h.fd)
-}
-
 func marshalBTF(types interface{}, strings []byte, bo binary.ByteOrder) []byte {
 	const minHeaderLength = 24
 
diff --git a/btf/handle.go b/btf/handle.go
index 3d540e4..9a864d1 100644
--- a/btf/handle.go
+++ b/btf/handle.go
@@ -1,14 +1,155 @@
 package btf
 
 import (
+	"bytes"
 	"errors"
 	"fmt"
+	"math"
 	"os"
 
+	"github.com/cilium/ebpf/internal"
 	"github.com/cilium/ebpf/internal/sys"
 	"github.com/cilium/ebpf/internal/unix"
 )
 
+// Handle is a reference to BTF loaded into the kernel.
+type Handle struct {
+	fd *sys.FD
+
+	// Size of the raw BTF in bytes.
+	size uint32
+
+	needsKernelBase bool
+}
+
+// NewHandle loads BTF into the kernel.
+//
+// Returns ErrNotSupported if BTF is not supported.
+func NewHandle(spec *Spec) (*Handle, error) {
+	if spec.byteOrder != nil && spec.byteOrder != internal.NativeEndian {
+		return nil, fmt.Errorf("can't load %s BTF on %s", spec.byteOrder, internal.NativeEndian)
+	}
+
+	enc := newEncoder(kernelEncoderOptions, newStringTableBuilderFromTable(spec.strings))
+
+	for _, typ := range spec.types {
+		_, err := enc.Add(typ)
+		if err != nil {
+			return nil, fmt.Errorf("add %s: %w", typ, err)
+		}
+	}
+
+	btf, err := enc.Encode()
+	if err != nil {
+		return nil, fmt.Errorf("marshal BTF: %w", err)
+	}
+
+	return newHandleFromRawBTF(btf)
+}
+
+func newHandleFromRawBTF(btf []byte) (*Handle, error) {
+	if uint64(len(btf)) > math.MaxUint32 {
+		return nil, errors.New("BTF exceeds the maximum size")
+	}
+
+	attr := &sys.BtfLoadAttr{
+		Btf:     sys.NewSlicePointer(btf),
+		BtfSize: uint32(len(btf)),
+	}
+
+	fd, err := sys.BtfLoad(attr)
+	if err == nil {
+		return &Handle{fd, attr.BtfSize, false}, nil
+	}
+
+	if err := haveBTF(); err != nil {
+		return nil, err
+	}
+
+	logBuf := make([]byte, 64*1024)
+	attr.BtfLogBuf = sys.NewSlicePointer(logBuf)
+	attr.BtfLogSize = uint32(len(logBuf))
+	attr.BtfLogLevel = 1
+
+	// Up until at least kernel 6.0, the BTF verifier does not return ENOSPC
+	// if there are other verification errors. ENOSPC is only returned when
+	// the BTF blob is correct, a log was requested, and the provided buffer
+	// is too small.
+	_, ve := sys.BtfLoad(attr)
+	return nil, internal.ErrorWithLog("load btf", err, logBuf, errors.Is(ve, unix.ENOSPC))
+}
+
+// NewHandleFromID returns the BTF handle for a given id.
+//
+// Prefer calling [ebpf.Program.Handle] or [ebpf.Map.Handle] if possible.
+//
+// Returns ErrNotExist, if there is no BTF with the given id.
+//
+// Requires CAP_SYS_ADMIN.
+func NewHandleFromID(id ID) (*Handle, error) {
+	fd, err := sys.BtfGetFdById(&sys.BtfGetFdByIdAttr{
+		Id: uint32(id),
+	})
+	if err != nil {
+		return nil, fmt.Errorf("get FD for ID %d: %w", id, err)
+	}
+
+	info, err := newHandleInfoFromFD(fd)
+	if err != nil {
+		_ = fd.Close()
+		return nil, err
+	}
+
+	return &Handle{fd, info.size, info.IsModule()}, nil
+}
+
+// Spec parses the kernel BTF into Go types.
+func (h *Handle) Spec() (*Spec, error) {
+	var btfInfo sys.BtfInfo
+	btfBuffer := make([]byte, h.size)
+	btfInfo.Btf, btfInfo.BtfSize = sys.NewSlicePointerLen(btfBuffer)
+
+	if err := sys.ObjInfo(h.fd, &btfInfo); err != nil {
+		return nil, err
+	}
+
+	if !h.needsKernelBase {
+		return loadRawSpec(bytes.NewReader(btfBuffer), internal.NativeEndian, nil, nil)
+	}
+
+	base, fallback, err := kernelSpec()
+	if err != nil {
+		return nil, fmt.Errorf("load BTF base: %w", err)
+	}
+
+	if fallback {
+		return nil, fmt.Errorf("can't load split BTF without access to /sys")
+	}
+
+	return loadRawSpec(bytes.NewReader(btfBuffer), internal.NativeEndian, base.types, base.strings)
+}
+
+// Close destroys the handle.
+//
+// Subsequent calls to FD will return an invalid value.
+func (h *Handle) Close() error {
+	if h == nil {
+		return nil
+	}
+
+	return h.fd.Close()
+}
+
+// FD returns the file descriptor for the handle.
+func (h *Handle) FD() int {
+	return h.fd.Int()
+}
+
+// Info returns metadata about the handle.
+func (h *Handle) Info() (*HandleInfo, error) {
+	return newHandleInfoFromFD(h.fd)
+}
+
 // HandleInfo describes a Handle.
 type HandleInfo struct {
 	// ID of this handle in the kernel. The ID is only valid as long as the
diff --git a/link/tracing.go b/link/tracing.go
index 5c07b77..e26cc91 100644
--- a/link/tracing.go
+++ b/link/tracing.go
@@ -48,7 +48,7 @@ func AttachFreplace(targetProg *ebpf.Program, name string, prog *ebpf.Program) (
 		}
 		defer btfHandle.Close()
 
-		spec, err := btfHandle.Spec(nil)
+		spec, err := btfHandle.Spec()
 		if err != nil {
 			return nil, err
 		}
diff --git a/linker.go b/linker.go
index 2a2c1b6..d97a44b 100644
--- a/linker.go
+++ b/linker.go
@@ -4,7 +4,6 @@ import (
 	"encoding/binary"
 	"errors"
 	"fmt"
-	"sync"
 
 	"github.com/cilium/ebpf/asm"
 	"github.com/cilium/ebpf/btf"
@@ -88,9 +87,12 @@ func applyRelocations(insns asm.Instructions, target *btf.Spec, bo binary.ByteOr
 		bo = internal.NativeEndian
 	}
 
-	target, err := maybeLoadKernelBTF(target)
-	if err != nil {
-		return err
+	if target == nil {
+		var err error
+		target, err = btf.LoadKernelSpec()
+		if err != nil {
+			return fmt.Errorf("load kernel spec: %w", err)
+		}
 	}
 
 	fixups, err := btf.CORERelocate(relos, target, bo)
@@ -216,29 +218,3 @@ func fixupProbeReadKernel(ins *asm.Instruction) {
 		ins.Constant = int64(asm.FnProbeReadStr)
 	}
 }
-
-var kernelBTF struct {
-	sync.Mutex
-	spec *btf.Spec
-}
-
-// maybeLoadKernelBTF loads the current kernel's BTF if spec is nil, otherwise
-// it returns spec unchanged.
-//
-// The kernel BTF is cached for the lifetime of the process.
-func maybeLoadKernelBTF(spec *btf.Spec) (*btf.Spec, error) {
-	if spec != nil {
-		return spec, nil
-	}
-
-	kernelBTF.Lock()
-	defer kernelBTF.Unlock()
-
-	if kernelBTF.spec != nil {
-		return kernelBTF.spec, nil
-	}
-
-	var err error
-	kernelBTF.spec, err = btf.LoadKernelSpec()
-	return kernelBTF.spec, err
-}
diff --git a/prog.go b/prog.go
index 4df2432..e0e3a7a 100644
--- a/prog.go
+++ b/prog.go
@@ -846,17 +846,14 @@ func findTargetInKernel(name string, progType ProgramType, attachType AttachType
 		return nil, 0, errUnrecognizedAttachType
 	}
 
-	// maybeLoadKernelBTF may return external BTF if /sys/... is not available.
-	// Ideally we shouldn't use external BTF here, since we might try to use
-	// it for parsing kmod split BTF later on. That seems unlikely to work.
-	spec, err := maybeLoadKernelBTF(nil)
+	spec, err := btf.LoadKernelSpec()
 	if err != nil {
 		return nil, 0, fmt.Errorf("load kernel spec: %w", err)
 	}
 
 	err = spec.TypeByName(typeName, &target)
 	if errors.Is(err, btf.ErrNotFound) {
-		module, id, err := findTargetInModule(spec, typeName, target)
+		module, id, err := findTargetInModule(typeName, target)
 		if errors.Is(err, btf.ErrNotFound) {
 			return nil, 0, &internal.UnsupportedFeatureError{Name: featureName}
 		}
@@ -878,7 +875,7 @@ func findTargetInKernel(name string, progType ProgramType, attachType AttachType
 // vmlinux must contain the kernel's types and is used to parse kmod BTF.
 //
 // Returns btf.ErrNotFound if the target can't be found in any module.
-func findTargetInModule(vmlinux *btf.Spec, typeName string, target btf.Type) (*btf.Handle, btf.TypeID, error) {
+func findTargetInModule(typeName string, target btf.Type) (*btf.Handle, btf.TypeID, error) {
 	it := new(btf.HandleIterator)
 	defer it.Handle.Close()
 
@@ -892,7 +889,7 @@ func findTargetInModule(vmlinux *btf.Spec, typeName string, target btf.Type) (*b
 			continue
 		}
 
-		spec, err := it.Handle.Spec(vmlinux)
+		spec, err := it.Handle.Spec()
 		if err != nil {
 			return nil, 0, fmt.Errorf("parse types for module %s: %w", info.Name, err)
 		}
@@ -942,7 +939,7 @@ func findTargetInProgram(prog *Program, name string, progType ProgramType, attac
 	}
 	defer btfHandle.Close()
 
-	spec, err := btfHandle.Spec(nil)
+	spec, err := btfHandle.Spec()
 	if err != nil {
 		return 0, err
 	}


Test patch:
diff --git a/btf/btf_test.go b/btf/btf_test.go
index ed0f34d..d681a16 100644
--- a/btf/btf_test.go
+++ b/btf/btf_test.go
@@ -12,15 +12,9 @@ import (
 
 	"github.com/cilium/ebpf/internal"
 	"github.com/cilium/ebpf/internal/testutils"
+	qt "github.com/frankban/quicktest"
 )
 
-// vmlinux caches the result of parsing the running kernel's BTF.
-var vmlinux struct {
-	sync.Once
-	spec *Spec
-	err  error
-}
-
 func vmlinuxSpec(tb testing.TB) *Spec {
 	tb.Helper()
 
@@ -28,14 +22,14 @@ func vmlinuxSpec(tb testing.TB) *Spec {
 	// through sysfs"), which shipped in Linux 5.4.
 	testutils.SkipOnOldKernel(tb, "5.4", "vmlinux BTF in sysfs")
 
-	vmlinux.Do(func() {
-		vmlinux.spec, vmlinux.err = LoadKernelSpec()
-	})
-	if vmlinux.err != nil {
-		tb.Fatal(vmlinux.err)
+	spec, fallback, err := kernelSpec()
+	if err != nil {
+		tb.Fatal(err)
 	}
-
-	return vmlinux.spec.Copy()
+	if fallback {
+		tb.Fatal("/sys/kernel/btf/vmlinux is not available")
+	}
+	return spec
 }
 
 // vmlinuxTestdata caches the result of reading and parsing a BTF blob from
@@ -480,6 +474,11 @@ func TestLoadSplitSpecFromReader(t *testing.T) {
 	if err != nil {
 		t.Fatal(err)
 	}
+
+	typeByID, err := splitSpec.TypeByID(typeID)
+	qt.Assert(t, err, qt.IsNil)
+	qt.Assert(t, typeByID, qt.Equals, typ)
+
 	fnType := typ.(*Func)
 	fnProto := fnType.Type.(*FuncProto)
 
diff --git a/btf/handle_test.go b/btf/handle_test.go
index d322ce5..a6c22e0 100644
--- a/btf/handle_test.go
+++ b/btf/handle_test.go
@@ -70,20 +70,10 @@ func TestParseModuleSplitSpec(t *testing.T) {
 	}
 	defer vmlinux.Close()
 
-	vmlinuxSpec, err := vmlinux.Spec(nil)
-	if err != nil {
-		t.Fatal("Parse vmlinux BTF:", err)
-	}
-
-	_, err = module.Spec(vmlinuxSpec)
+	_, err = module.Spec()
 	if err != nil {
 		t.Fatal("Parse module BTF:", err)
 	}
-
-	_, err = module.Spec(nil)
-	if err == nil {
-		t.Fatal("Parsing module BTF without vmlinux base didn't fail")
-	}
 }
 
 func ExampleHandleIterator() {