From 38bedec8289342af17be59bc5f6c2b38f2992292 Mon Sep 17 00:00:00 2001 From: Kornilios Kourtis Date: Mon, 15 Jan 2024 11:16:48 +0100 Subject: [PATCH] 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")