fixup! feat: add support for untyped extern __ksym variables

Address PR comments

Signed-off-by: Patrick Pichler <patrick@cast.ai>

btf/btf.go

@@ -443,7 +443,8 @@ func fixupDatasec(types []Type, sectionSizes map[string]uint32, offsets map[symb
 		// Some Datasecs are virtual and don't have corresponding ELF sections.
 		switch name {
 		case ".ksyms":
-			// .ksyms describes forward declarations of kfunc signatures.
+			// .ksyms describes forward declarations of kfunc signatures, as wel as
+			// references to kernel symbols.
 			// Nothing to fix up, all sizes and offsets are 0.
 			for _, vsi := range ds.Vars {
 				switch t := vsi.Type.(type) {

collection.go

@@ -315,6 +315,7 @@ func (cs *CollectionSpec) LoadAndAssign(to interface{}, opts *CollectionOptions)
 	return nil
 }
 
+
 // Collection is a collection of Programs and Maps associated
 // with their symbols
 type Collection struct {

elf_reader.go

@@ -15,6 +15,7 @@ import (
 	"github.com/cilium/ebpf/asm"
 	"github.com/cilium/ebpf/btf"
 	"github.com/cilium/ebpf/internal"
+	"github.com/cilium/ebpf/internal/kallsyms"
 	"github.com/cilium/ebpf/internal/sys"
 )
 
@@ -35,7 +36,13 @@ type kfuncMeta struct {
 type ksymMetaKey struct{}
 
 type ksymMeta struct {
-	name string
+	VarSpec *KsymVarSpec
+	Binding elf.SymBind
+}
+
+type KsymVarSpec struct {
+	Var  *btf.Var
+	Addr uint64
 }
 
 // elfCode is a convenience to reduce the amount of arguments that have to
@@ -49,7 +56,7 @@ type elfCode struct {
 	extInfo  *btf.ExtInfos
 	maps     map[string]*MapSpec
 	vars     map[string]*VariableSpec
-	ksymVars map[string]*btf.Var
+	ksymVars map[string]*KsymVarSpec
 	kfuncs   map[string]*btf.Func
 	kconfig  *MapSpec
 }
@@ -143,7 +150,7 @@ func LoadCollectionSpecFromReader(rd io.ReaderAt) (*CollectionSpec, error) {
 		maps:        make(map[string]*MapSpec),
 		vars:        make(map[string]*VariableSpec),
 		kfuncs:      make(map[string]*btf.Func),
-		ksymVars:    make(map[string]*btf.Var),
+		ksymVars:    make(map[string]*KsymVarSpec),
 	}
 
 	symbols, err := f.Symbols()
@@ -634,8 +641,6 @@ func (ec *elfCode) relocateInstruction(ins *asm.Instruction, rel elf.Symbol) err
 			return fmt.Errorf("asm relocation: %s: unsupported type %s", name, typ)
 		}
 
-		// TODO(patrick.pichler): handle me
-
 		kf := ec.kfuncs[name]
 		ksymsVar := ec.ksymVars[name]
 
@@ -660,12 +665,12 @@ func (ec *elfCode) relocateInstruction(ins *asm.Instruction, rel elf.Symbol) err
 			ins.Constant = 0
 
 		case ksymsVar != nil && ins.OpCode.IsDWordLoad():
-			if bind != elf.STB_GLOBAL {
+			if bind != elf.STB_GLOBAL && bind != elf.STB_WEAK {
 				return fmt.Errorf("asm relocation: %s: %w: %s", name, errUnsupportedBinding, bind)
 			}
-			println("==> set")
 			ins.Metadata.Set(ksymMetaKey{}, &ksymMeta{
-				rel.Name,
+				VarSpec: ksymsVar,
+				Binding: bind,
 			})
 
 		// If no kconfig map is found, this must be a symbol reference from inline
@@ -1305,14 +1310,30 @@ func (ec *elfCode) loadKsymsSection() error {
 		case *btf.Func:
 			ec.kfuncs[t.TypeName()] = t
 		case *btf.Var:
-			ec.ksymVars[t.TypeName()] = t
+			ec.ksymVars[t.TypeName()] = &KsymVarSpec{
+				Var: t,
+			}
 		// This case should never happen, as we already checked that only vars and funcs
 		// are present in the vars.
 		default:
 			return fmt.Errorf("unexpected variable types: %T", v)
 		}
 	}
 
+	symbolsToLoad := make([]string, 0, len(ec.ksymVars))
+	for k := range ec.ksymVars {
+		symbolsToLoad = append(symbolsToLoad, k)
+	}
+
+	ksymAddrs, err := kallsyms.LoadSymbolAddresses(symbolsToLoad)
+	if err != nil {
+		return fmt.Errorf("error while loading ksym addresses: %w", err)
+	}
+
+	for ksym, addr := range ksymAddrs {
+		ec.ksymVars[ksym].Addr = addr
+	}
+
 	return nil
 }
 

elf_reader_test.go

@@ -774,10 +774,6 @@ func TestKconfigConfig(t *testing.T) {
 }
 
 func TestKsym(t *testing.T) {
-	if !haveTestmod(t) {
-		t.Skip("bpf_testmod not loaded")
-	}
-
 	file := testutils.NativeFile(t, "testdata/ksym-%s.elf")
 	spec, err := LoadCollectionSpec(file)
 	if err != nil {
@@ -803,19 +799,54 @@ func TestKsym(t *testing.T) {
 		t.Fatal(err)
 	}
 
+	ksyms, err := kallsyms.LoadSymbolAddresses([]string{
+		"socket_file_ops",
+		"tty_fops",
+	})
+	if err != nil {
+		t.Fatal(err)
+	}
+
 	var value uint64
 	err = obj.ArrayMap.Lookup(uint32(0), &value)
 	if err != nil {
 		t.Fatal(err)
 	}
 
-	ksyms, err := kallsyms.LoadSymbolAddresses()
+	qt.Assert(t, qt.Equals(value, ksyms["socket_file_ops"]))
+
+	err = obj.ArrayMap.Lookup(uint32(1), &value)
+	if err != nil {
+		t.Fatal(err)
+	}
+	qt.Assert(t, qt.Equals(value, ksyms["tty_fops"]))
+}
+
+func TestKsymWeakMissing(t *testing.T) {
+	file := testutils.NativeFile(t, "testdata/ksym-%s.elf")
+	spec, err := LoadCollectionSpec(file)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	var obj struct {
+		Main *Program `ebpf:"ksym_missing_test"`
+	}
+
+	err = spec.LoadAndAssign(&obj, nil)
+	testutils.SkipIfNotSupported(t, err)
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer obj.Main.Close()
+
+	res, _, err := obj.Main.Test(internal.EmptyBPFContext)
+	testutils.SkipIfNotSupported(t, err)
 	if err != nil {
 		t.Fatal(err)
 	}
 
-	// socket_file_ops must be present and the value we have in kallsyms
-	qt.Assert(t, qt.Not(qt.Equals(value, ksyms["socket_file_ops __ksym"])))
+	qt.Assert(t, qt.Equals(res, 1))
 }
 
 func TestKfunc(t *testing.T) {

internal/kallsyms/kallsyms.go

@@ -6,6 +6,7 @@ import (
 	"fmt"
 	"io"
 	"os"
+	"slices"
 	"sync"
 )
 
@@ -79,34 +80,25 @@ func loadKernelModuleMapping(f io.Reader) (map[string]string, error) {
 	return mods, nil
 }
 
-func LoadSymbolAddresses() (map[string]uint64, error) {
-	kernelSymbols.RLock()
-	ksyms := kernelSymbols.ksyms
-	kernelSymbols.RUnlock()
-
-	if ksyms == nil {
-		kernelSymbols.Lock()
-		defer kernelSymbols.Unlock()
-		ksyms = kernelSymbols.ksyms
-	}
-
+func LoadSymbolAddresses(symbols []string) (map[string]uint64, error) {
 	f, err := os.Open("/proc/kallsyms")
 	if err != nil {
 		return nil, err
 	}
 
-	ksyms, err = loadSymbolAddresses(f)
+	ksyms, err := loadSymbolAddresses(f, symbols)
 	if err != nil {
 		return nil, err
 	}
 
 	return ksyms, nil
 }
 
-func loadSymbolAddresses(f io.Reader) (map[string]uint64, error) {
+func loadSymbolAddresses(f io.Reader, symbols []string) (map[string]uint64, error) {
 	scan := bufio.NewScanner(f)
 
 	result := make(map[string]uint64)
+	slices.Sort(symbols)
 
 	for scan.Scan() {
 		var (
@@ -121,7 +113,9 @@ func loadSymbolAddresses(f io.Reader) (map[string]uint64, error) {
 		if err != nil {
 			return nil, err
 		}
-		result[symbol] = addr
+		if _, found := slices.BinarySearch(symbols, symbol); found {
+			result[symbol] = addr
+		}
 	}
 
 	if scan.Err() != nil {

internal/kallsyms/kallsyms_test.go

@@ -7,18 +7,19 @@ import (
 	"github.com/go-quicktest/qt"
 )
 
+var kallsyms = []byte(`0000000000000000 t hid_generic_probe	[hid_generic]
+00000000000000A0 T tcp_connect
+00000000000000B0 B empty_zero_page
+00000000000000C0 D kimage_vaddr
+00000000000000D0 R __start_pci_fixups_early
+00000000000000E0 V hv_root_partition
+00000000000000F0 W calibrate_delay_is_known
+A0000000000000AA a nft_counter_seq	[nft_counter]
+A0000000000000BA b bootconfig_found
+A0000000000000CA d __func__.10
+A0000000000000DA r __ksymtab_LZ4_decompress_fast`)
+
 func TestKernelModule(t *testing.T) {
-	kallsyms := []byte(`0000000000000000 t hid_generic_probe	[hid_generic]
-0000000000000000 T tcp_connect
-0000000000000000 B empty_zero_page
-0000000000000000 D kimage_vaddr
-0000000000000000 R __start_pci_fixups_early
-0000000000000000 V hv_root_partition
-0000000000000000 W calibrate_delay_is_known
-0000000000000000 a nft_counter_seq	[nft_counter]
-0000000000000000 b bootconfig_found
-0000000000000000 d __func__.10
-0000000000000000 r __ksymtab_LZ4_decompress_fast`)
 	krdr := bytes.NewBuffer(kallsyms)
 	kmods, err := loadKernelModuleMapping(krdr)
 	qt.Assert(t, qt.IsNil(err))
@@ -39,19 +40,12 @@ func TestKernelModule(t *testing.T) {
 }
 
 func TestLoadSymbolAddresses(t *testing.T) {
-	kallsyms := []byte(`0000000000000000 t hid_generic_probe	[hid_generic]
-00000000000000A0 T tcp_connect
-00000000000000B0 B empty_zero_page
-00000000000000C0 D kimage_vaddr
-00000000000000D0 R __start_pci_fixups_early
-00000000000000E0 V hv_root_partition
-00000000000000F0 W calibrate_delay_is_known
-A0000000000000AA a nft_counter_seq	[nft_counter]
-A0000000000000BA b bootconfig_found
-A0000000000000CA d __func__.10
-A0000000000000DA r __ksymtab_LZ4_decompress_fast`)
 	krdr := bytes.NewBuffer(kallsyms)
-	ksyms, err := loadSymbolAddresses(krdr)
+	ksyms, err := loadSymbolAddresses(krdr, []string{
+		"hid_generic_probe",
+		"tcp_connect",
+		"bootconfig_found",
+	})
 	qt.Assert(t, qt.IsNil(err))
 
 	qt.Assert(t, qt.Equals(ksyms["hid_generic_probe"], 0))

linker.go

@@ -14,7 +14,6 @@ import (
 	"github.com/cilium/ebpf/asm"
 	"github.com/cilium/ebpf/btf"
 	"github.com/cilium/ebpf/internal"
-	"github.com/cilium/ebpf/internal/kallsyms"
 )
 
 // handles stores handle objects to avoid gc cleanup
@@ -461,34 +460,26 @@ func resolveKconfigReferences(insns asm.Instructions) (_ *Map, err error) {
 }
 
 func resolveKsymReferences(insns asm.Instructions) error {
-	var ksymMap map[string]uint64
-	var err error
-
 	iter := insns.Iterate()
+
 	var missingKsyms []string
 
 	for iter.Next() {
 		meta, _ := iter.Ins.Metadata.Get(ksymMetaKey{}).(*ksymMeta)
 		if meta == nil {
 			continue
 		}
-		// We load the kallsyms map only when really needed
-		if ksymMap == nil {
-			ksymMap, err = kallsyms.LoadSymbolAddresses()
-			if err != nil {
-				return fmt.Errorf("error whild trying to load kallsyms: %v", err)
-			}
-		}
 
-		addr, found := ksymMap[meta.name]
-		if !found {
-			if !slices.Contains(missingKsyms, meta.name) {
-				missingKsyms = append(missingKsyms, meta.name)
+		switch {
+		case meta.VarSpec.Addr != 0:
+			iter.Ins.Constant = int64(meta.VarSpec.Addr)
+		case meta.VarSpec.Addr == 0 && meta.Binding == elf.STB_WEAK:
+			iter.Ins.Constant = 0
+		default:
+			if slices.Contains(missingKsyms, meta.VarSpec.Var.TypeName()) {
+				missingKsyms = append(missingKsyms, meta.VarSpec.Var.TypeName())
 			}
 		}
-
-		iter.Ins.OpCode = asm.LoadImmOp(asm.DWord)
-		iter.Ins.Constant = int64(addr)
 	}
 
 	if len(missingKsyms) > 0 {

prog.go

@@ -337,10 +337,10 @@ func newProgramWithOptions(spec *ProgramSpec, opts ProgramOptions) (*Program, er
 	}
 	defer kconfig.Close()
 
-	err = resolveKsymReferences(insns)
-	if err != nil {
+  err = resolveKsymReferences(insns)
+  if err != nil {
 		return nil, fmt.Errorf("resolve .ksyms: %w", err)
-	}
+  }
 
 	if err := fixupAndValidate(insns); err != nil {
 		return nil, err