Skip to content

Commit

Permalink
btf: fix CO-RE relocations for local type id
Browse files Browse the repository at this point in the history
At some point the kernel gained the ability to dynamically allocated
typed memory via a bpf_obj_new helper (see [1]). This helper receives
the ID of the type to allocate as an argument.

Trying to use this helper with the library will currently fail, since
we took a short-cut when implementing on the fly BTF generation. At
that point in time there was no use of the local_type_id relocation in
the kernel, so we just stuck with the value present in ext_infos.
This is of course not correct when building a BTF blob from scratch:
type IDs are reallocated, and only used types are passed to the kernel
in the first place.

Fix this by allocating an ID for any type that is the target of a
local_type_id relocation.

1: https://lwn.net/Articles/915404/

Signed-off-by: Lorenz Bauer <[email protected]>
  • Loading branch information
lmb committed Oct 31, 2023
1 parent 6b0a090 commit 901f16d
Show file tree
Hide file tree
Showing 12 changed files with 105 additions and 58 deletions.
17 changes: 9 additions & 8 deletions btf/core.go
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,7 @@ func (k coreKind) String() string {
//
// Fixups are returned in the order of relos, e.g. fixup[i] is the solution
// for relos[i].
func CORERelocate(relos []*CORERelocation, target *Spec, bo binary.ByteOrder) ([]COREFixup, error) {
func CORERelocate(relos []*CORERelocation, target *Spec, bo binary.ByteOrder, typeID func(Type) (TypeID, error)) ([]COREFixup, error) {
if target == nil {
var err error
target, _, err = kernelSpec()
Expand Down Expand Up @@ -194,14 +194,15 @@ func CORERelocate(relos []*CORERelocation, target *Spec, bo binary.ByteOrder) ([
return nil, fmt.Errorf("%s: unexpected accessor %v", relo.kind, relo.accessor)
}

id, err := typeID(relo.typ)
if err != nil {
return nil, fmt.Errorf("%s: get type id: %w", relo.kind, err)
}

result[i] = COREFixup{
kind: relo.kind,
local: uint64(relo.id),
// NB: Using relo.id as the target here is incorrect, since
// it doesn't match the BTF we generate on the fly. This isn't
// too bad for now since there are no uses of the local type ID
// in the kernel, yet.
target: uint64(relo.id),
kind: relo.kind,
local: uint64(relo.id),
target: uint64(id),
}
continue
}
Expand Down
1 change: 1 addition & 0 deletions btf/core_reloc_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ func TestCORERelocationLoad(t *testing.T) {
prog, err := ebpf.NewProgramWithOptions(progSpec, ebpf.ProgramOptions{
KernelTypes: spec.Types,
})
testutils.SkipIfNotSupported(t, err)

if strings.HasPrefix(progSpec.Name, "err_") {
if err == nil {
Expand Down
4 changes: 2 additions & 2 deletions btf/core_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -590,7 +590,7 @@ func TestCORERelocation(t *testing.T) {
relos = append(relos, reloInfo.relo)
}

fixups, err := CORERelocate(relos, spec, spec.byteOrder)
fixups, err := CORERelocate(relos, spec, spec.byteOrder, spec.TypeID)
if want := errs[name]; want != nil {
if !errors.Is(err, want) {
t.Fatal("Expected", want, "got", err)
Expand Down Expand Up @@ -737,7 +737,7 @@ func BenchmarkCORESkBuff(b *testing.B) {
b.ReportAllocs()

for i := 0; i < b.N; i++ {
_, err = CORERelocate([]*CORERelocation{relo}, spec, spec.byteOrder)
_, err = CORERelocate([]*CORERelocation{relo}, spec, spec.byteOrder, spec.TypeID)
if err != nil {
b.Fatal(err)
}
Expand Down
24 changes: 7 additions & 17 deletions btf/ext_info.go
Original file line number Diff line number Diff line change
Expand Up @@ -142,15 +142,7 @@ func AssignMetadataToInstructions(

// MarshalExtInfos encodes function and line info embedded in insns into kernel
// wire format.
//
// Returns ErrNotSupported if the kernel doesn't support BTF-associated programs.
func MarshalExtInfos(insns asm.Instructions) (_ *Handle, funcInfos, lineInfos []byte, _ error) {
// Bail out early if the kernel doesn't support Func(Proto). If this is the
// case, func_info will also be unsupported.
if err := haveProgBTF(); err != nil {
return nil, nil, nil, err
}

func MarshalExtInfos(insns asm.Instructions, b *Builder) (funcInfos, lineInfos []byte, _ error) {
iter := insns.Iterate()
for iter.Next() {
_, ok := iter.Ins.Source().(*Line)
Expand All @@ -160,19 +152,18 @@ func MarshalExtInfos(insns asm.Instructions) (_ *Handle, funcInfos, lineInfos []
}
}

return nil, nil, nil, nil
return nil, nil, nil

marshal:
var b Builder
var fiBuf, liBuf bytes.Buffer
for {
if fn := FuncMetadata(iter.Ins); fn != nil {
fi := &funcInfo{
fn: fn,
offset: iter.Offset,
}
if err := fi.marshal(&fiBuf, &b); err != nil {
return nil, nil, nil, fmt.Errorf("write func info: %w", err)
if err := fi.marshal(&fiBuf, b); err != nil {
return nil, nil, fmt.Errorf("write func info: %w", err)
}
}

Expand All @@ -181,8 +172,8 @@ marshal:
line: line,
offset: iter.Offset,
}
if err := li.marshal(&liBuf, &b); err != nil {
return nil, nil, nil, fmt.Errorf("write line info: %w", err)
if err := li.marshal(&liBuf, b); err != nil {
return nil, nil, fmt.Errorf("write line info: %w", err)
}
}

Expand All @@ -191,8 +182,7 @@ marshal:
}
}

handle, err := NewHandle(&b)
return handle, fiBuf.Bytes(), liBuf.Bytes(), err
return fiBuf.Bytes(), liBuf.Bytes(), nil
}

// btfExtHeader is found at the start of the .BTF.ext section.
Expand Down
5 changes: 5 additions & 0 deletions btf/marshal.go
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,11 @@ func NewBuilder(types []Type) (*Builder, error) {
return b, nil
}

// Empty returns true if [Add] has not been invoked on the builder.
func (b *Builder) Empty() bool {
return len(b.types) == 0
}

// Add a Type and allocate a stable ID for it.
//
// Adding the identical Type multiple times is valid and will return the same ID.
Expand Down
Binary file modified btf/testdata/relocs-eb.elf
Binary file not shown.
Binary file modified btf/testdata/relocs-el.elf
Binary file not shown.
39 changes: 18 additions & 21 deletions btf/testdata/relocs.c
Original file line number Diff line number Diff line change
Expand Up @@ -29,23 +29,16 @@ union u {

typedef union u u_t;

#define local_id_zero(expr) \
({ \
if (bpf_core_type_id_local(expr) != 0) { \
return __LINE__; \
} \
})

#define local_id_not_zero(expr) \
({ \
if (bpf_core_type_id_local(expr) == 0) { \
return __LINE__; \
} \
})

#define target_and_local_id_match(expr) \
#define target_and_local_id_dont_match(expr) \
({ \
if (bpf_core_type_id_kernel(expr) != bpf_core_type_id_local(expr)) { \
if (bpf_core_type_id_kernel(expr) == bpf_core_type_id_local(expr)) { \
return __LINE__; \
} \
})
Expand All @@ -69,19 +62,23 @@ __section("socket_filter/type_ids") int type_ids() {
local_id_not_zero(const u_t);
local_id_not_zero(volatile u_t);

// In this context, target is the BTF generated by clang. local is
// generated on the fly by the library. There is a low chance that
// the order on both is the same, so we assert this to make sure that
// CO-RE uses the IDs from the dynamic BTF.
// Qualifiers on types crash clang.
target_and_local_id_match(struct s);
target_and_local_id_match(s_t);
// target_and_local_id_match(const s_t);
// target_and_local_id_match(volatile s_t);
target_and_local_id_match(enum e);
target_and_local_id_match(e_t);
// target_and_local_id_match(const e_t);
// target_and_local_id_match(volatile e_t);
target_and_local_id_match(union u);
target_and_local_id_match(u_t);
// target_and_local_id_match(const u_t);
// target_and_local_id_match(volatile u_t);
target_and_local_id_dont_match(struct s);
target_and_local_id_dont_match(s_t);
// target_and_local_id_dont_match(const s_t);
// target_and_local_id_dont_match(volatile s_t);
target_and_local_id_dont_match(enum e);
target_and_local_id_dont_match(e_t);
// target_and_local_id_dont_match(const e_t);
// target_and_local_id_dont_match(volatile e_t);
target_and_local_id_dont_match(union u);
target_and_local_id_dont_match(u_t);
// target_and_local_id_dont_match(const u_t);
// target_and_local_id_dont_match(volatile u_t);

return 0;
}
Expand Down
4 changes: 2 additions & 2 deletions linker.go
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ func hasFunctionReferences(insns asm.Instructions) bool {
//
// Passing a nil target will relocate against the running kernel. insns are
// modified in place.
func applyRelocations(insns asm.Instructions, target *btf.Spec, bo binary.ByteOrder) error {
func applyRelocations(insns asm.Instructions, target *btf.Spec, bo binary.ByteOrder, b *btf.Builder) error {
var relos []*btf.CORERelocation
var reloInsns []*asm.Instruction
iter := insns.Iterate()
Expand All @@ -125,7 +125,7 @@ func applyRelocations(insns asm.Instructions, target *btf.Spec, bo binary.ByteOr
bo = internal.NativeEndian
}

fixups, err := btf.CORERelocate(relos, target, bo)
fixups, err := btf.CORERelocate(relos, target, bo, b.Add)
if err != nil {
return err
}
Expand Down
32 changes: 24 additions & 8 deletions prog.go
Original file line number Diff line number Diff line change
Expand Up @@ -242,14 +242,24 @@ func newProgramWithOptions(spec *ProgramSpec, opts ProgramOptions) (*Program, er
insns := make(asm.Instructions, len(spec.Instructions))
copy(insns, spec.Instructions)

handle, fib, lib, err := btf.MarshalExtInfos(insns)
if err != nil && !errors.Is(err, btf.ErrNotSupported) {
return nil, fmt.Errorf("load ext_infos: %w", err)
var b btf.Builder
if err := applyRelocations(insns, opts.KernelTypes, spec.ByteOrder, &b); err != nil {
return nil, fmt.Errorf("apply CO-RE relocations: %w", err)
}
if handle != nil {
defer handle.Close()

attr.ProgBtfFd = uint32(handle.FD())
if !b.Empty() && haveProgramExtInfos() != nil {
// Return ErrNotSupported instead of E2BIG if CO-RE relies on BTF
// being present.
return nil, haveProgramExtInfos()
}

if haveProgramExtInfos() == nil {
// Only add func and line info if the kernel supports it. This allows
// BPF compiled with modern toolchains to work on old kernels.
fib, lib, err := btf.MarshalExtInfos(insns, &b)
if err != nil {
return nil, fmt.Errorf("marshal ext_infos: %w", err)
}

attr.FuncInfoRecSize = btf.FuncInfoSize
attr.FuncInfoCnt = uint32(len(fib)) / btf.FuncInfoSize
Expand All @@ -260,8 +270,14 @@ func newProgramWithOptions(spec *ProgramSpec, opts ProgramOptions) (*Program, er
attr.LineInfo = sys.NewSlicePointer(lib)
}

if err := applyRelocations(insns, opts.KernelTypes, spec.ByteOrder); err != nil {
return nil, fmt.Errorf("apply CO-RE relocations: %w", err)
if !b.Empty() {
handle, err := btf.NewHandle(&b)
if err != nil {
return nil, fmt.Errorf("load BTF: %w", err)
}
defer handle.Close()

attr.ProgBtfFd = uint32(handle.FD())
}

kconfig, err := resolveKconfigReferences(insns)
Expand Down
33 changes: 33 additions & 0 deletions syscalls.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"bytes"
"errors"
"fmt"
"math"
"os"
"runtime"

Expand Down Expand Up @@ -302,3 +303,35 @@ var haveSyscallWrapper = internal.NewFeatureTest("syscall wrapper", "4.17", func

return evt.Close()
})

var haveProgramExtInfos = internal.NewFeatureTest("program ext_infos", "5.0", func() error {
insns := asm.Instructions{
asm.Mov.Imm(asm.R0, 0),
asm.Return(),
}

buf := bytes.NewBuffer(make([]byte, 0, insns.Size()))
if err := insns.Marshal(buf, internal.NativeEndian); err != nil {
return err
}
bytecode := buf.Bytes()

_, err := sys.ProgLoad(&sys.ProgLoadAttr{
ProgType: sys.ProgType(SocketFilter),
License: sys.NewStringPointer("MIT"),
Insns: sys.NewSlicePointer(bytecode),
InsnCnt: uint32(len(bytecode) / asm.InstructionSize),
FuncInfoCnt: 1,
ProgBtfFd: math.MaxUint32,
})

if errors.Is(err, unix.EBADF) {
return nil
}

if errors.Is(err, unix.E2BIG) {
return ErrNotSupported
}

return err
})
4 changes: 4 additions & 0 deletions syscalls_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -62,3 +62,7 @@ func TestHaveBPFToBPFCalls(t *testing.T) {
func TestHaveSyscallWrapper(t *testing.T) {
testutils.CheckFeatureTest(t, haveSyscallWrapper)
}

func TestHaveProgramExtInfos(t *testing.T) {
testutils.CheckFeatureTest(t, haveProgramExtInfos)
}

0 comments on commit 901f16d

Please sign in to comment.