Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: support bpf_core_type_exists() #655

Merged
merged 1 commit into from May 5, 2022
Merged

Conversation

holyspectral
Copy link
Contributor

@holyspectral holyspectral commented May 3, 2022

Hi folks,

In the previous version, during relocations, when no corresponding target is found in BTF for bpf_core_type_exists(), coreCalculateFixups() will mark its relocation as poison and cause failure when verified by Linux verifier, instead of returning false at runtime:

19: (85) call unknown#195896080
invalid func unknown#195896080

This can be triggered with below ebpf program:

// +build ignore

#include "common.h"
#include "bpf_core_read.h"

char __license[] SEC("license") = "Dual MIT/GPL";

struct bpf_map_def SEC("maps") kprobe_map = {
	.type        = BPF_MAP_TYPE_ARRAY,
	.key_size    = sizeof(u32),
	.value_size  = sizeof(u64),
	.max_entries = 1,
};

struct bpf_ringbuf_2 {

};

SEC("kprobe/sys_execve")
int kprobe_execve() {
	u32 key     = 0;
	u64 initval = 1, *valp;

	int exist = bpf_core_type_exists(struct bpf_ringbuf_2);

	valp = bpf_map_lookup_elem(&kprobe_map, &key);
	if (!valp) {
		bpf_map_update_elem(&kprobe_map, &key, &initval, BPF_ANY);
		return 0;
	}
	__sync_fetch_and_add(valp, exist ? 1: 100000);

	return 0;
}

And load it with below userspace program:

package main

import (
	"os"
	"time"

	log "github.com/sirupsen/logrus"

	"github.com/cilium/ebpf"
	"github.com/cilium/ebpf/link"
)

type bpfObjects struct {
	bpfPrograms
	bpfMaps
}

type bpfPrograms struct {
	KprobeExecve *ebpf.Program `ebpf:"kprobe_execve"`
}

type bpfMaps struct {
	KprobeMap *ebpf.Map `ebpf:"kprobe_map"`
}

//go:generate go run github.com/cilium/ebpf/cmd/bpf2go@96aa1a7a929f1c954193b537b19e5e59bdd950ef -cc clang-12 -cflags $BPF_CFLAGS bpf ../../bpf/kprobe.c -- -I../../bpf/include
func main() {
	// cilium/ebpf does relocation in newProgramWithOptions().
	bpf_reader, err := os.Open("bpf_bpfel.o")
	if err != nil {
		log.WithError(err).Fatal("Failed to load bpf program.")
	}
	spec, err := ebpf.LoadCollectionSpecFromReader(bpf_reader)
	if err != nil {
		log.WithError(err).Fatal("Failed to load collection spec.")
	}

	objs := bpfObjects{}
	err = spec.LoadAndAssign(&objs, nil)
	if err != nil {
		log.WithError(err).Fatal("Failed to LoadAndAssign().")
	}

	defer objs.KprobeExecve.Close()
	defer objs.KprobeMap.Close()

	kp, err := link.Kprobe("sys_execve", objs.KprobeExecve, nil)
	if err != nil {
		log.WithError(err).Fatal("Failed to attach kprobe().")
	}
	defer kp.Close()

	ticker := time.NewTicker(1 * time.Second)
	defer ticker.Stop()
	for range ticker.C {
		const mapKey uint32 = 0
		var value uint64
		if err := objs.KprobeMap.Lookup(mapKey, &value); err != nil {
			log.WithError(err).Fatal("failed to read map")
		}
		log.Println(value)
	}
}

This issue is fixed by providing fixup for "checksForExistence" type of relocations even when no target is found.

@holyspectral holyspectral changed the title bug: support bpf_core_type_exists() fix: support bpf_core_type_exists() May 3, 2022
@holyspectral
Copy link
Contributor Author

Hi @ti-mo @lmb Would you like to take a look? Appreciated.

@lmb lmb self-requested a review May 4, 2022 17:07
@lmb
Copy link
Collaborator

lmb commented May 4, 2022

Thank you for your PR :) Could you extend internal/btf/testdata/relocs_read.c so that it exercises this code path?

In the previous version, when no target is found in BTF, coreCalculateFixups()
will mark its relocation as poison and cause failure when verified by Linux verifier.

This issue is fixed by using right fixup for "checksForExistence" type of relocations.

Signed-off-by: Shang-Wen Wang (Sam Wang) <sam_s_wang@trendmicro.com>
@ti-mo
Copy link
Collaborator

ti-mo commented May 5, 2022

Thanks!

@ti-mo ti-mo merged commit c8d1cae into cilium:master May 5, 2022
@holyspectral
Copy link
Contributor Author

@ti-mo @lmb Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants