elf: correctly relocate calls to static functions

Clang emits calls to static functions in a different section by
encoding the offset of the function in the imm field. The first
instruction in another section is encoded with an immediate value
of -1. The relocation for the function call the references a
section in the ELF. I suspect this has to do with saving work for
the linker.

For the library this is a problem: we track function calls by name
and not by offset. Reverse the offset calculation to find the real
function name, and force the immediate to -1. From that point
onwards static calls follow the same path as non-static ones.

Fixes cilium#72

Author: lmb

asm/instruction.go

@@ -394,7 +394,7 @@ func (insns Instructions) Marshal(w io.Writer, bo binary.ByteOrder) error {
 	num := 0
 	for i, ins := range insns {
 		switch {
-		case ins.OpCode.JumpOp() == Call && ins.Constant == -1:
+		case ins.OpCode.JumpOp() == Call && ins.Src == PseudoCall && ins.Constant == -1:
 			// Rewrite bpf to bpf call
 			offset, ok := absoluteOffsets[ins.Reference]
 			if !ok {

elf_reader.go

@@ -20,7 +20,7 @@ import (
 type elfCode struct {
 	*elf.File
 	symbols           []elf.Symbol
-	symbolsPerSection map[elf.SectionIndex]map[uint64]string
+	symbolsPerSection map[elf.SectionIndex]map[uint64]elf.Symbol
 	license           string
 	version           uint32
 }
@@ -173,20 +173,20 @@ func (ec *elfCode) loadPrograms(progSections map[elf.SectionIndex]*elf.Section,
 			return nil, xerrors.Errorf("section %v: missing symbols", prog.Name)
 		}
 
-		funcSym := syms[0]
-		if funcSym == "" {
+		funcSym, ok := syms[0]
+		if !ok {
 			return nil, xerrors.Errorf("section %v: no label at start", prog.Name)
 		}
 
 		insns, length, err := ec.loadInstructions(prog, syms, relocations[idx])
 		if err != nil {
-			return nil, xerrors.Errorf("program %s: can't unmarshal instructions: %w", funcSym, err)
+			return nil, xerrors.Errorf("program %s: can't unmarshal instructions: %w", funcSym.Name, err)
 		}
 
 		progType, attachType := getProgType(prog.Name)
 
 		spec := &ProgramSpec{
-			Name:          funcSym,
+			Name:          funcSym.Name,
 			Type:          progType,
 			AttachType:    attachType,
 			License:       ec.license,
@@ -223,7 +223,7 @@ func (ec *elfCode) loadPrograms(progSections map[elf.SectionIndex]*elf.Section,
 	return res, nil
 }
 
-func (ec *elfCode) loadInstructions(section *elf.Section, symbols map[uint64]string, relocations map[uint64]elf.Symbol) (asm.Instructions, uint64, error) {
+func (ec *elfCode) loadInstructions(section *elf.Section, symbols, relocations map[uint64]elf.Symbol) (asm.Instructions, uint64, error) {
 	var (
 		r      = section.Open()
 		insns  asm.Instructions
@@ -239,7 +239,7 @@ func (ec *elfCode) loadInstructions(section *elf.Section, symbols map[uint64]str
 			return nil, 0, xerrors.Errorf("offset %d: %w", offset, err)
 		}
 
-		ins.Symbol = symbols[offset]
+		ins.Symbol = symbols[offset].Name
 
 		if rel, ok := relocations[offset]; ok {
 			if err = ec.relocateInstruction(&ins, rel); err != nil {
@@ -256,9 +256,20 @@ func (ec *elfCode) relocateInstruction(ins *asm.Instruction, rel elf.Symbol) err
 	var (
 		typ  = elf.ST_TYPE(rel.Info)
 		bind = elf.ST_BIND(rel.Info)
-		ref  = rel.Name
+		name = rel.Name
 	)
 
+	if typ == elf.STT_SECTION {
+		// Symbols with section type do not have a name set. Get it
+		// from the section itself.
+		idx := int(rel.Section)
+		if idx > len(ec.Sections) {
+			return xerrors.New("out-of-bounds section index")
+		}
+
+		name = ec.Sections[idx].Name
+	}
+
 outer:
 	switch {
 	case ins.OpCode == asm.LoadImmOp(asm.DWord):
@@ -272,18 +283,10 @@ outer:
 			// This is a direct load since the referenced symbol is a
 			// section. Weirdly, the offset of the real symbol in the
 			// section is encoded in the instruction stream.
-			idx := int(rel.Section)
-			if idx > len(ec.Sections) {
-				return xerrors.New("out-of-bounds section index")
-			}
-
 			if bind != elf.STB_LOCAL {
-				return xerrors.Errorf("direct load: %s: unsupported relocation %s", ref, bind)
+				return xerrors.Errorf("direct load: %s: unsupported relocation %s", name, bind)
 			}
 
-			// Make the instruction reference the map it's loading from.
-			ref = ec.Sections[idx].Name
-
 			// For some reason, clang encodes the offset of the symbol its
 			// section in the first basic BPF instruction, while the kernel
 			// expects it in the second one.
@@ -293,7 +296,7 @@ outer:
 		case elf.STT_NOTYPE:
 			if bind == elf.STB_GLOBAL && rel.Section == elf.SHN_UNDEF {
 				// This is a relocation generated by inline assembly.
-				// We can't do more than assing ins.Reference.
+				// We can't do more than assigning ins.Reference.
 				break outer
 			}
 
@@ -303,10 +306,13 @@ outer:
 
 		case elf.STT_OBJECT:
 			if bind != elf.STB_GLOBAL {
-				return xerrors.Errorf("load: %s: unsupported relocation %s", ref, bind)
+				return xerrors.Errorf("load: %s: unsupported binding: %s", name, bind)
 			}
 
 			ins.Src = asm.PseudoMapFD
+
+		default:
+			return xerrors.Errorf("load: %s: unsupported relocation: %s", name, typ)
 		}
 
 		// Mark the instruction as needing an update when creating the
@@ -316,19 +322,47 @@ outer:
 		}
 
 	case ins.OpCode.JumpOp() == asm.Call:
-		if bind != elf.STB_GLOBAL {
-			return xerrors.Errorf("call: %s: unsupported relocation %s", ref, bind)
+		if ins.Src != asm.PseudoCall {
+			return xerrors.Errorf("call: %s: incorrect source register", name)
 		}
 
-		if typ != elf.STT_NOTYPE && typ != elf.STT_FUNC {
-			return xerrors.Errorf("call: %s: invalid symbol type %s", ref, typ)
+		switch typ {
+		case elf.STT_NOTYPE, elf.STT_FUNC:
+			if bind != elf.STB_GLOBAL {
+				return xerrors.Errorf("call: %s: unsupported binding: %s", name, bind)
+			}
+
+		case elf.STT_SECTION:
+			if bind != elf.STB_LOCAL {
+				return xerrors.Errorf("call: %s: unsupported binding: %s", name, bind)
+			}
+
+			// The function we want to call is in the indicated section,
+			// at the offset encoded in the instruction itself. Reverse
+			// the calculation to find the real function we're looking for.
+			// A value of -1 references the first instruction in the section.
+			offset := int64(int32(ins.Constant)+1) * asm.InstructionSize
+			if offset < 0 {
+				return xerrors.Errorf("call: %s: invalid offset %d", offset)
+			}
+
+			sym, ok := ec.symbolsPerSection[rel.Section][uint64(offset)]
+			if !ok {
+				return xerrors.Errorf("call: %s: no symbol at offset %d", name, offset)
+			}
+
+			ins.Constant = -1
+			name = sym.Name
+
+		default:
+			return xerrors.Errorf("call: %s: invalid symbol type %s", name, typ)
 		}
 
 	default:
 		return xerrors.Errorf("relocation for unsupported instruction: %s", ins.OpCode)
 	}
 
-	ins.Reference = ref
+	ins.Reference = name
 	return nil
 }
 
@@ -348,19 +382,19 @@ func (ec *elfCode) loadMaps(maps map[string]*MapSpec, mapSections map[elf.Sectio
 			size = sec.Size / uint64(len(syms))
 		)
 		for i, offset := 0, uint64(0); i < len(syms); i, offset = i+1, offset+size {
-			mapSym := syms[offset]
-			if mapSym == "" {
+			mapSym, ok := syms[offset]
+			if !ok {
 				return xerrors.Errorf("section %s: missing symbol for map at offset %d", sec.Name, offset)
 			}
 
-			if maps[mapSym] != nil {
+			if maps[mapSym.Name] != nil {
 				return xerrors.Errorf("section %v: map %v already exists", sec.Name, mapSym)
 			}
 
 			lr := io.LimitReader(r, int64(size))
 
 			spec := MapSpec{
-				Name: SanitizeName(mapSym, -1),
+				Name: SanitizeName(mapSym.Name, -1),
 			}
 			switch {
 			case binary.Read(lr, ec.ByteOrder, &spec.Type) != nil:
@@ -379,7 +413,7 @@ func (ec *elfCode) loadMaps(maps map[string]*MapSpec, mapSections map[elf.Sectio
 				return xerrors.Errorf("map %v: unknown and non-zero fields in definition", mapSym)
 			}
 
-			maps[mapSym] = &spec
+			maps[mapSym.Name] = &spec
 		}
 	}
 
@@ -398,21 +432,22 @@ func (ec *elfCode) loadBTFMaps(maps map[string]*MapSpec, mapSections map[elf.Sec
 		}
 
 		for _, sym := range syms {
-			if maps[sym] != nil {
+			name := sym.Name
+			if maps[name] != nil {
 				return xerrors.Errorf("section %v: map %v already exists", sec.Name, sym)
 			}
 
-			btfMap, btfMapMembers, err := spec.Map(sym)
+			btfMap, btfMapMembers, err := spec.Map(name)
 			if err != nil {
-				return xerrors.Errorf("map %v: can't get BTF: %w", sym, err)
+				return xerrors.Errorf("map %v: can't get BTF: %w", name, err)
 			}
 
 			spec, err := mapSpecFromBTF(btfMap, btfMapMembers)
 			if err != nil {
-				return xerrors.Errorf("map %v: %w", sym, err)
+				return xerrors.Errorf("map %v: %w", name, err)
 			}
 
-			maps[sym] = spec
+			maps[name] = spec
 		}
 	}
 
@@ -640,9 +675,9 @@ func (ec *elfCode) loadRelocations(sections map[elf.SectionIndex]*elf.Section) (
 	return result, nil
 }
 
-func symbolsPerSection(symbols []elf.Symbol) map[elf.SectionIndex]map[uint64]string {
-	result := make(map[elf.SectionIndex]map[uint64]string)
-	for i, sym := range symbols {
+func symbolsPerSection(symbols []elf.Symbol) map[elf.SectionIndex]map[uint64]elf.Symbol {
+	result := make(map[elf.SectionIndex]map[uint64]elf.Symbol)
+	for _, sym := range symbols {
 		switch elf.ST_TYPE(sym.Info) {
 		case elf.STT_NOTYPE:
 			// Older versions of LLVM doesn't tag
@@ -666,9 +701,9 @@ func symbolsPerSection(symbols []elf.Symbol) map[elf.SectionIndex]map[uint64]str
 
 		idx := sym.Section
 		if _, ok := result[idx]; !ok {
-			result[idx] = make(map[uint64]string)
+			result[idx] = make(map[uint64]elf.Symbol)
 		}
-		result[idx][sym.Value] = symbols[i].Name
+		result[idx][sym.Value] = sym
 	}
 	return result
 }

testdata/loader-clang-6.0.elf

@@ -53,6 +53,13 @@ int __attribute__((noinline)) helper_func(uint32_t arg) {
 	return helper_func2(arg);
 }
 
+// Static functions in other sections get different relocations
+// than global functions.
+static int __attribute__((noinline)) helper_func_static() {
+	uint32_t key = 0;
+	return !!map_lookup_elem(&hash_map, (void*)&key);
+}
+
 #if __clang_major__ >= 9
 static volatile unsigned int key1 = 0; // .bss
 static volatile unsigned int key2 = 1; // .data
@@ -70,6 +77,9 @@ __section("xdp") int xdp_prog() {
 	map_lookup_elem(&hash_map, (void*)&key1);
 	map_lookup_elem(&hash_map2, (void*)&key2);
 	map_lookup_elem(&hash_map2, (void*)&key3);
+	if (helper_func_static()) {
+		return 2;
+	}
 	return helper_func(arg);
 }