From c748da2270cf8fec67503583f07a22c796746713 Mon Sep 17 00:00:00 2001 From: Dylan Reimerink Date: Tue, 17 Oct 2023 18:29:23 +0200 Subject: [PATCH] btf,info: Fix bad instruction offset when parsing infos from kernel When BTF ext info is encoded in ELF the instruction offsets are in bytes, but when you pass them to the kernel they must be instruction indices. Therefore we divide the offset by the instruction size in the parsing logic for ELF. This was missed during the initial implementation of reading back BTF ext info from the kernel. This would cause an error when loading back ext info which was not a multiple of the instruction size or a bad instruction offset if it was. Fix this by making LoadLineInfos and LoadFuncInfos work on the kernel format which contains instruction offsets while the ELF parser uses bytes instead. Signed-off-by: Dylan Reimerink Co-developed-by: Lorenz Bauer --- btf/ext_info.go | 42 ++++++++++++++++++++++++------------------ info_test.go | 46 ++++++++++++++++++++++++++++++++++------------ 2 files changed, 58 insertions(+), 30 deletions(-) diff --git a/btf/ext_info.go b/btf/ext_info.go index 36803504b..3eae08b28 100644 --- a/btf/ext_info.go +++ b/btf/ext_info.go @@ -391,13 +391,14 @@ func newFuncInfos(bfis []bpfFuncInfo, spec *Spec) (FuncInfos, error) { return fis, nil } -// LoadFuncInfos parses btf func info in wire format. +// LoadFuncInfos parses BTF func info in kernel wire format. func LoadFuncInfos(reader io.Reader, bo binary.ByteOrder, recordNum uint32, spec *Spec) (FuncInfos, error) { fis, err := parseFuncInfoRecords( reader, bo, FuncInfoSize, recordNum, + false, ) if err != nil { return FuncInfos{}, fmt.Errorf("parsing BTF func info: %w", err) @@ -441,7 +442,7 @@ func parseFuncInfos(r io.Reader, bo binary.ByteOrder, strings *stringTable) (map return nil, err } - records, err := parseFuncInfoRecords(r, bo, recordSize, infoHeader.NumInfo) + records, err := parseFuncInfoRecords(r, bo, recordSize, infoHeader.NumInfo, true) if err != nil { return nil, fmt.Errorf("section %v: %w", secName, err) } @@ -453,7 +454,7 @@ func parseFuncInfos(r io.Reader, bo binary.ByteOrder, strings *stringTable) (map // parseFuncInfoRecords parses a stream of func_infos into a funcInfos. // These records appear after a btf_ext_info_sec header in the func_info // sub-section of .BTF.ext. -func parseFuncInfoRecords(r io.Reader, bo binary.ByteOrder, recordSize uint32, recordNum uint32) ([]bpfFuncInfo, error) { +func parseFuncInfoRecords(r io.Reader, bo binary.ByteOrder, recordSize uint32, recordNum uint32, offsetInBytes bool) ([]bpfFuncInfo, error) { var out []bpfFuncInfo var fi bpfFuncInfo @@ -467,13 +468,15 @@ func parseFuncInfoRecords(r io.Reader, bo binary.ByteOrder, recordSize uint32, r return nil, fmt.Errorf("can't read function info: %v", err) } - if fi.InsnOff%asm.InstructionSize != 0 { - return nil, fmt.Errorf("offset %v is not aligned with instruction size", fi.InsnOff) - } + if offsetInBytes { + if fi.InsnOff%asm.InstructionSize != 0 { + return nil, fmt.Errorf("offset %v is not aligned with instruction size", fi.InsnOff) + } - // ELF tracks offset in bytes, the kernel expects raw BPF instructions. - // Convert as early as possible. - fi.InsnOff /= asm.InstructionSize + // ELF tracks offset in bytes, the kernel expects raw BPF instructions. + // Convert as early as possible. + fi.InsnOff /= asm.InstructionSize + } out = append(out, fi) } @@ -537,13 +540,14 @@ type bpfLineInfo struct { LineCol uint32 } -// LoadLineInfos parses btf line info in wire format. +// LoadLineInfos parses BTF line info in kernel wire format. func LoadLineInfos(reader io.Reader, bo binary.ByteOrder, recordNum uint32, spec *Spec) (LineInfos, error) { lis, err := parseLineInfoRecords( reader, bo, LineInfoSize, recordNum, + false, ) if err != nil { return LineInfos{}, fmt.Errorf("parsing BTF line info: %w", err) @@ -649,7 +653,7 @@ func parseLineInfos(r io.Reader, bo binary.ByteOrder, strings *stringTable) (map return nil, err } - records, err := parseLineInfoRecords(r, bo, recordSize, infoHeader.NumInfo) + records, err := parseLineInfoRecords(r, bo, recordSize, infoHeader.NumInfo, true) if err != nil { return nil, fmt.Errorf("section %v: %w", secName, err) } @@ -661,7 +665,7 @@ func parseLineInfos(r io.Reader, bo binary.ByteOrder, strings *stringTable) (map // parseLineInfoRecords parses a stream of line_infos into a lineInfos. // These records appear after a btf_ext_info_sec header in the line_info // sub-section of .BTF.ext. -func parseLineInfoRecords(r io.Reader, bo binary.ByteOrder, recordSize uint32, recordNum uint32) ([]bpfLineInfo, error) { +func parseLineInfoRecords(r io.Reader, bo binary.ByteOrder, recordSize uint32, recordNum uint32, offsetInBytes bool) ([]bpfLineInfo, error) { var out []bpfLineInfo var li bpfLineInfo @@ -675,13 +679,15 @@ func parseLineInfoRecords(r io.Reader, bo binary.ByteOrder, recordSize uint32, r return nil, fmt.Errorf("can't read line info: %v", err) } - if li.InsnOff%asm.InstructionSize != 0 { - return nil, fmt.Errorf("offset %v is not aligned with instruction size", li.InsnOff) - } + if offsetInBytes { + if li.InsnOff%asm.InstructionSize != 0 { + return nil, fmt.Errorf("offset %v is not aligned with instruction size", li.InsnOff) + } - // ELF tracks offset in bytes, the kernel expects raw BPF instructions. - // Convert as early as possible. - li.InsnOff /= asm.InstructionSize + // ELF tracks offset in bytes, the kernel expects raw BPF instructions. + // Convert as early as possible. + li.InsnOff /= asm.InstructionSize + } out = append(out, li) } diff --git a/info_test.go b/info_test.go index d0aca4e24..9324bbac6 100644 --- a/info_test.go +++ b/info_test.go @@ -370,18 +370,23 @@ func TestHaveProgramInfoMapIDs(t *testing.T) { func TestProgInfoExtBTF(t *testing.T) { testutils.SkipOnOldKernel(t, "5.0", "Program BTF (func/line_info)") - spec, err := LoadCollectionSpec("testdata/raw_tracepoint-el.elf") + spec, err := LoadCollectionSpec(fmt.Sprintf("testdata/loader-%s.elf", internal.ClangEndian)) if err != nil { t.Fatal(err) } - coll, err := NewCollection(spec) + var obj struct { + Main *Program `ebpf:"xdp_prog"` + } + + err = spec.LoadAndAssign(&obj, nil) + testutils.SkipIfNotSupported(t, err) if err != nil { t.Fatal(err) } - defer coll.Close() + defer obj.Main.Close() - info, err := coll.Programs["sched_process_exec"].Info() + info, err := obj.Main.Info() if err != nil { t.Fatal(err) } @@ -391,17 +396,34 @@ func TestProgInfoExtBTF(t *testing.T) { t.Fatal(err) } - const expectedSource = "\treturn 0;" - if inst[0].Source().String() != expectedSource { - t.Fatalf("Source of first instruction incorrect. Got '%s', expected: '%s'", inst[0].Source().String(), expectedSource) + expectedLineInfoCount := 26 + expectedFuncInfo := map[string]bool{ + "xdp_prog": false, + "static_fn": false, + "global_fn2": false, + "global_fn3": false, + } + + lineInfoCount := 0 + + for _, ins := range inst { + if ins.Source() != nil { + lineInfoCount++ + } + + fn := btf.FuncMetadata(&ins) + if fn != nil { + expectedFuncInfo[fn.Name] = true + } } - fn := btf.FuncMetadata(&inst[0]) - if fn == nil { - t.Fatal("Func metadata missing") + if lineInfoCount != expectedLineInfoCount { + t.Errorf("expected %d line info entries, got %d", expectedLineInfoCount, lineInfoCount) } - if fn.Name != "sched_process_exec" { - t.Fatalf("Func metadata incorrect. Got '%s', expected: 'sched_process_exec'", fn.Name) + for fn, found := range expectedFuncInfo { + if !found { + t.Errorf("func %q not found", fn) + } } }