From 48363dbee0d506cb3d6b5557e29aaecd9f9e367a Mon Sep 17 00:00:00 2001 From: Kornilios Kourtis Date: Wed, 22 Nov 2023 23:56:45 +0100 Subject: [PATCH 01/15] killer: respect mutli-probe spec setting Have the killer sensor respect the spec setting to disable multi-probe. Also, rename the options from kprobeOptions to specOptions since they refer to the whole spec rather than just the kprobe section. Signed-off-by: Kornilios Kourtis --- pkg/sensors/tracing/generickprobe.go | 9 ++++----- pkg/sensors/tracing/killer.go | 13 +++++++++++-- pkg/sensors/tracing/options.go | 10 +++++----- 3 files changed, 20 insertions(+), 12 deletions(-) diff --git a/pkg/sensors/tracing/generickprobe.go b/pkg/sensors/tracing/generickprobe.go index 3015fc2700f..eff1bd61e29 100644 --- a/pkg/sensors/tracing/generickprobe.go +++ b/pkg/sensors/tracing/generickprobe.go @@ -553,18 +553,17 @@ func createGenericKprobeSensor( kprobes := spec.KProbes lists := spec.Lists - options, err := getKprobeOptions(spec.Options) + specOpts, err := getSpecOptions(spec.Options) if err != nil { - return nil, fmt.Errorf("failed to set options: %s", err) + return nil, fmt.Errorf("failed to get spec options: %s", err) } // use multi kprobe only if: // - it's not disabled by spec option // - it's not disabled by command line option // - there's support detected - if !options.DisableKprobeMulti { - useMulti = !option.Config.DisableKprobeMulti && - bpf.HasKprobeMulti() + if !specOpts.DisableKprobeMulti { + useMulti = !option.Config.DisableKprobeMulti && bpf.HasKprobeMulti() } if useMulti { diff --git a/pkg/sensors/tracing/killer.go b/pkg/sensors/tracing/killer.go index 7cc191e0691..7d6b4f965bb 100644 --- a/pkg/sensors/tracing/killer.go +++ b/pkg/sensors/tracing/killer.go @@ -48,7 +48,7 @@ func (k killerSensor) PolicyHandler( } if len(spec.Killers) > 0 { name := fmt.Sprintf("killer-sensor-%d", atomic.AddUint64(&sensorCounter, 1)) - return createKillerSensor(spec.Killers, spec.Lists, name) + return createKillerSensor(spec.Killers, spec.Lists, spec.Options, name) } return nil, nil @@ -96,6 +96,7 @@ func unloadKiller() error { func createKillerSensor( killers []v1alpha1.KillerSpec, lists []v1alpha1.ListSpec, + opts []v1alpha1.OptionSpec, name string, ) (*sensors.Sensor, error) { @@ -140,8 +141,16 @@ func createKillerSensor( var load *program.Program var progs []*program.Program var maps []*program.Map + var useMulti bool - useMulti := !option.Config.DisableKprobeMulti && bpf.HasKprobeMulti() + specOpts, err := getSpecOptions(opts) + if err != nil { + return nil, fmt.Errorf("failed to get spec options: %s", err) + } + + if !specOpts.DisableKprobeMulti { + useMulti = !option.Config.DisableKprobeMulti && bpf.HasKprobeMulti() + } attach := fmt.Sprintf("%d syscalls: %s", len(syscallsSyms), syscallsSyms) prog := sensors.PathJoin(name, "killer_kprobe") diff --git a/pkg/sensors/tracing/options.go b/pkg/sensors/tracing/options.go index 1445d6f68a7..ce84c78d58c 100644 --- a/pkg/sensors/tracing/options.go +++ b/pkg/sensors/tracing/options.go @@ -12,26 +12,26 @@ import ( "github.com/cilium/tetragon/pkg/option" ) -type kprobeOptions struct { +type specOptions struct { DisableKprobeMulti bool } type opt struct { - set func(val string, options *kprobeOptions) error + set func(val string, options *specOptions) error } // Allowed kprobe options var opts = map[string]opt{ option.KeyDisableKprobeMulti: opt{ - set: func(str string, options *kprobeOptions) (err error) { + set: func(str string, options *specOptions) (err error) { options.DisableKprobeMulti, err = strconv.ParseBool(str) return err }, }, } -func getKprobeOptions(specs []v1alpha1.OptionSpec) (*kprobeOptions, error) { - options := &kprobeOptions{} +func getSpecOptions(specs []v1alpha1.OptionSpec) (*specOptions, error) { + options := &specOptions{} for _, spec := range specs { opt, ok := opts[spec.Name] From da53e4ea87cea59c0eebbfc803d9b24785be2f92 Mon Sep 17 00:00:00 2001 From: Kornilios Kourtis Date: Tue, 19 Dec 2023 14:59:21 +0100 Subject: [PATCH 02/15] bpf: killer: add an fmod_ret version Some kernels do not include CONFIG_BPF_KPROBE_OVERRIDE so it's not possible to use bpf_override_return(). Instead, we can use fmod_ret. Previously, we were building two versions of the killer program: one with multi-kprobe and one without. With this patch, we build a third version: one that uses fmod_ret. Signed-off-by: Kornilios Kourtis --- bpf/Makefile | 31 +++++++++++++++++++++------ bpf/process/bpf_killer.c | 45 +++++++++++++++++++++++++++++++--------- 2 files changed, 60 insertions(+), 16 deletions(-) diff --git a/bpf/Makefile b/bpf/Makefile index 477d110b605..2e1ce7f86a7 100644 --- a/bpf/Makefile +++ b/bpf/Makefile @@ -21,7 +21,7 @@ PROCESS = bpf_execve_event.o bpf_execve_event_v53.o bpf_fork.o bpf_exit.o bpf_ge bpf_multi_kprobe_v61.o bpf_multi_retkprobe_v61.o \ bpf_generic_uprobe_v61.o \ bpf_loader.o \ - bpf_killer.o bpf_multi_killer.o + bpf_killer.o bpf_multi_killer.o bpf_fmodret_killer.o CGROUP = bpf_cgroup_mkdir.o bpf_cgroup_rmdir.o bpf_cgroup_release.o BPFTEST = bpf_lseek.o bpf_globals.o @@ -75,6 +75,30 @@ objs/%.ll: $(ALIGNCHECKERDIR)%.c $(DEPSDIR)%.d: $(ALIGNCHECKERDIR)%.c $(CLANG) $(CLANG_FLAGS) -MM -MP -MT $(patsubst $(DEPSDIR)%.d, $(OBJSDIR)%.ll, $@) $< > $@ + +# Killer programs: bpf_killer, bpf_multi_killer, bpf_fmodret_killer + +## bpf_killer: __BPF_OVERRIDE_RETURN, but no __MULTI_KPROBE +objs/bpf_killer.ll: process/bpf_killer.c + $(CLANG) $(CLANG_FLAGS) -D__BPF_OVERRIDE_RETURN -c $< -o $@ + +$(DEPSDIR)bpf_killer.d: process/bpf_killer.c + $(CLANG) $(CLANG_FLAGS) -D__BPF_OVERRIDE_RETURN -MM -MP -MT $(patsubst $(DEPSDIR)%.d, $(OBJSDIR)%.ll, $@) $< > $@ + +## bpf_multi_killer: __BPF_OVERRIDE_RETURN and __MULTI_KPROBE +objs/bpf_multi_killer.ll: process/bpf_killer.c + $(CLANG) $(CLANG_FLAGS) -D__BPF_OVERRIDE_RETURN -D__MULTI_KPROBE -c $< -o $@ + +$(DEPSDIR)/bpf_multi_killer.d: process/bpf_killer.c + $(CLANG) $(CLANG_FLAGS) -D__BPF_OVERRIDE_RETURN -D__MULTI_KPROBE -MM -MP -MT $(patsubst $(DEPSDIR)%.d, $(OBJSDIR)%.ll, $@) $< > $@ + +## bpf_fmodret_killer no bpf_override_return: we need fmod_ret +objs/bpf_fmodret_killer.ll: process/bpf_killer.c + $(CLANG) $(CLANG_FLAGS) -c $< -o $@ + +$(DEPSDIR)/bpf_fmodret_killer.d: process/bpf_killer.c + $(CLANG) $(CLANG_FLAGS) -MM -MP -MT $(patsubst $(DEPSDIR)%.d, $(OBJSDIR)%.ll, $@) $< > $@ + # PROCESSDIR objs/%.ll: $(PROCESSDIR)%.c $(CLANG) $(CLANG_FLAGS) -c $< -o $@ @@ -88,11 +112,6 @@ objs/%_v53.ll: $(DEPSDIR)%.d: $(PROCESSDIR)%.c $(CLANG) $(CLANG_FLAGS) -MM -MP -MT $(patsubst $(DEPSDIR)%.d, $(OBJSDIR)%.ll, $@) $< > $@ -objs/bpf_multi_killer.ll: process/bpf_killer.c - $(CLANG) $(CLANG_FLAGS) -D__LARGE_BPF_PROG -D__MULTI_KPROBE -c $< -o $@ - -$(DEPSDIR)/bpf_multi_killer.d: process/bpf_killer.c - $(CLANG) $(CLANG_FLAGS) -D__LARGE_BPF_PROG -D__MULTI_KPROBE -MM -MP -MT $(patsubst $(DEPSDIR)%.d, $(OBJSDIR)%.ll, $@) $< > $@ $(DEPSDIR)%_v53.d: $(CLANG) $(CLANG_FLAGS) -D__LARGE_BPF_PROG -MM -MP -MT $(patsubst $(DEPSDIR)%.d, $(OBJSDIR)%.ll, $@) $< > $@ diff --git a/bpf/process/bpf_killer.c b/bpf/process/bpf_killer.c index 7ff4d0024ce..91eda228818 100644 --- a/bpf/process/bpf_killer.c +++ b/bpf/process/bpf_killer.c @@ -2,14 +2,8 @@ char _license[] __attribute__((section("license"), used)) = "Dual BSD/GPL"; -#ifdef __MULTI_KPROBE -#define MAIN "kprobe.multi/killer" -#else -#define MAIN "kprobe/killer" -#endif - -__attribute__((section(MAIN), used)) int -killer(void *ctx) +static inline __attribute__((always_inline)) int +do_killer(void *ctx) { __u64 id = get_current_pid_tgid(); struct killer_data *data; @@ -18,11 +12,42 @@ killer(void *ctx) if (!data) return 0; - if (data->error) - override_return(ctx, data->error); if (data->signal) send_signal(data->signal); map_delete_elem(&killer_data, &id); + return data->error; +} + +#if defined(__BPF_OVERRIDE_RETURN) + +#ifdef __MULTI_KPROBE +#define MAIN "kprobe.multi/killer" +#else +#define MAIN "kprobe/killer" +#endif + +__attribute__((section(MAIN), used)) int +multi_kprobe_killer(void *ctx) +{ + long ret; + + ret = do_killer(ctx); + if (ret) + override_return(ctx, ret); + return 0; } + +#else /* !__BPF_OVERRIDE_RETURN */ + +/* Putting security_task_prctl in here to pass contrib/verify/verify.sh test, + * in normal run the function is set by tetragon dynamically. + */ +__attribute__((section("fmod_ret/security_task_prctl"), used)) long +fmodret_killer(void *ctx) +{ + return do_killer(ctx); +} + +#endif From 23d22e0cbb74271028c0b91bd71f317026ca5f63 Mon Sep 17 00:00:00 2001 From: Kornilios Kourtis Date: Wed, 20 Dec 2023 08:25:01 +0100 Subject: [PATCH 03/15] killer sensor: support fmod_ret This patch adds support for using fmod_ret in the killer sensor. This is a less preferable option than bpf_override_return() because it requires multiple programs. Signed-off-by: Kornilios Kourtis --- pkg/sensors/program/loader.go | 36 +++++++++++++++++++++++ pkg/sensors/tracing/killer.go | 47 ++++++++++++++++++++---------- pkg/sensors/tracing/killer_test.go | 2 +- 3 files changed, 69 insertions(+), 16 deletions(-) diff --git a/pkg/sensors/program/loader.go b/pkg/sensors/program/loader.go index a054da29eb5..dd0d84d5f3e 100644 --- a/pkg/sensors/program/loader.go +++ b/pkg/sensors/program/loader.go @@ -528,6 +528,42 @@ func LoadMultiKprobeProgram(bpfDir, mapDir string, load *Program, verbose int) e return loadProgram(bpfDir, []string{mapDir}, load, opts, verbose) } +func LoadFmodRetProgram(bpfDir, mapDir string, load *Program, progName string, verbose int) error { + opts := &loadOpts{ + attach: func( + coll *ebpf.Collection, + collSpec *ebpf.CollectionSpec, + prog *ebpf.Program, + spec *ebpf.ProgramSpec, + ) (unloader.Unloader, error) { + linkFn := func() (link.Link, error) { + return link.AttachTracing(link.TracingOptions{ + Program: prog, + }) + } + lnk, err := linkFn() + if err != nil { + return nil, fmt.Errorf("attaching '%s' failed: %w", spec.Name, err) + } + return &unloader.RelinkUnloader{ + UnloadProg: unloader.PinUnloader{Prog: prog}.Unload, + IsLinked: true, + Link: lnk, + RelinkFn: linkFn, + }, nil + }, + open: func(coll *ebpf.CollectionSpec) error { + progSpec, ok := coll.Programs[progName] + if !ok { + return fmt.Errorf("progName %s not in collecition spec programs: %+v", progName, coll.Programs) + } + progSpec.AttachTo = load.Attach + return nil + }, + } + return loadProgram(bpfDir, []string{mapDir}, load, opts, verbose) +} + func LoadTracingProgram(bpfDir, mapDir string, load *Program, verbose int) error { opts := &loadOpts{ attach: TracingAttach(), diff --git a/pkg/sensors/tracing/killer.go b/pkg/sensors/tracing/killer.go index 7d6b4f965bb..b846b69e9ad 100644 --- a/pkg/sensors/tracing/killer.go +++ b/pkg/sensors/tracing/killer.go @@ -80,10 +80,18 @@ func loadMultiKillerSensor(bpfDir, mapDir string, load *program.Program, verbose } func (k *killerSensor) LoadProbe(args sensors.LoadProbeArgs) error { + if args.Load.Label == "kprobe.multi/killer" { + return loadMultiKillerSensor(args.BPFDir, args.MapDir, args.Load, args.Verbose) + } if args.Load.Label == "kprobe/killer" { return loadSingleKillerSensor(args.BPFDir, args.MapDir, args.Load, args.Verbose) } - return loadMultiKillerSensor(args.BPFDir, args.MapDir, args.Load, args.Verbose) + + if strings.HasPrefix(args.Load.Label, "fmod_ret/") { + return program.LoadFmodRetProgram(args.BPFDir, args.MapDir, args.Load, "fmodret_killer", args.Verbose) + } + + return fmt.Errorf("killer loader: unknown label: %s", args.Load.Label) } func unloadKiller() error { @@ -152,29 +160,38 @@ func createKillerSensor( useMulti = !option.Config.DisableKprobeMulti && bpf.HasKprobeMulti() } - attach := fmt.Sprintf("%d syscalls: %s", len(syscallsSyms), syscallsSyms) prog := sensors.PathJoin(name, "killer_kprobe") - - if useMulti { + if bpf.HasOverrideHelper() { + attach := fmt.Sprintf("%d syscalls: %s", len(syscallsSyms), syscallsSyms) + label := "kprobe/killer" + prog := "bpf_killer.o" + if useMulti { + label = "kprobe.multi/killer" + prog = "bpf_multi_killer.o" + } load = program.Builder( - path.Join(option.Config.HubbleLib, "bpf_multi_killer.o"), + path.Join(option.Config.HubbleLib, prog), attach, - "kprobe.multi/killer", + label, prog, "killer") - + progs = append(progs, load) + } else if bpf.HasModifyReturn() { + // for fmod_ret, we need one program per syscall + for _, syscallSym := range syscallsSyms { + load = program.Builder( + path.Join(option.Config.HubbleLib, "bpf_fmodret_killer.o"), + syscallSym, + "fmod_ret/security_task_prctl", + prog, + "killer") + progs = append(progs, load) + } } else { - load = program.Builder( - path.Join(option.Config.HubbleLib, "bpf_killer.o"), - attach, - "kprobe/killer", - prog, - "killer") + return nil, fmt.Errorf("no override helper or override support: cannot load killer") } killerDataMap := program.MapBuilderPin("killer_data", "killer_data", load) - - progs = append(progs, load) maps = append(maps, killerDataMap) return &sensors.Sensor{ diff --git a/pkg/sensors/tracing/killer_test.go b/pkg/sensors/tracing/killer_test.go index fd2eac6f407..5ec52884de6 100644 --- a/pkg/sensors/tracing/killer_test.go +++ b/pkg/sensors/tracing/killer_test.go @@ -63,7 +63,7 @@ func testKiller(t *testing.T, configHook string, } func TestKillerOverride(t *testing.T) { - if !bpf.HasOverrideHelper() { + if !bpf.HasOverrideHelper() && !bpf.HasModifyReturn() { t.Skip("skipping killer test, bpf_override_return helper not available") } if !bpf.HasSignalHelper() { From 0ea9b5c3985b4025142e1fb7ee46122e74b6396f Mon Sep 17 00:00:00 2001 From: Kornilios Kourtis Date: Tue, 9 Jan 2024 10:24:13 +0100 Subject: [PATCH 04/15] killer sensor: fail if signal helper is missing The killer program uses bpf_send_signal(). If the helper is not supported, it cannot be loaded. Add a check in the killer sensor for this. We could have another version of the killer program that does not use bpf_send_signal() and can only be used for override, but this is beyond the scope of this patch. Signed-off-by: Kornilios Kourtis --- pkg/sensors/tracing/killer.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/pkg/sensors/tracing/killer.go b/pkg/sensors/tracing/killer.go index b846b69e9ad..7d8cf1c6ed9 100644 --- a/pkg/sensors/tracing/killer.go +++ b/pkg/sensors/tracing/killer.go @@ -160,6 +160,10 @@ func createKillerSensor( useMulti = !option.Config.DisableKprobeMulti && bpf.HasKprobeMulti() } + if !bpf.HasSignalHelper() { + return nil, fmt.Errorf("killer sensor requires signal helper which is not available") + } + prog := sensors.PathJoin(name, "killer_kprobe") if bpf.HasOverrideHelper() { attach := fmt.Sprintf("%d syscalls: %s", len(syscallsSyms), syscallsSyms) From e910f59c72669b08d336f9965b7aad7db8945372 Mon Sep 17 00:00:00 2001 From: Kornilios Kourtis Date: Wed, 20 Dec 2023 16:41:56 +0100 Subject: [PATCH 05/15] introduce builder for killer policies KillerSpecBuilder is a simple builder for creating tracing policies that use the killer sensor. It is meant for testing and it will be used in subsequent patches. Signed-off-by: Kornilios Kourtis --- pkg/sensors/tracing/killer_builder.go | 199 ++++++++++++++++++++++++++ 1 file changed, 199 insertions(+) create mode 100644 pkg/sensors/tracing/killer_builder.go diff --git a/pkg/sensors/tracing/killer_builder.go b/pkg/sensors/tracing/killer_builder.go new file mode 100644 index 00000000000..ade59a370cf --- /dev/null +++ b/pkg/sensors/tracing/killer_builder.go @@ -0,0 +1,199 @@ +// SPDX-License-Identifier: Apache-2.0 +// Copyright Authors of Tetragon + +package tracing + +import ( + "fmt" + "log" + + k8sv1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "sigs.k8s.io/yaml" + + "github.com/cilium/tetragon/pkg/k8s/apis/cilium.io/v1alpha1" + "github.com/cilium/tetragon/pkg/option" +) + +type KillerSpecBuilder struct { + name string + syscalls [][]string + kill *uint32 + override *int32 + binaries []string + overrideMethod string + multiKprobe *bool +} + +func NewKillerSpecBuilder(name string) *KillerSpecBuilder { + return &KillerSpecBuilder{ + name: name, + } +} + +func (ksb *KillerSpecBuilder) WithSyscallList(calls ...string) *KillerSpecBuilder { + ksb.syscalls = append(ksb.syscalls, calls) + return ksb +} + +func (ksb *KillerSpecBuilder) WithKill(sig uint32) *KillerSpecBuilder { + ksb.kill = &sig + return ksb +} + +func (ksb *KillerSpecBuilder) WithMultiKprobe() *KillerSpecBuilder { + multi := true + ksb.multiKprobe = &multi + return ksb +} + +func (ksb *KillerSpecBuilder) WithoutMultiKprobe() *KillerSpecBuilder { + multi := false + ksb.multiKprobe = &multi + return ksb +} + +func (ksb *KillerSpecBuilder) WithOverrideValue(ret int32) *KillerSpecBuilder { + ksb.override = &ret + return ksb +} + +func (ksb *KillerSpecBuilder) WithMatchBinaries(bins ...string) *KillerSpecBuilder { + ksb.binaries = append(ksb.binaries, bins...) + return ksb +} + +func (ksb *KillerSpecBuilder) WithOverrideReturn() *KillerSpecBuilder { + ksb.overrideMethod = valOverrideReturn + return ksb +} + +func (ksb *KillerSpecBuilder) WithFmodRet() *KillerSpecBuilder { + ksb.overrideMethod = valFmodRet + return ksb + +} + +func (ksb *KillerSpecBuilder) WithDefaultOverride() *KillerSpecBuilder { + ksb.overrideMethod = "" + return ksb +} + +func (ksb *KillerSpecBuilder) MustBuild() *v1alpha1.TracingPolicy { + spec, err := ksb.Build() + if err != nil { + log.Fatalf("MustBuild failed with %v", err) + } + return spec +} + +func (ksb *KillerSpecBuilder) MustYAML() string { + tp, err := ksb.Build() + if err != nil { + log.Fatalf("MustYAML: build failed with %v", err) + } + + b, err := yaml.Marshal(tp) + if err != nil { + log.Fatalf("MustYAML: marshal failed with %v", err) + } + return string(b) +} + +func (ksb *KillerSpecBuilder) Build() (*v1alpha1.TracingPolicy, error) { + + var listNames []string + var lists []v1alpha1.ListSpec + var killers []v1alpha1.KillerSpec + var matchBinaries []v1alpha1.BinarySelector + var options []v1alpha1.OptionSpec + + for i, syscallList := range ksb.syscalls { + var name string + if len(ksb.syscalls) > 1 { + name = fmt.Sprintf("%s-%d", ksb.name, i+1) + } else { + name = ksb.name + } + listName := fmt.Sprintf("list:%s", name) + listNames = append(listNames, listName) + lists = append(lists, v1alpha1.ListSpec{ + Name: name, + Type: "syscalls", + Values: syscallList, + Pattern: nil, + Validated: false, + }) + killers = append(killers, v1alpha1.KillerSpec{ + Syscalls: []string{listName}, + }) + } + + actions := []v1alpha1.ActionSelector{{Action: "NotifyKiller"}} + act := &actions[0] + if ksb.kill == nil && ksb.override == nil { + return nil, fmt.Errorf("need either override or kill to notify killer") + } + if ksb.kill != nil { + act.ArgSig = *ksb.kill + } + if ksb.override != nil { + act.ArgError = *ksb.override + } + + if len(ksb.binaries) > 0 { + matchBinaries = []v1alpha1.BinarySelector{{ + Operator: "In", + Values: ksb.binaries, + }} + } + + if ksb.overrideMethod != "" { + options = append(options, v1alpha1.OptionSpec{ + Name: keyOverrideMethod, + Value: ksb.overrideMethod, + }) + } + + if ksb.multiKprobe != nil { + options = append(options, v1alpha1.OptionSpec{ + Name: option.KeyDisableKprobeMulti, + Value: fmt.Sprintf("%t", *ksb.multiKprobe), + }) + } + + // NB: We might want to add options for these in the future + syscallIDTy := "syscall64" + operator := "InMap" + + return &v1alpha1.TracingPolicy{ + TypeMeta: k8sv1.TypeMeta{ + Kind: "TracingPolicy", + APIVersion: "cilium.io/v1alpha1", + }, + ObjectMeta: k8sv1.ObjectMeta{ + Name: ksb.name, + }, + Spec: v1alpha1.TracingPolicySpec{ + Lists: lists, + Tracepoints: []v1alpha1.TracepointSpec{{ + Subsystem: "raw_syscalls", + Event: "sys_enter", + Args: []v1alpha1.KProbeArg{{ + Index: 4, + Type: syscallIDTy, + }}, + Selectors: []v1alpha1.KProbeSelector{{ + MatchArgs: []v1alpha1.ArgSelector{{ + Index: 0, + Operator: operator, + Values: listNames, + }}, + MatchActions: actions, + MatchBinaries: matchBinaries, + }}, + }}, + Killers: killers, + Options: options, + }, + }, nil +} From f6ba44811818371db5a18ad87f753df7487e22f8 Mon Sep 17 00:00:00 2001 From: Kornilios Kourtis Date: Tue, 9 Jan 2024 08:43:55 +0100 Subject: [PATCH 06/15] killer: add spec option for override method There are two ways to override the return value of a syscall: using bpf_override_return() and fmod_ret. The former is preferable because we do not need multiple programs. The killer sensor detects what is available and acts accordingly. This PR adds a spec option that forces a specific override method. While this might be useful to end-users, it is mainly intended for testing because it allows us to test fmod_ret for kernels that do support bpf_override_return(). It will be used in a subsequent patch. Signed-off-by: Kornilios Kourtis --- pkg/sensors/tracing/killer.go | 58 ++++++++++++++++++++++++++-------- pkg/sensors/tracing/options.go | 48 ++++++++++++++++++++++++++-- 2 files changed, 89 insertions(+), 17 deletions(-) diff --git a/pkg/sensors/tracing/killer.go b/pkg/sensors/tracing/killer.go index 7d8cf1c6ed9..6440564c35a 100644 --- a/pkg/sensors/tracing/killer.go +++ b/pkg/sensors/tracing/killer.go @@ -94,6 +94,32 @@ func (k *killerSensor) LoadProbe(args sensors.LoadProbeArgs) error { return fmt.Errorf("killer loader: unknown label: %s", args.Load.Label) } +// select proper override method based on configuration and spec options +func selectOverrideMethod(specOpts *specOptions) (OverrideMethod, error) { + overrideMethod := specOpts.OverrideMethod + switch overrideMethod { + case OverrideMethodDefault: + // by default, first try OverrideReturn and if this does not work try fmod_ret + if bpf.HasOverrideHelper() { + overrideMethod = OverrideMethodReturn + } else if bpf.HasModifyReturn() { + overrideMethod = OverrideMethodFmodRet + } else { + return OverrideMethodInvalid, fmt.Errorf("no override helper or mod_ret support: cannot load killer") + } + case OverrideMethodReturn: + if !bpf.HasOverrideHelper() { + return OverrideMethodInvalid, fmt.Errorf("option override return set, but it is not supported") + } + case OverrideMethodFmodRet: + if !bpf.HasModifyReturn() { + return OverrideMethodInvalid, fmt.Errorf("option fmod_ret set, but it is not supported") + } + } + + return overrideMethod, nil +} + func unloadKiller() error { configured = false syscallsSyms = []string{} @@ -149,50 +175,54 @@ func createKillerSensor( var load *program.Program var progs []*program.Program var maps []*program.Map - var useMulti bool - specOpts, err := getSpecOptions(opts) if err != nil { return nil, fmt.Errorf("failed to get spec options: %s", err) } - if !specOpts.DisableKprobeMulti { - useMulti = !option.Config.DisableKprobeMulti && bpf.HasKprobeMulti() - } - if !bpf.HasSignalHelper() { return nil, fmt.Errorf("killer sensor requires signal helper which is not available") } - prog := sensors.PathJoin(name, "killer_kprobe") - if bpf.HasOverrideHelper() { - attach := fmt.Sprintf("%d syscalls: %s", len(syscallsSyms), syscallsSyms) + // select proper override method based on configuration and spec options + overrideMethod, err := selectOverrideMethod(specOpts) + if err != nil { + return nil, err + } + + pinPath := sensors.PathJoin(name, "killer_kprobe") + switch overrideMethod { + case OverrideMethodReturn: + useMulti := !specOpts.DisableKprobeMulti && !option.Config.DisableKprobeMulti && bpf.HasKprobeMulti() + logger.GetLogger().Infof("killer: using override return (multi-kprobe: %t)", useMulti) label := "kprobe/killer" prog := "bpf_killer.o" if useMulti { label = "kprobe.multi/killer" prog = "bpf_multi_killer.o" } + attach := fmt.Sprintf("%d syscalls: %s", len(syscallsSyms), syscallsSyms) load = program.Builder( path.Join(option.Config.HubbleLib, prog), attach, label, - prog, + pinPath, "killer") progs = append(progs, load) - } else if bpf.HasModifyReturn() { + case OverrideMethodFmodRet: // for fmod_ret, we need one program per syscall + logger.GetLogger().Infof("killer: using fmod_ret") for _, syscallSym := range syscallsSyms { load = program.Builder( path.Join(option.Config.HubbleLib, "bpf_fmodret_killer.o"), syscallSym, "fmod_ret/security_task_prctl", - prog, + pinPath, "killer") progs = append(progs, load) } - } else { - return nil, fmt.Errorf("no override helper or override support: cannot load killer") + default: + return nil, fmt.Errorf("unexpected override method: %d", overrideMethod) } killerDataMap := program.MapBuilderPin("killer_data", "killer_data", load) diff --git a/pkg/sensors/tracing/options.go b/pkg/sensors/tracing/options.go index ce84c78d58c..ac34b853ed9 100644 --- a/pkg/sensors/tracing/options.go +++ b/pkg/sensors/tracing/options.go @@ -12,14 +12,48 @@ import ( "github.com/cilium/tetragon/pkg/option" ) +type OverrideMethod int + +const ( + keyOverrideMethod = "override-method" + valFmodRet = "fmod-ret" + valOverrideReturn = "override-return" +) + +const ( + OverrideMethodDefault OverrideMethod = iota + OverrideMethodReturn + OverrideMethodFmodRet + OverrideMethodInvalid +) + +func overrideMethodParse(s string) OverrideMethod { + switch s { + case valFmodRet: + return OverrideMethodFmodRet + case valOverrideReturn: + return OverrideMethodReturn + default: + return OverrideMethodInvalid + } +} + type specOptions struct { DisableKprobeMulti bool + OverrideMethod OverrideMethod } type opt struct { set func(val string, options *specOptions) error } +func newDefaultSpecOptions() *specOptions { + return &specOptions{ + DisableKprobeMulti: false, + OverrideMethod: OverrideMethodDefault, + } +} + // Allowed kprobe options var opts = map[string]opt{ option.KeyDisableKprobeMulti: opt{ @@ -28,11 +62,20 @@ var opts = map[string]opt{ return err }, }, + keyOverrideMethod: opt{ + set: func(str string, options *specOptions) (err error) { + m := overrideMethodParse(str) + if m == OverrideMethodInvalid { + return fmt.Errorf("invalid override method: '%s'", str) + } + options.OverrideMethod = m + return nil + }, + }, } func getSpecOptions(specs []v1alpha1.OptionSpec) (*specOptions, error) { - options := &specOptions{} - + options := newDefaultSpecOptions() for _, spec := range specs { opt, ok := opts[spec.Name] if ok { @@ -42,6 +85,5 @@ func getSpecOptions(specs []v1alpha1.OptionSpec) (*specOptions, error) { logger.GetLogger().Infof("Set option %s = %s", spec.Name, spec.Value) } } - return options, nil } From a8f5f5af953840a52c739b1f8a800106541de024 Mon Sep 17 00:00:00 2001 From: Kornilios Kourtis Date: Tue, 9 Jan 2024 12:11:52 +0100 Subject: [PATCH 07/15] tester-progs: add killer-tester-32 to ignore Signed-off-by: Kornilios Kourtis --- contrib/tester-progs/.gitignore | 1 + 1 file changed, 1 insertion(+) diff --git a/contrib/tester-progs/.gitignore b/contrib/tester-progs/.gitignore index db3a507daa5..28d1a691c78 100644 --- a/contrib/tester-progs/.gitignore +++ b/contrib/tester-progs/.gitignore @@ -17,3 +17,4 @@ threads-tester bench-reader threads-exit killer-tester +killer-tester-32 From 59d70dbecd9c66a2c0f2e57600fa833cdfaa6bf8 Mon Sep 17 00:00:00 2001 From: Kornilios Kourtis Date: Tue, 9 Jan 2024 15:26:23 +0100 Subject: [PATCH 08/15] killer_test: use builder Modify the killer tests to use the builder introduced in a previous patch. Signed-off-by: Kornilios Kourtis --- pkg/sensors/tracing/killer_amd64_test.go | 134 ++++------------------ pkg/sensors/tracing/killer_test.go | 137 ++++------------------- 2 files changed, 45 insertions(+), 226 deletions(-) diff --git a/pkg/sensors/tracing/killer_amd64_test.go b/pkg/sensors/tracing/killer_amd64_test.go index 9383ff0e8fb..22b3e396020 100644 --- a/pkg/sensors/tracing/killer_amd64_test.go +++ b/pkg/sensors/tracing/killer_amd64_test.go @@ -20,48 +20,19 @@ import ( ) func TestKillerOverride32(t *testing.T) { - if !bpf.HasOverrideHelper() { - t.Skip("skipping killer test, bpf_override_return helper not available") + if !bpf.HasOverrideHelper() && !bpf.HasModifyReturn() { + t.Skip("skipping killer test, neither bpf_override_return nor fmod_ret is available") } if !bpf.HasSignalHelper() { t.Skip("skipping killer test, bpf_send_signal helper not available") } test := testutils.RepoRootPath("contrib/tester-progs/killer-tester-32") - configHook := ` -apiVersion: cilium.io/v1alpha1 -kind: TracingPolicy -metadata: - name: "kill-syscalls" -spec: - lists: - - name: "mine" - type: "syscalls" - values: - - "__ia32_sys_prctl" - killers: - - syscalls: - - "list:mine" - tracepoints: - - subsystem: "raw_syscalls" - event: "sys_enter" - args: - - index: 4 - type: "syscall64" - selectors: - - matchArgs: - - index: 0 - operator: "InMap" - values: - - "list:mine" - matchBinaries: - - operator: "In" - values: - - "` + test + `" - matchActions: - - action: "NotifyKiller" - argError: -17 # EEXIST -` + yaml := NewKillerSpecBuilder("killer-override"). + WithSyscallList("__ia32_sys_prctl"). + WithMatchBinaries(test). + WithOverrideValue(-17). // EEXIST + MustYAML() tpChecker := ec.NewProcessTracepointChecker(""). WithArgs(ec.NewKprobeArgumentListMatcher(). @@ -79,11 +50,11 @@ spec: } } - testKiller(t, configHook, test, "", checker, checkerFunc) + testKiller(t, yaml, test, "", checker, checkerFunc) } func TestKillerSignal32(t *testing.T) { - if !bpf.HasOverrideHelper() { + if !bpf.HasOverrideHelper() && !bpf.HasModifyReturn() { t.Skip("skipping killer test, bpf_override_return helper not available") } if !bpf.HasSignalHelper() { @@ -91,40 +62,12 @@ func TestKillerSignal32(t *testing.T) { } test := testutils.RepoRootPath("contrib/tester-progs/killer-tester-32") - configHook := ` -apiVersion: cilium.io/v1alpha1 -kind: TracingPolicy -metadata: - name: "kill-syscalls" -spec: - lists: - - name: "mine" - type: "syscalls" - values: - - "__ia32_sys_prctl" - killers: - - syscalls: - - "list:mine" - tracepoints: - - subsystem: "raw_syscalls" - event: "sys_enter" - args: - - index: 4 - type: "syscall64" - selectors: - - matchArgs: - - index: 0 - operator: "InMap" - values: - - "list:mine" - matchBinaries: - - operator: "In" - values: - - "` + test + `" - matchActions: - - action: "NotifyKiller" - argSig: 9 # SIGKILL -` + yaml := NewKillerSpecBuilder("killer-signal"). + WithSyscallList("__ia32_sys_prctl"). + WithMatchBinaries(test). + WithOverrideValue(-17). // EEXIST + WithKill(9). // SigKill + MustYAML() tpChecker := ec.NewProcessTracepointChecker(""). WithArgs(ec.NewKprobeArgumentListMatcher(). @@ -142,11 +85,11 @@ spec: } } - testKiller(t, configHook, test, "", checker, checkerFunc) + testKiller(t, yaml, test, "", checker, checkerFunc) } func TestKillerOverrideBothBits(t *testing.T) { - if !bpf.HasOverrideHelper() { + if !bpf.HasOverrideHelper() && !bpf.HasModifyReturn() { t.Skip("skipping killer test, bpf_override_return helper not available") } if !bpf.HasSignalHelper() { @@ -156,42 +99,11 @@ func TestKillerOverrideBothBits(t *testing.T) { test32 := testutils.RepoRootPath("contrib/tester-progs/killer-tester-32") test64 := testutils.RepoRootPath("contrib/tester-progs/killer-tester") - configHook := ` -apiVersion: cilium.io/v1alpha1 -kind: TracingPolicy -metadata: - name: "kill-syscalls" -spec: - lists: - - name: "mine" - type: "syscalls" - values: - - "sys_prctl" - - "__ia32_sys_prctl" - killers: - - syscalls: - - "list:mine" - tracepoints: - - subsystem: "raw_syscalls" - event: "sys_enter" - args: - - index: 4 - type: "syscall64" - selectors: - - matchArgs: - - index: 0 - operator: "InMap" - values: - - "list:mine" - matchBinaries: - - operator: "In" - values: - - "` + test32 + `" - - "` + test64 + `" - matchActions: - - action: "NotifyKiller" - argError: -17 # EEXIST -` + yaml := NewKillerSpecBuilder("killer-override"). + WithSyscallList("__ia32_sys_prctl", "sys_prctl"). + WithMatchBinaries(test32, test64). + WithOverrideValue(-17). // EEXIST + MustYAML() tpChecker32 := ec.NewProcessTracepointChecker(""). WithArgs(ec.NewKprobeArgumentListMatcher(). @@ -217,5 +129,5 @@ spec: } } - testKiller(t, configHook, test64, test32, checker, checkerFunc) + testKiller(t, yaml, test64, test32, checker, checkerFunc) } diff --git a/pkg/sensors/tracing/killer_test.go b/pkg/sensors/tracing/killer_test.go index 5ec52884de6..a51783969ce 100644 --- a/pkg/sensors/tracing/killer_test.go +++ b/pkg/sensors/tracing/killer_test.go @@ -71,40 +71,11 @@ func TestKillerOverride(t *testing.T) { } test := testutils.RepoRootPath("contrib/tester-progs/killer-tester") - configHook := ` -apiVersion: cilium.io/v1alpha1 -kind: TracingPolicy -metadata: - name: "kill-syscalls" -spec: - lists: - - name: "mine" - type: "syscalls" - values: - - "sys_prctl" - killers: - - syscalls: - - "list:mine" - tracepoints: - - subsystem: "raw_syscalls" - event: "sys_enter" - args: - - index: 4 - type: "syscall64" - selectors: - - matchArgs: - - index: 0 - operator: "InMap" - values: - - "list:mine" - matchBinaries: - - operator: "In" - values: - - "` + test + `" - matchActions: - - action: "NotifyKiller" - argError: -17 # EEXIST -` + yaml := NewKillerSpecBuilder("killer-override"). + WithSyscallList("sys_prctl"). + WithMatchBinaries(test). + WithOverrideValue(-17). // EEXIST + MustYAML() tpChecker := ec.NewProcessTracepointChecker(""). WithArgs(ec.NewKprobeArgumentListMatcher(). @@ -122,52 +93,24 @@ spec: } } - testKiller(t, configHook, test, "", checker, checkerFunc) + testKiller(t, yaml, test, "", checker, checkerFunc) } func TestKillerSignal(t *testing.T) { - if !bpf.HasOverrideHelper() { - t.Skip("skipping killer test, bpf_override_return helper not available") + if !bpf.HasOverrideHelper() && !bpf.HasModifyReturn() { + t.Skip("skipping killer test, neither bpf_override_return nor fmod_ret is available") } if !bpf.HasSignalHelper() { t.Skip("skipping killer test, bpf_send_signal helper not available") } test := testutils.RepoRootPath("contrib/tester-progs/killer-tester") - configHook := ` -apiVersion: cilium.io/v1alpha1 -kind: TracingPolicy -metadata: - name: "kill-syscalls" -spec: - lists: - - name: "mine" - type: "syscalls" - values: - - "sys_prctl" - killers: - - syscalls: - - "list:mine" - tracepoints: - - subsystem: "raw_syscalls" - event: "sys_enter" - args: - - index: 4 - type: "syscall64" - selectors: - - matchArgs: - - index: 0 - operator: "InMap" - values: - - "list:mine" - matchBinaries: - - operator: "In" - values: - - "` + test + `" - matchActions: - - action: "NotifyKiller" - argSig: 9 # SIGKILL -` + yaml := NewKillerSpecBuilder("killer-signal"). + WithSyscallList("sys_prctl"). + WithMatchBinaries(test). + WithOverrideValue(-17). // EEXIST + WithKill(9). // SigKill + MustYAML() tpChecker := ec.NewProcessTracepointChecker(""). WithArgs(ec.NewKprobeArgumentListMatcher(). @@ -185,51 +128,15 @@ spec: } } - testKiller(t, configHook, test, "", checker, checkerFunc) + testKiller(t, yaml, test, "", checker, checkerFunc) } -func TestKillerMulti(t *testing.T) { - if !bpf.HasOverrideHelper() { - t.Skip("skipping killer test, bpf_override_return helper not available") - } - - crd := ` -apiVersion: cilium.io/v1alpha1 -kind: TracingPolicy -metadata: - name: "kill-syscalls" -spec: - lists: - - name: "mine1" - type: "syscalls" - values: - - "sys_prctl" - - name: "mine2" - type: "syscalls" - values: - - "sys_prctl" - killers: - - syscalls: - - "list:mine1" - - syscalls: - - "list:mine2" - tracepoints: - - subsystem: "raw_syscalls" - event: "sys_enter" - args: - - index: 4 - type: "syscall64" - selectors: - - matchArgs: - - index: 0 - operator: "InMap" - values: - - "list:mine1" - matchActions: - - action: "NotifyKiller" - argSig: 9 # SIGKILL -` - - err := checkCrd(t, crd) +func TestKillerMultiNotSupported(t *testing.T) { + yaml := NewKillerSpecBuilder("killer-multi"). + WithSyscallList("sys_prctl"). + WithSyscallList("sys_dup"). + WithOverrideValue(-17). // EEXIST + MustYAML() + err := checkCrd(t, yaml) assert.Error(t, err) } From dd6bc5c2162827117d5971c55dfbaa3f1c1634e1 Mon Sep 17 00:00:00 2001 From: Kornilios Kourtis Date: Wed, 10 Jan 2024 10:03:39 +0100 Subject: [PATCH 09/15] killer_test: test more combinations This patch uses the override option in the spec (introduced in a previous patch) to test more combinations. Specifically, it tests fmod_ret even when bpf_override_return() is available, and single kprobe when multi-kprobe is available. Signed-off-by: Kornilios Kourtis --- pkg/sensors/tracing/killer_test.go | 72 +++++++++++++++++++++++------- 1 file changed, 56 insertions(+), 16 deletions(-) diff --git a/pkg/sensors/tracing/killer_test.go b/pkg/sensors/tracing/killer_test.go index a51783969ce..eb0c43842ef 100644 --- a/pkg/sensors/tracing/killer_test.go +++ b/pkg/sensors/tracing/killer_test.go @@ -63,19 +63,17 @@ func testKiller(t *testing.T, configHook string, } func TestKillerOverride(t *testing.T) { - if !bpf.HasOverrideHelper() && !bpf.HasModifyReturn() { - t.Skip("skipping killer test, bpf_override_return helper not available") - } if !bpf.HasSignalHelper() { t.Skip("skipping killer test, bpf_send_signal helper not available") } test := testutils.RepoRootPath("contrib/tester-progs/killer-tester") - yaml := NewKillerSpecBuilder("killer-override"). - WithSyscallList("sys_prctl"). - WithMatchBinaries(test). - WithOverrideValue(-17). // EEXIST - MustYAML() + builder := func() *KillerSpecBuilder { + return NewKillerSpecBuilder("killer-override"). + WithSyscallList("sys_prctl"). + WithMatchBinaries(test). + WithOverrideValue(-17) // EEXIST + } tpChecker := ec.NewProcessTracepointChecker(""). WithArgs(ec.NewKprobeArgumentListMatcher(). @@ -93,7 +91,31 @@ func TestKillerOverride(t *testing.T) { } } - testKiller(t, yaml, test, "", checker, checkerFunc) + t.Run("override_helper", func(t *testing.T) { + if !bpf.HasOverrideHelper() { + t.Skip("override_helper not supported") + } + + t.Run("multi kprobe", func(t *testing.T) { + if !bpf.HasKprobeMulti() { + t.Skip("no multi-kprobe support") + } + yaml := builder().WithOverrideReturn().WithMultiKprobe().MustYAML() + testKiller(t, yaml, test, "", checker, checkerFunc) + }) + + t.Run("kprobe (no multi)", func(t *testing.T) { + yaml := builder().WithOverrideReturn().WithoutMultiKprobe().MustYAML() + testKiller(t, yaml, test, "", checker, checkerFunc) + }) + }) + t.Run("fmod_ret", func(t *testing.T) { + if !bpf.HasModifyReturn() { + t.Skip("fmod_ret not supported") + } + yaml := builder().WithFmodRet().MustYAML() + testKiller(t, yaml, test, "", checker, checkerFunc) + }) } func TestKillerSignal(t *testing.T) { @@ -105,12 +127,6 @@ func TestKillerSignal(t *testing.T) { } test := testutils.RepoRootPath("contrib/tester-progs/killer-tester") - yaml := NewKillerSpecBuilder("killer-signal"). - WithSyscallList("sys_prctl"). - WithMatchBinaries(test). - WithOverrideValue(-17). // EEXIST - WithKill(9). // SigKill - MustYAML() tpChecker := ec.NewProcessTracepointChecker(""). WithArgs(ec.NewKprobeArgumentListMatcher(). @@ -128,7 +144,31 @@ func TestKillerSignal(t *testing.T) { } } - testKiller(t, yaml, test, "", checker, checkerFunc) + builder := func() *KillerSpecBuilder { + return NewKillerSpecBuilder("killer-signal"). + WithSyscallList("sys_prctl"). + WithMatchBinaries(test). + WithOverrideValue(-17). // EEXIST + WithKill(9) // SigKill + } + + t.Run("multi kprobe", func(t *testing.T) { + if !bpf.HasKprobeMulti() { + t.Skip("no multi-kprobe support") + } + if !bpf.HasOverrideHelper() { + t.Skip("no override helper, so cannot use multi kprobes") + } + + yaml := builder().WithMultiKprobe().MustYAML() + testKiller(t, yaml, test, "", checker, checkerFunc) + }) + + t.Run("kprobe (no multi)", func(t *testing.T) { + yaml := builder().WithoutMultiKprobe().MustYAML() + testKiller(t, yaml, test, "", checker, checkerFunc) + }) + } func TestKillerMultiNotSupported(t *testing.T) { From f2b130e19273f753dc939070a88807c0e99e9fa2 Mon Sep 17 00:00:00 2001 From: Kornilios Kourtis Date: Wed, 10 Jan 2024 11:45:01 +0100 Subject: [PATCH 10/15] bpf: fix verification failure On the latest 5.4, I'm seeing the following failure: ``` === RUN TestKillerOverride/override_helper/kprobe_(no_multi) ... load program: invalid argument: ; generic_tracepoint_filter(void *ctx) 0: (bf) r6 = r1 1: (b7) r1 = 0 ; int ret, zero = 0; 2: (63) *(u32 *)(r10 -28) = r1 last_idx 2 first_idx 0 regs=2 stack=0 before 1: (b7) r1 = 0 3: (bf) r2 = r10 ; 4: (07) r2 += -28 ; msg = map_lookup_elem(&tp_heap, &zero); 5: (18) r1 = 0xffff954178190400 7: (85) call bpf_map_lookup_elem#1 8: (bf) r7 = r0 ; if (!msg) 9: (15) if r7 == 0x0 goto pc+1989 R0_w=map_value(id=0,off=0,ks=4,vs=24304,imm=0) R6_w=ctx(id=0,off=0,imm=0) R7_w=map_value(id=0,off=0,ks=4,vs=24304,imm=0) R10=fp0 fp-32=mmmm???? ; &msg->caps, &filter_map, msg->idx); 10: (61) r1 = *(u32 *)(r7 +24288) R0_w=map_value(id=0,off=0,ks=4,vs=24304,imm=0) R6_w=ctx(id=0,off=0,imm=0) R7_w=map_value(id=0,off=0,ks=4,vs=24304,imm=0) R10=fp0 fp-32=mmmm???? 11: (63) *(u32 *)(r10 -24) = r1 ; struct task_struct *task = (struct task_struct *)get_current_task(); 12: (85) call bpf_get_current_task#35 13: (bf) r9 = r0 ; struct task_struct *task = (struct task_struct *)get_current_task(); 14: (7b) *(u64 *)(r10 -16) = r9 ; __u32 pid = get_current_pid_tgid() >> 32; 15: (85) call bpf_get_current_pid_tgid#14 ; __u32 pid = get_current_pid_tgid() >> 32; ... regs=4 stack=0 before 1895: (4f) r2 |= r6 regs=44 stack=0 before 1894: (4f) r2 |= r1 regs=46 stack=0 before 1893: (67) r2 <<= 24 regs=46 stack=0 before 1892: (79) r2 = *(u64 *)(r10 -40) regs=42 stack=10 before 1891: (67) r1 <<= 16 regs=42 stack=10 before 1890: (79) r1 = *(u64 *)(r10 -48) regs=40 stack=30 before 1889: (4f) r6 |= r1 regs=42 stack=30 before 1888: (79) r1 = *(u64 *)(r10 -56) regs=40 stack=70 before 1887: (67) r6 <<= 8 regs=40 stack=70 before 1886: (4f) r9 |= r7 regs=40 stack=70 before 1885: (4f) r9 |= r8 regs=40 stack=70 before 1884: (67) r9 <<= 24 regs=40 stack=70 before 1883: (67) r8 <<= 16 regs=40 stack=70 before 1882: (4f) r7 |= r1 regs=40 stack=70 before 1881: (79) r1 = *(u64 *)(r10 -64) regs=40 stack=70 before 1880: (67) r7 <<= 8 regs=40 stack=70 before 1879: (15) if r0 == 0x0 goto pc+95 regs=40 stack=70 before 1878: (85) call bpf_map_lookup_elem#1 regs=40 stack=70 before 1876: (18) r1 = 0xffff954177317000 regs=40 stack=70 before 1875: (07) r2 += -16 regs=40 stack=70 before 1874: (bf) r2 = r10 regs=40 stack=70 before 1873: (63) *(u32 *)(r10 -16) = r0 regs=40 stack=70 before 1872: (77) r0 >>= 32 R0_rw=inv(id=0) R6_rw=invP(id=0,umax_value=255,var_off=(0x0; 0xff)) R7_rw=inv(id=0,umax_value=255,var_off=(0x0; 0xff)) R8_rw=inv(id=0,umax_value=255,var_off=(0x0; 0xff)) R9_rw=inv(id=0,umax_value=255,var_off=(0x0; 0xff)) R10=fp0 fp-8=mmmm???? fp-16_r=mmmmmmmm fp-24=mmmmmmmm fp-32=mmmm???? fp-40_rw=mmmmmmmm fp-48_rw=mmmmmmmm fp-56_rw=mmmmmmmm fp-64_rw=mmmmmmmm fp-72_rw=mmmmmmmm fp-80_rw=mmmmmmmm fp-88_rw=mmmmmmmm fp-96_r=map_value fp-104=map_value fp-112=ctx fp-120=mmmmmmmm fp-128=mmmmmmmm fp-136_r=map_value fp-144_rw=mmmmmmmm fp-152_rw=mmmmmmmm fp-160_rw=mmmmmmmm fp-168_rw=mmmmmmmm fp-176_rw=mmmmmmmm fp-184_rw=mmmmmmmm fp-192_rw=mmmmmmmm fp-200_rw=mmmmmmmm fp-208_rw=mmmmmmmm parent didn't have regs=40 stack=0 marks last_idx 1871 first_idx 1821 regs=40 stack=0 before 1871: (85) call bpf_get_current_pid_tgid#14 regs=40 stack=0 before 1870: (71) r6 = *(u8 *)(r1 +1) math between map_value pointer and register with unbounded min value is not allowed processed 3841 insns (limit 1000000) max_states_per_insn 4 total_states 173 peak_states 170 mark_read 136 logcapture.go:25: time="2024-01-10T10:30:34Z" level=info msg="Unloading sensor __killer__" logcapture.go:25: time="2024-01-10T10:30:34Z" level=info msg="BPF prog was unloaded" label=kprobe/killer pin=killer-sensor-1-killer_kprobe logcapture.go:25: time="2024-01-10T10:30:34Z" level=info msg="map was unloaded" map=killer_data pin=killer_data logcapture.go:25: time="2024-01-10T10:30:34Z" level=info msg="map was unloaded" map=killer_data pin=killer_data logcapture.go:25: time="2024-01-10T10:30:34Z" level=info msg="Cleaning up killer" observer_test_helper.go:443: SensorManager.AddTracingPolicy error: sensor gtp-sensor-2 from collection killer-override failed to load: failed prog /home/kkourt/src/tetragon/bpf/objs/bpf_generic_tracepoint_v53.o kern_version 328959 loadInstance: opening collection '/home/kkourt/src/tetragon/bpf/objs/bpf_generic_tracepoint_v53.o' failed: program generic_tracepoint_filter: load program: invalid argument: math between map_value pointer and register with unbounded min value is not allowed (18925 line(s) omitted) filenames.go:47: keeping export file for TestKillerOverride-override_helper-kprobe_(no_multi) (/tmp/tetragon.gotest.TestKillerOverride-override_helper-kprobe_(no_multi).4012676103.json) ``` It's not clear to me what the issue exatly is, but reading the code I found an opportunity to simplify it which seems to fix the issue. Signed-off-by: Kornilios Kourtis --- bpf/lib/bpf_task.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/bpf/lib/bpf_task.h b/bpf/lib/bpf_task.h index f63c2e00440..f4e2e9bc13a 100644 --- a/bpf/lib/bpf_task.h +++ b/bpf/lib/bpf_task.h @@ -158,12 +158,13 @@ static inline __attribute__((always_inline)) struct execve_map_value * event_find_curr(__u32 *ppid, bool *walked) { struct task_struct *task = (struct task_struct *)get_current_task(); - __u32 pid = get_current_pid_tgid() >> 32; struct execve_map_value *value = 0; int i; + __u32 pid; #pragma unroll for (i = 0; i < 4; i++) { + probe_read(&pid, sizeof(pid), _(&task->tgid)); value = execve_map_get_noinit(pid); if (value && value->key.ktime != 0) break; @@ -172,7 +173,6 @@ event_find_curr(__u32 *ppid, bool *walked) probe_read(&task, sizeof(task), _(&task->real_parent)); if (!task) break; - probe_read(&pid, sizeof(pid), _(&task->tgid)); } *ppid = pid; return value; From 8e965f76a18396774a21c4570d6493d26089f182 Mon Sep 17 00:00:00 2001 From: Kornilios Kourtis Date: Thu, 11 Jan 2024 13:17:38 +0100 Subject: [PATCH 11/15] killer: move global state to a single struct This is a preparation patch for subsequent patches. This is just reorginization, there are no functional changes. Signed-off-by: Kornilios Kourtis --- pkg/sensors/tracing/killer.go | 66 ++++++++++++++++++++--------------- 1 file changed, 37 insertions(+), 29 deletions(-) diff --git a/pkg/sensors/tracing/killer.go b/pkg/sensors/tracing/killer.go index 6440564c35a..923a53a28ed 100644 --- a/pkg/sensors/tracing/killer.go +++ b/pkg/sensors/tracing/killer.go @@ -20,20 +20,28 @@ import ( "github.com/cilium/tetragon/pkg/tracingpolicy" ) -type killerSensor struct{} +type killerHandler struct { + configured bool + syscallsSyms []string +} -func init() { - killer := &killerSensor{} - sensors.RegisterProbeType("killer", killer) - sensors.RegisterPolicyHandlerAtInit("killer", killerSensor{}) +func newKillerHandler() *killerHandler { + return &killerHandler{ + configured: false, + } } var ( - configured = false - syscallsSyms []string + // global killer handler + gKillerHandler = newKillerHandler() ) -func (k killerSensor) PolicyHandler( +func init() { + sensors.RegisterProbeType("killer", gKillerHandler) + sensors.RegisterPolicyHandlerAtInit("killer", gKillerHandler) +} + +func (kh *killerHandler) PolicyHandler( policy tracingpolicy.TracingPolicy, _ policyfilter.PolicyID, ) (*sensors.Sensor, error) { @@ -48,26 +56,27 @@ func (k killerSensor) PolicyHandler( } if len(spec.Killers) > 0 { name := fmt.Sprintf("killer-sensor-%d", atomic.AddUint64(&sensorCounter, 1)) - return createKillerSensor(spec.Killers, spec.Lists, spec.Options, name) + return kh.createKillerSensor(spec.Killers, spec.Lists, spec.Options, name) } return nil, nil } -func loadSingleKillerSensor(bpfDir, mapDir string, load *program.Program, verbose int) error { - if err := program.LoadKprobeProgramAttachMany(bpfDir, mapDir, load, syscallsSyms, verbose); err == nil { +func (kh *killerHandler) loadSingleKillerSensor( + bpfDir, mapDir string, load *program.Program, verbose int, +) error { + if err := program.LoadKprobeProgramAttachMany(bpfDir, mapDir, load, kh.syscallsSyms, verbose); err == nil { logger.GetLogger().Infof("Loaded killer sensor: %s", load.Attach) } else { return err } - return nil } -func loadMultiKillerSensor(bpfDir, mapDir string, load *program.Program, verbose int) error { +func (kh *killerHandler) loadMultiKillerSensor(bpfDir, mapDir string, load *program.Program, verbose int) error { data := &program.MultiKprobeAttachData{} - data.Symbols = append(data.Symbols, syscallsSyms...) + data.Symbols = append(data.Symbols, kh.syscallsSyms...) load.SetAttachData(data) @@ -79,12 +88,12 @@ func loadMultiKillerSensor(bpfDir, mapDir string, load *program.Program, verbose return nil } -func (k *killerSensor) LoadProbe(args sensors.LoadProbeArgs) error { +func (kh *killerHandler) LoadProbe(args sensors.LoadProbeArgs) error { if args.Load.Label == "kprobe.multi/killer" { - return loadMultiKillerSensor(args.BPFDir, args.MapDir, args.Load, args.Verbose) + return kh.loadMultiKillerSensor(args.BPFDir, args.MapDir, args.Load, args.Verbose) } if args.Load.Label == "kprobe/killer" { - return loadSingleKillerSensor(args.BPFDir, args.MapDir, args.Load, args.Verbose) + return kh.loadSingleKillerSensor(args.BPFDir, args.MapDir, args.Load, args.Verbose) } if strings.HasPrefix(args.Load.Label, "fmod_ret/") { @@ -120,14 +129,14 @@ func selectOverrideMethod(specOpts *specOptions) (OverrideMethod, error) { return overrideMethod, nil } -func unloadKiller() error { - configured = false - syscallsSyms = []string{} +func (kh *killerHandler) unload() error { + kh.configured = false + kh.syscallsSyms = []string{} logger.GetLogger().Infof("Cleaning up killer") return nil } -func createKillerSensor( +func (kh *killerHandler) createKillerSensor( killers []v1alpha1.KillerSpec, lists []v1alpha1.ListSpec, opts []v1alpha1.OptionSpec, @@ -138,11 +147,10 @@ func createKillerSensor( return nil, fmt.Errorf("failed: we support only single killer sensor") } - if configured { + if kh.configured { return nil, fmt.Errorf("failed: killer sensor is already configured") } - - configured = true + kh.configured = true killer := killers[0] @@ -160,7 +168,7 @@ func createKillerSensor( if !isSyscallListType(list.Type) { return nil, fmt.Errorf("Error list '%s' is not syscall type", listName) } - syscallsSyms = append(syscallsSyms, list.Values...) + kh.syscallsSyms = append(kh.syscallsSyms, list.Values...) continue } @@ -168,7 +176,7 @@ func createKillerSensor( if err != nil { return nil, err } - syscallsSyms = append(syscallsSyms, pfxSym) + kh.syscallsSyms = append(kh.syscallsSyms, pfxSym) } // register killer sensor @@ -201,7 +209,7 @@ func createKillerSensor( label = "kprobe.multi/killer" prog = "bpf_multi_killer.o" } - attach := fmt.Sprintf("%d syscalls: %s", len(syscallsSyms), syscallsSyms) + attach := fmt.Sprintf("%d syscalls: %s", len(kh.syscallsSyms), kh.syscallsSyms) load = program.Builder( path.Join(option.Config.HubbleLib, prog), attach, @@ -212,7 +220,7 @@ func createKillerSensor( case OverrideMethodFmodRet: // for fmod_ret, we need one program per syscall logger.GetLogger().Infof("killer: using fmod_ret") - for _, syscallSym := range syscallsSyms { + for _, syscallSym := range kh.syscallsSyms { load = program.Builder( path.Join(option.Config.HubbleLib, "bpf_fmodret_killer.o"), syscallSym, @@ -232,6 +240,6 @@ func createKillerSensor( Name: "__killer__", Progs: progs, Maps: maps, - PostUnloadHook: unloadKiller, + PostUnloadHook: gKillerHandler.unload, }, nil } From 12bfc3e48cae99574806ddb63ac14615310f3fbb Mon Sep 17 00:00:00 2001 From: Kornilios Kourtis Date: Fri, 12 Jan 2024 10:19:22 +0100 Subject: [PATCH 12/15] killer: use const for map name Signed-off-by: Kornilios Kourtis --- pkg/sensors/tracing/killer.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/pkg/sensors/tracing/killer.go b/pkg/sensors/tracing/killer.go index 923a53a28ed..678b6dd97ab 100644 --- a/pkg/sensors/tracing/killer.go +++ b/pkg/sensors/tracing/killer.go @@ -20,6 +20,10 @@ import ( "github.com/cilium/tetragon/pkg/tracingpolicy" ) +const ( + killerDataMapName = "killer_data" +) + type killerHandler struct { configured bool syscallsSyms []string @@ -233,7 +237,7 @@ func (kh *killerHandler) createKillerSensor( return nil, fmt.Errorf("unexpected override method: %d", overrideMethod) } - killerDataMap := program.MapBuilderPin("killer_data", "killer_data", load) + killerDataMap := program.MapBuilderPin(killerDataMapName, killerDataMapName, load) maps = append(maps, killerDataMap) return &sensors.Sensor{ From 542aad0af16f926b3f01c05e1dc437206b4a4903 Mon Sep 17 00:00:00 2001 From: Kornilios Kourtis Date: Tue, 16 Jan 2024 14:46:20 +0100 Subject: [PATCH 13/15] contrib/tester-progs: add getcpu prog Add a getcpu program that just performs the getcpu syscall and exits with the error value. Will be used in the next patch. Signed-off-by: Kornilios Kourtis --- contrib/tester-progs/.gitignore | 1 + contrib/tester-progs/Makefile | 6 +++++- contrib/tester-progs/go/getcpu/getcpu.go | 22 ++++++++++++++++++++++ 3 files changed, 28 insertions(+), 1 deletion(-) create mode 100644 contrib/tester-progs/go/getcpu/getcpu.go diff --git a/contrib/tester-progs/.gitignore b/contrib/tester-progs/.gitignore index 28d1a691c78..fe0e46e31bc 100644 --- a/contrib/tester-progs/.gitignore +++ b/contrib/tester-progs/.gitignore @@ -18,3 +18,4 @@ bench-reader threads-exit killer-tester killer-tester-32 +/getcpu diff --git a/contrib/tester-progs/Makefile b/contrib/tester-progs/Makefile index 9009cb64197..c906352314e 100644 --- a/contrib/tester-progs/Makefile +++ b/contrib/tester-progs/Makefile @@ -19,7 +19,8 @@ PROGS = sigkill-tester \ bench-reader \ threads-exit \ killer-tester \ - drop-privileges + drop-privileges \ + getcpu # For now killer-tester is compiled to 32-bit only on x86_64 as we want # to test 32-bit binaries and system calls compatibility layer. @@ -79,6 +80,9 @@ killer-tester-32: killer-tester.c lseek-pipe: FORCE go build -o lseek-pipe ./go/lseek-pipe +getcpu: FORCE + go build -o getcpu ./go/getcpu + .PHONY: clean clean: rm -f $(PROGS) diff --git a/contrib/tester-progs/go/getcpu/getcpu.go b/contrib/tester-progs/go/getcpu/getcpu.go new file mode 100644 index 00000000000..40176cfdba1 --- /dev/null +++ b/contrib/tester-progs/go/getcpu/getcpu.go @@ -0,0 +1,22 @@ +// SPDX-License-Identifier: Apache-2.0 +// Copyright Authors of Tetragon + +package main + +import ( + "os" + "unsafe" + + "golang.org/x/sys/unix" +) + +func main() { + var cpu, node int + _, _, err := unix.Syscall( + unix.SYS_GETCPU, + uintptr(unsafe.Pointer(&cpu)), + uintptr(unsafe.Pointer(&node)), + 0, + ) + os.Exit(int(err)) +} From 2f66501a4b6dd3c04269634161d1bb7fc4fad798 Mon Sep 17 00:00:00 2001 From: Kornilios Kourtis Date: Tue, 16 Jan 2024 14:47:06 +0100 Subject: [PATCH 14/15] killer_test: use a different syscall than prctl In one of the previous iterations of this PR, the patch that implemented the fmod_ret changes had an bug. The bug, instead of hooking the program to the syscall, hooked the program to security_task_prctl, which is the default label we use in the ELF object. As a result, the test was never caught by our testing. This patch, updates the override test to the getcpu syscall to ensure that the test works as expected. Signed-off-by: Kornilios Kourtis --- pkg/sensors/tracing/killer_test.go | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/pkg/sensors/tracing/killer_test.go b/pkg/sensors/tracing/killer_test.go index eb0c43842ef..fb6cb519187 100644 --- a/pkg/sensors/tracing/killer_test.go +++ b/pkg/sensors/tracing/killer_test.go @@ -21,6 +21,7 @@ import ( "github.com/cilium/tetragon/pkg/testutils" tus "github.com/cilium/tetragon/pkg/testutils/sensors" "github.com/stretchr/testify/assert" + "golang.org/x/sys/unix" ) func testKiller(t *testing.T, configHook string, @@ -67,10 +68,10 @@ func TestKillerOverride(t *testing.T) { t.Skip("skipping killer test, bpf_send_signal helper not available") } - test := testutils.RepoRootPath("contrib/tester-progs/killer-tester") + test := testutils.RepoRootPath("contrib/tester-progs/getcpu") builder := func() *KillerSpecBuilder { return NewKillerSpecBuilder("killer-override"). - WithSyscallList("sys_prctl"). + WithSyscallList("sys_getcpu"). WithMatchBinaries(test). WithOverrideValue(-17) // EEXIST } @@ -79,7 +80,7 @@ func TestKillerOverride(t *testing.T) { WithArgs(ec.NewKprobeArgumentListMatcher(). WithOperator(lc.Ordered). WithValues( - ec.NewKprobeArgumentChecker().WithSizeArg(syscall.SYS_PRCTL), + ec.NewKprobeArgumentChecker().WithSizeArg(unix.SYS_GETCPU), )). WithAction(tetragon.KprobeAction_KPROBE_ACTION_NOTIFYKILLER) From 8706ff3c707e757647d6ef099264c1b8d7c51632 Mon Sep 17 00:00:00 2001 From: Kornilios Kourtis Date: Mon, 15 Jan 2024 11:16:48 +0100 Subject: [PATCH 15/15] killer: add and use HasModifyReturnSyscall check Our detection code already has a function for detecting of fmod_ret is supported named HasModifyReturn. If we want to load an fmod_ret program in a syscall function, however, checking HasModifyReturn() is not sufficient. This is because in kernels where CONFIG_FUNCTION_ERROR_INJECTION is not set, loading fmod_ret programs in syscalls is not supported. So add a separate check, where we try to attach to a system call (we use getcpu) instead of a security_ function and use it for feature detection in the killer sensor. Kernel check is: ``` static int check_attach_modify_return(unsigned long addr, const char *func_name) { if (within_error_injection_list(addr) || !strncmp(SECURITY_PREFIX, func_name, sizeof(SECURITY_PREFIX) - 1)) return 0; return -EINVAL; } ``` And: ``` ... static inline bool within_error_injection_list(unsigned long addr) { return false; } static inline int get_injectable_error_type(unsigned long addr) { return -EOPNOTSUPP; } ``` Signed-off-by: Kornilios Kourtis --- pkg/bpf/detect.go | 54 +++++++++++++++++++++--- pkg/sensors/tracing/killer.go | 4 +- pkg/sensors/tracing/killer_amd64_test.go | 22 ++-------- pkg/sensors/tracing/killer_test.go | 20 +++++---- 4 files changed, 65 insertions(+), 35 deletions(-) diff --git a/pkg/bpf/detect.go b/pkg/bpf/detect.go index e728f78c273..ac168c33638 100644 --- a/pkg/bpf/detect.go +++ b/pkg/bpf/detect.go @@ -15,6 +15,8 @@ import ( "github.com/cilium/ebpf/asm" "github.com/cilium/ebpf/features" "github.com/cilium/ebpf/link" + "github.com/cilium/tetragon/pkg/arch" + "github.com/cilium/tetragon/pkg/logger" "golang.org/x/sys/unix" ) @@ -24,9 +26,10 @@ type Feature struct { } var ( - kprobeMulti Feature - buildid Feature - modifyReturn Feature + kprobeMulti Feature + buildid Feature + modifyReturn Feature + modifyReturnSyscall Feature ) func HasOverrideHelper() bool { @@ -119,6 +122,40 @@ func detectModifyReturn() bool { return true } +func detectModifyReturnSyscall() bool { + sysGetcpu, err := arch.AddSyscallPrefix("sys_getcpu") + if err != nil { + return false + } + logger.GetLogger().Infof("probing detectModifyReturnSyscall using %s", sysGetcpu) + prog, err := ebpf.NewProgram(&ebpf.ProgramSpec{ + Name: "probe_sys_fmod_ret", + Type: ebpf.Tracing, + Instructions: asm.Instructions{ + asm.Mov.Imm(asm.R0, 0), + asm.Return(), + }, + AttachType: ebpf.AttachModifyReturn, + AttachTo: sysGetcpu, + License: "MIT", + }) + if err != nil { + logger.GetLogger().WithError(err).Info("detectModifyReturnSyscall: failed to load") + return false + } + defer prog.Close() + + link, err := link.AttachTracing(link.TracingOptions{ + Program: prog, + }) + if err != nil { + logger.GetLogger().WithError(err).Info("detectModifyReturnSyscall, failed to attach") + return false + } + link.Close() + return true +} + func HasModifyReturn() bool { modifyReturn.init.Do(func() { modifyReturn.detected = detectModifyReturn() @@ -126,11 +163,18 @@ func HasModifyReturn() bool { return modifyReturn.detected } +func HasModifyReturnSyscall() bool { + modifyReturnSyscall.init.Do(func() { + modifyReturnSyscall.detected = detectModifyReturnSyscall() + }) + return modifyReturnSyscall.detected +} + func HasProgramLargeSize() bool { return features.HaveLargeInstructions() == nil } func LogFeatures() string { - return fmt.Sprintf("override_return: %t, buildid: %t, kprobe_multi: %t, fmodret: %t, signal: %t, large: %t", - HasOverrideHelper(), HasBuildId(), HasKprobeMulti(), HasModifyReturn(), HasSignalHelper(), HasProgramLargeSize()) + return fmt.Sprintf("override_return: %t, buildid: %t, kprobe_multi: %t, fmodret: %t, fmodret_syscall: %t, signal: %t, large: %t", + HasOverrideHelper(), HasBuildId(), HasKprobeMulti(), HasModifyReturn(), HasModifyReturnSyscall(), HasSignalHelper(), HasProgramLargeSize()) } diff --git a/pkg/sensors/tracing/killer.go b/pkg/sensors/tracing/killer.go index 678b6dd97ab..108dbd2894a 100644 --- a/pkg/sensors/tracing/killer.go +++ b/pkg/sensors/tracing/killer.go @@ -115,7 +115,7 @@ func selectOverrideMethod(specOpts *specOptions) (OverrideMethod, error) { // by default, first try OverrideReturn and if this does not work try fmod_ret if bpf.HasOverrideHelper() { overrideMethod = OverrideMethodReturn - } else if bpf.HasModifyReturn() { + } else if bpf.HasModifyReturnSyscall() { overrideMethod = OverrideMethodFmodRet } else { return OverrideMethodInvalid, fmt.Errorf("no override helper or mod_ret support: cannot load killer") @@ -125,7 +125,7 @@ func selectOverrideMethod(specOpts *specOptions) (OverrideMethod, error) { return OverrideMethodInvalid, fmt.Errorf("option override return set, but it is not supported") } case OverrideMethodFmodRet: - if !bpf.HasModifyReturn() { + if !bpf.HasModifyReturnSyscall() { return OverrideMethodInvalid, fmt.Errorf("option fmod_ret set, but it is not supported") } } diff --git a/pkg/sensors/tracing/killer_amd64_test.go b/pkg/sensors/tracing/killer_amd64_test.go index 22b3e396020..ff47bb0cabd 100644 --- a/pkg/sensors/tracing/killer_amd64_test.go +++ b/pkg/sensors/tracing/killer_amd64_test.go @@ -11,7 +11,6 @@ import ( "testing" "github.com/cilium/tetragon/api/v1/tetragon" - "github.com/cilium/tetragon/pkg/bpf" "github.com/cilium/tetragon/pkg/syscallinfo/i386" "github.com/cilium/tetragon/pkg/testutils" @@ -20,12 +19,7 @@ import ( ) func TestKillerOverride32(t *testing.T) { - if !bpf.HasOverrideHelper() && !bpf.HasModifyReturn() { - t.Skip("skipping killer test, neither bpf_override_return nor fmod_ret is available") - } - if !bpf.HasSignalHelper() { - t.Skip("skipping killer test, bpf_send_signal helper not available") - } + testKillerCheckSkip(t) test := testutils.RepoRootPath("contrib/tester-progs/killer-tester-32") yaml := NewKillerSpecBuilder("killer-override"). @@ -54,12 +48,7 @@ func TestKillerOverride32(t *testing.T) { } func TestKillerSignal32(t *testing.T) { - if !bpf.HasOverrideHelper() && !bpf.HasModifyReturn() { - t.Skip("skipping killer test, bpf_override_return helper not available") - } - if !bpf.HasSignalHelper() { - t.Skip("skipping killer test, bpf_send_signal helper not available") - } + testKillerCheckSkip(t) test := testutils.RepoRootPath("contrib/tester-progs/killer-tester-32") yaml := NewKillerSpecBuilder("killer-signal"). @@ -89,12 +78,7 @@ func TestKillerSignal32(t *testing.T) { } func TestKillerOverrideBothBits(t *testing.T) { - if !bpf.HasOverrideHelper() && !bpf.HasModifyReturn() { - t.Skip("skipping killer test, bpf_override_return helper not available") - } - if !bpf.HasSignalHelper() { - t.Skip("skipping killer test, bpf_send_signal helper not available") - } + testKillerCheckSkip(t) test32 := testutils.RepoRootPath("contrib/tester-progs/killer-tester-32") test64 := testutils.RepoRootPath("contrib/tester-progs/killer-tester") diff --git a/pkg/sensors/tracing/killer_test.go b/pkg/sensors/tracing/killer_test.go index fb6cb519187..920c14efbcc 100644 --- a/pkg/sensors/tracing/killer_test.go +++ b/pkg/sensors/tracing/killer_test.go @@ -24,6 +24,15 @@ import ( "golang.org/x/sys/unix" ) +func testKillerCheckSkip(t *testing.T) { + if !bpf.HasSignalHelper() { + t.Skip("skipping killer test, bpf_send_signal helper not available") + } + if !bpf.HasOverrideHelper() && !bpf.HasModifyReturnSyscall() { + t.Skip("skipping test, neither bpf_override_return nor fmod_ret for syscalls is available") + } +} + func testKiller(t *testing.T, configHook string, test string, test2 string, checker *eventchecker.UnorderedEventChecker, @@ -64,9 +73,7 @@ func testKiller(t *testing.T, configHook string, } func TestKillerOverride(t *testing.T) { - if !bpf.HasSignalHelper() { - t.Skip("skipping killer test, bpf_send_signal helper not available") - } + testKillerCheckSkip(t) test := testutils.RepoRootPath("contrib/tester-progs/getcpu") builder := func() *KillerSpecBuilder { @@ -120,12 +127,7 @@ func TestKillerOverride(t *testing.T) { } func TestKillerSignal(t *testing.T) { - if !bpf.HasOverrideHelper() && !bpf.HasModifyReturn() { - t.Skip("skipping killer test, neither bpf_override_return nor fmod_ret is available") - } - if !bpf.HasSignalHelper() { - t.Skip("skipping killer test, bpf_send_signal helper not available") - } + testKillerCheckSkip(t) test := testutils.RepoRootPath("contrib/tester-progs/killer-tester")