From a0e810a9f8bcf47095ccc2593405c73d0d381eeb Mon Sep 17 00:00:00 2001 From: Jiri Olsa Date: Fri, 20 Oct 2023 07:13:33 +0000 Subject: [PATCH 1/6] tetragon: Remove GenericTracepointConf type Removing GenericTracepointConf type as a cleanup, because that abstraction is not actually needed anywhere. Signed-off-by: Jiri Olsa --- pkg/sensors/tracing/generictracepoint.go | 8 ++------ pkg/sensors/tracing/policyfilter_test.go | 2 +- pkg/sensors/tracing/tracepoint_test.go | 20 ++++++++++---------- 3 files changed, 13 insertions(+), 17 deletions(-) diff --git a/pkg/sensors/tracing/generictracepoint.go b/pkg/sensors/tracing/generictracepoint.go index 22b148b0d68..824fffcdee7 100644 --- a/pkg/sensors/tracing/generictracepoint.go +++ b/pkg/sensors/tracing/generictracepoint.go @@ -148,10 +148,6 @@ func (t *tracepointTable) getTracepoint(idx int) (*genericTracepoint, error) { return nil, fmt.Errorf("tracepoint table: invalid id:%d (len=%d)", idx, len(t.arr)) } -// GenericTracepointConf is the configuration for a generic tracepoint. This is -// a caller-defined structure that configures a tracepoint. -type GenericTracepointConf = v1alpha1.TracepointSpec - // getTracepointMetaArg is a temporary helper to find meta values while tracepoint // converts into new CRD and config formats. func getTracepointMetaValue(arg *v1alpha1.KProbeArg) int { @@ -318,7 +314,7 @@ func buildGenericTracepointArgs(info *tracepoint.Tracepoint, specArgs []v1alpha1 // the user-provided configuration func createGenericTracepoint( sensorName string, - conf *GenericTracepointConf, + conf *v1alpha1.TracepointSpec, policyID policyfilter.PolicyID, policyName string, customHandler eventhandler.Handler, @@ -354,7 +350,7 @@ func createGenericTracepoint( // createGenericTracepointSensor will create a sensor that can be loaded based on a generic tracepoint configuration func createGenericTracepointSensor( name string, - confs []GenericTracepointConf, + confs []v1alpha1.TracepointSpec, policyID policyfilter.PolicyID, policyName string, lists []v1alpha1.ListSpec, diff --git a/pkg/sensors/tracing/policyfilter_test.go b/pkg/sensors/tracing/policyfilter_test.go index f9a8089c4e3..c8692eeb37f 100644 --- a/pkg/sensors/tracing/policyfilter_test.go +++ b/pkg/sensors/tracing/policyfilter_test.go @@ -182,7 +182,7 @@ func TestNamespacedPolicies(t *testing.T) { }, } - tpSpec := GenericTracepointConf{ + tpSpec := v1alpha1.TracepointSpec{ Subsystem: "syscalls", Event: "sys_enter_lseek", Args: []v1alpha1.KProbeArg{ diff --git a/pkg/sensors/tracing/tracepoint_test.go b/pkg/sensors/tracing/tracepoint_test.go index 239dd409c0e..2aead746780 100644 --- a/pkg/sensors/tracing/tracepoint_test.go +++ b/pkg/sensors/tracing/tracepoint_test.go @@ -60,7 +60,7 @@ func TestGenericTracepointSimple(t *testing.T) { ctx, cancel := context.WithTimeout(context.Background(), tus.Conf().CmdWaitTime) defer cancel() - lseekConf := GenericTracepointConf{ + lseekConf := v1alpha1.TracepointSpec{ Subsystem: "syscalls", Event: "sys_enter_lseek", Args: []v1alpha1.KProbeArg{ @@ -77,7 +77,7 @@ func TestGenericTracepointSimple(t *testing.T) { sm := tus.GetTestSensorManager(ctx, t) // create and add sensor - sensor, err := createGenericTracepointSensor("GtpLseekTest", []GenericTracepointConf{lseekConf}, policyfilter.NoFilterID, + sensor, err := createGenericTracepointSensor("GtpLseekTest", []v1alpha1.TracepointSpec{lseekConf}, policyfilter.NoFilterID, "policyName", []v1alpha1.ListSpec{}, nil) if err != nil { t.Fatalf("failed to create generic tracepoint sensor: %s", err) @@ -106,7 +106,7 @@ func TestGenericTracepointSimple(t *testing.T) { // doTestGenericTracepointPidFilter is a utility function for doing generic // tracepoint tests. It filters events based on the test program's pid, so that // we get more predictable results. -func doTestGenericTracepointPidFilter(t *testing.T, conf GenericTracepointConf, selfOp func(), checkFn func(*tetragon.ProcessTracepoint) error) { +func doTestGenericTracepointPidFilter(t *testing.T, conf v1alpha1.TracepointSpec, selfOp func(), checkFn func(*tetragon.ProcessTracepoint) error) { if _, err := os.Stat("/sys/kernel/debug/tracing/events/syscalls"); os.IsNotExist(err) { t.Skip("cannot use syscall tracepoints (consider enabling CONFIG_FTRACE_SYSCALLS)") } @@ -137,7 +137,7 @@ func doTestGenericTracepointPidFilter(t *testing.T, conf GenericTracepointConf, sm := tus.GetTestSensorManager(ctx, t) // create and add sensor - sensor, err := createGenericTracepointSensor("GtpLseekTest", []GenericTracepointConf{conf}, policyfilter.NoFilterID, + sensor, err := createGenericTracepointSensor("GtpLseekTest", []v1alpha1.TracepointSpec{conf}, policyfilter.NoFilterID, "policyName", []v1alpha1.ListSpec{}, nil) if err != nil { t.Fatalf("failed to create generic tracepoint sensor: %s", err) @@ -193,7 +193,7 @@ func doTestGenericTracepointPidFilter(t *testing.T, conf GenericTracepointConf, } func TestGenericTracepointPidFilterLseek(t *testing.T) { - tracepointConf := GenericTracepointConf{ + tracepointConf := v1alpha1.TracepointSpec{ Subsystem: "syscalls", Event: "sys_enter_lseek", } @@ -217,7 +217,7 @@ func TestGenericTracepointArgFilterLseek(t *testing.T) { whenceStr := fmt.Sprintf("%d", whenceBogusValue) whence := whenceBogusValue - tracepointConf := GenericTracepointConf{ + tracepointConf := v1alpha1.TracepointSpec{ Subsystem: "syscalls", Event: "sys_enter_lseek", Args: []v1alpha1.KProbeArg{ @@ -280,7 +280,7 @@ func TestGenericTracepointMeta(t *testing.T) { assert.NoError(t, err) defer func() { syscall.Unlink("/tmp/testificate") }() - tracepointConf := GenericTracepointConf{ + tracepointConf := v1alpha1.TracepointSpec{ Subsystem: "syscalls", Event: "sys_enter_write", Args: []v1alpha1.KProbeArg{ @@ -345,7 +345,7 @@ func TestGenericTracepointMeta(t *testing.T) { // // print fmt: "NR %ld (%lx, %lx, %lx, %lx, %lx, %lx)", REC->id, REC->args[0], REC->args[1], REC->args[2], REC->args[3], REC->args[4], REC->args[5] func TestGenericTracepointRawSyscall(t *testing.T) { - tracepointConf := GenericTracepointConf{ + tracepointConf := v1alpha1.TracepointSpec{ Subsystem: "raw_syscalls", Event: "sys_enter", Args: []v1alpha1.KProbeArg{ @@ -506,7 +506,7 @@ func TestTracepointCloneThreads(t *testing.T) { ctx, cancel := context.WithTimeout(context.Background(), tus.Conf().CmdWaitTime) defer cancel() - lseekConf := GenericTracepointConf{ + lseekConf := v1alpha1.TracepointSpec{ Subsystem: "syscalls", Event: "sys_enter_lseek", Args: []v1alpha1.KProbeArg{ @@ -533,7 +533,7 @@ func TestTracepointCloneThreads(t *testing.T) { sm := tus.GetTestSensorManager(ctx, t) // create and add sensor - sensor, err := createGenericTracepointSensor("GtpLseekTest", []GenericTracepointConf{lseekConf}, policyfilter.NoFilterID, + sensor, err := createGenericTracepointSensor("GtpLseekTest", []v1alpha1.TracepointSpec{lseekConf}, policyfilter.NoFilterID, "policyName", []v1alpha1.ListSpec{}, nil) if err != nil { t.Fatalf("failed to create generic tracepoint sensor: %s", err) From 16c13380c71540e7a326b26fbd2d0dd88534cfd7 Mon Sep 17 00:00:00 2001 From: Jiri Olsa Date: Tue, 12 Sep 2023 11:49:40 +0000 Subject: [PATCH 2/6] tetragon: Add options into spec Adding possibility to specify option for the set. It's an array of name/value pairs and it's meant to be processed and interpreted by each sensor. Signed-off-by: Jiri Olsa --- .../v1alpha1/cilium.io_tracingpolicies.yaml | 14 +++++++++++++ .../cilium.io_tracingpoliciesnamespaced.yaml | 14 +++++++++++++ .../v1alpha1/tracing_policy_types.go | 4 ++++ pkg/k8s/apis/cilium.io/v1alpha1/types.go | 8 +++++++ pkg/k8s/apis/cilium.io/v1alpha1/version.go | 2 +- .../v1alpha1/zz_generated.deepcopy.go | 21 +++++++++++++++++++ .../v1alpha1/cilium.io_tracingpolicies.yaml | 14 +++++++++++++ .../cilium.io_tracingpoliciesnamespaced.yaml | 14 +++++++++++++ .../v1alpha1/tracing_policy_types.go | 4 ++++ .../pkg/k8s/apis/cilium.io/v1alpha1/types.go | 8 +++++++ .../k8s/apis/cilium.io/v1alpha1/version.go | 2 +- .../v1alpha1/zz_generated.deepcopy.go | 21 +++++++++++++++++++ 12 files changed, 124 insertions(+), 2 deletions(-) diff --git a/pkg/k8s/apis/cilium.io/client/crds/v1alpha1/cilium.io_tracingpolicies.yaml b/pkg/k8s/apis/cilium.io/client/crds/v1alpha1/cilium.io_tracingpolicies.yaml index ec761c98e89..97b99fcd84a 100644 --- a/pkg/k8s/apis/cilium.io/client/crds/v1alpha1/cilium.io_tracingpolicies.yaml +++ b/pkg/k8s/apis/cilium.io/client/crds/v1alpha1/cilium.io_tracingpolicies.yaml @@ -610,6 +610,20 @@ spec: loader: description: Enable loader events type: boolean + options: + description: A list of overloaded options + items: + properties: + name: + description: Name of the option + type: string + value: + description: Value of the option + type: string + required: + - name + type: object + type: array podSelector: description: PodSelector selects pods that this policy applies to properties: diff --git a/pkg/k8s/apis/cilium.io/client/crds/v1alpha1/cilium.io_tracingpoliciesnamespaced.yaml b/pkg/k8s/apis/cilium.io/client/crds/v1alpha1/cilium.io_tracingpoliciesnamespaced.yaml index ce8c9faaba1..7a1c87ff27b 100644 --- a/pkg/k8s/apis/cilium.io/client/crds/v1alpha1/cilium.io_tracingpoliciesnamespaced.yaml +++ b/pkg/k8s/apis/cilium.io/client/crds/v1alpha1/cilium.io_tracingpoliciesnamespaced.yaml @@ -610,6 +610,20 @@ spec: loader: description: Enable loader events type: boolean + options: + description: A list of overloaded options + items: + properties: + name: + description: Name of the option + type: string + value: + description: Value of the option + type: string + required: + - name + type: object + type: array podSelector: description: PodSelector selects pods that this policy applies to properties: diff --git a/pkg/k8s/apis/cilium.io/v1alpha1/tracing_policy_types.go b/pkg/k8s/apis/cilium.io/v1alpha1/tracing_policy_types.go index 0423ae46318..935b68686d4 100644 --- a/pkg/k8s/apis/cilium.io/v1alpha1/tracing_policy_types.go +++ b/pkg/k8s/apis/cilium.io/v1alpha1/tracing_policy_types.go @@ -97,6 +97,10 @@ type TracingPolicySpec struct { // +kubebuilder:validation:Optional // A killer spec. Killers []KillerSpec `json:"killers,omitempty"` + + // +kubebuilder:validation:Optional + // A list of overloaded options + Options []OptionSpec `json:"options,omitempty"` } func (tp *TracingPolicy) TpName() string { diff --git a/pkg/k8s/apis/cilium.io/v1alpha1/types.go b/pkg/k8s/apis/cilium.io/v1alpha1/types.go index c202953245c..8b7e03a6f77 100644 --- a/pkg/k8s/apis/cilium.io/v1alpha1/types.go +++ b/pkg/k8s/apis/cilium.io/v1alpha1/types.go @@ -259,6 +259,14 @@ type ListSpec struct { Validated bool `json:"validated"` } +type OptionSpec struct { + // Name of the option + Name string `json:"name"` + // +kubebuilder:validation:Optional + // Value of the option + Value string `json:"value"` +} + type PodInfoSpec struct { // Host networking requested for this pod. Use the host's network namespace. // If this option is set, the ports that will be used must be specified. diff --git a/pkg/k8s/apis/cilium.io/v1alpha1/version.go b/pkg/k8s/apis/cilium.io/v1alpha1/version.go index dcdaae9602f..980fe39ab18 100644 --- a/pkg/k8s/apis/cilium.io/v1alpha1/version.go +++ b/pkg/k8s/apis/cilium.io/v1alpha1/version.go @@ -7,4 +7,4 @@ package v1alpha1 // Used to determine if CRD needs to be updated in cluster // // Developers: Bump patch for each change in the CRD schema. -const CustomResourceDefinitionSchemaVersion = "1.0.0" +const CustomResourceDefinitionSchemaVersion = "1.0.1" diff --git a/pkg/k8s/apis/cilium.io/v1alpha1/zz_generated.deepcopy.go b/pkg/k8s/apis/cilium.io/v1alpha1/zz_generated.deepcopy.go index 6cc4449f79b..ec8f17b6877 100644 --- a/pkg/k8s/apis/cilium.io/v1alpha1/zz_generated.deepcopy.go +++ b/pkg/k8s/apis/cilium.io/v1alpha1/zz_generated.deepcopy.go @@ -307,6 +307,22 @@ func (in *NamespaceSelector) DeepCopy() *NamespaceSelector { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *OptionSpec) DeepCopyInto(out *OptionSpec) { + *out = *in + return +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new OptionSpec. +func (in *OptionSpec) DeepCopy() *OptionSpec { + if in == nil { + return nil + } + out := new(OptionSpec) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *PIDSelector) DeepCopyInto(out *PIDSelector) { *out = *in @@ -635,6 +651,11 @@ func (in *TracingPolicySpec) DeepCopyInto(out *TracingPolicySpec) { (*in)[i].DeepCopyInto(&(*out)[i]) } } + if in.Options != nil { + in, out := &in.Options, &out.Options + *out = make([]OptionSpec, len(*in)) + copy(*out, *in) + } return } diff --git a/vendor/github.com/cilium/tetragon/pkg/k8s/apis/cilium.io/client/crds/v1alpha1/cilium.io_tracingpolicies.yaml b/vendor/github.com/cilium/tetragon/pkg/k8s/apis/cilium.io/client/crds/v1alpha1/cilium.io_tracingpolicies.yaml index ec761c98e89..97b99fcd84a 100644 --- a/vendor/github.com/cilium/tetragon/pkg/k8s/apis/cilium.io/client/crds/v1alpha1/cilium.io_tracingpolicies.yaml +++ b/vendor/github.com/cilium/tetragon/pkg/k8s/apis/cilium.io/client/crds/v1alpha1/cilium.io_tracingpolicies.yaml @@ -610,6 +610,20 @@ spec: loader: description: Enable loader events type: boolean + options: + description: A list of overloaded options + items: + properties: + name: + description: Name of the option + type: string + value: + description: Value of the option + type: string + required: + - name + type: object + type: array podSelector: description: PodSelector selects pods that this policy applies to properties: diff --git a/vendor/github.com/cilium/tetragon/pkg/k8s/apis/cilium.io/client/crds/v1alpha1/cilium.io_tracingpoliciesnamespaced.yaml b/vendor/github.com/cilium/tetragon/pkg/k8s/apis/cilium.io/client/crds/v1alpha1/cilium.io_tracingpoliciesnamespaced.yaml index ce8c9faaba1..7a1c87ff27b 100644 --- a/vendor/github.com/cilium/tetragon/pkg/k8s/apis/cilium.io/client/crds/v1alpha1/cilium.io_tracingpoliciesnamespaced.yaml +++ b/vendor/github.com/cilium/tetragon/pkg/k8s/apis/cilium.io/client/crds/v1alpha1/cilium.io_tracingpoliciesnamespaced.yaml @@ -610,6 +610,20 @@ spec: loader: description: Enable loader events type: boolean + options: + description: A list of overloaded options + items: + properties: + name: + description: Name of the option + type: string + value: + description: Value of the option + type: string + required: + - name + type: object + type: array podSelector: description: PodSelector selects pods that this policy applies to properties: diff --git a/vendor/github.com/cilium/tetragon/pkg/k8s/apis/cilium.io/v1alpha1/tracing_policy_types.go b/vendor/github.com/cilium/tetragon/pkg/k8s/apis/cilium.io/v1alpha1/tracing_policy_types.go index 0423ae46318..935b68686d4 100644 --- a/vendor/github.com/cilium/tetragon/pkg/k8s/apis/cilium.io/v1alpha1/tracing_policy_types.go +++ b/vendor/github.com/cilium/tetragon/pkg/k8s/apis/cilium.io/v1alpha1/tracing_policy_types.go @@ -97,6 +97,10 @@ type TracingPolicySpec struct { // +kubebuilder:validation:Optional // A killer spec. Killers []KillerSpec `json:"killers,omitempty"` + + // +kubebuilder:validation:Optional + // A list of overloaded options + Options []OptionSpec `json:"options,omitempty"` } func (tp *TracingPolicy) TpName() string { diff --git a/vendor/github.com/cilium/tetragon/pkg/k8s/apis/cilium.io/v1alpha1/types.go b/vendor/github.com/cilium/tetragon/pkg/k8s/apis/cilium.io/v1alpha1/types.go index c202953245c..8b7e03a6f77 100644 --- a/vendor/github.com/cilium/tetragon/pkg/k8s/apis/cilium.io/v1alpha1/types.go +++ b/vendor/github.com/cilium/tetragon/pkg/k8s/apis/cilium.io/v1alpha1/types.go @@ -259,6 +259,14 @@ type ListSpec struct { Validated bool `json:"validated"` } +type OptionSpec struct { + // Name of the option + Name string `json:"name"` + // +kubebuilder:validation:Optional + // Value of the option + Value string `json:"value"` +} + type PodInfoSpec struct { // Host networking requested for this pod. Use the host's network namespace. // If this option is set, the ports that will be used must be specified. diff --git a/vendor/github.com/cilium/tetragon/pkg/k8s/apis/cilium.io/v1alpha1/version.go b/vendor/github.com/cilium/tetragon/pkg/k8s/apis/cilium.io/v1alpha1/version.go index dcdaae9602f..980fe39ab18 100644 --- a/vendor/github.com/cilium/tetragon/pkg/k8s/apis/cilium.io/v1alpha1/version.go +++ b/vendor/github.com/cilium/tetragon/pkg/k8s/apis/cilium.io/v1alpha1/version.go @@ -7,4 +7,4 @@ package v1alpha1 // Used to determine if CRD needs to be updated in cluster // // Developers: Bump patch for each change in the CRD schema. -const CustomResourceDefinitionSchemaVersion = "1.0.0" +const CustomResourceDefinitionSchemaVersion = "1.0.1" diff --git a/vendor/github.com/cilium/tetragon/pkg/k8s/apis/cilium.io/v1alpha1/zz_generated.deepcopy.go b/vendor/github.com/cilium/tetragon/pkg/k8s/apis/cilium.io/v1alpha1/zz_generated.deepcopy.go index 6cc4449f79b..ec8f17b6877 100644 --- a/vendor/github.com/cilium/tetragon/pkg/k8s/apis/cilium.io/v1alpha1/zz_generated.deepcopy.go +++ b/vendor/github.com/cilium/tetragon/pkg/k8s/apis/cilium.io/v1alpha1/zz_generated.deepcopy.go @@ -307,6 +307,22 @@ func (in *NamespaceSelector) DeepCopy() *NamespaceSelector { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *OptionSpec) DeepCopyInto(out *OptionSpec) { + *out = *in + return +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new OptionSpec. +func (in *OptionSpec) DeepCopy() *OptionSpec { + if in == nil { + return nil + } + out := new(OptionSpec) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *PIDSelector) DeepCopyInto(out *PIDSelector) { *out = *in @@ -635,6 +651,11 @@ func (in *TracingPolicySpec) DeepCopyInto(out *TracingPolicySpec) { (*in)[i].DeepCopyInto(&(*out)[i]) } } + if in.Options != nil { + in, out := &in.Options, &out.Options + *out = make([]OptionSpec, len(*in)) + copy(*out, *in) + } return } From d5dbc03849f5d905a04273540b23cce3572dd94f Mon Sep 17 00:00:00 2001 From: Jiri Olsa Date: Fri, 20 Oct 2023 07:33:33 +0000 Subject: [PATCH 3/6] tetragon: Pass whole spec to createGenericKprobeSensor We are about to pass options to createGenericKprobeSensor function, so it's better the function takes the whole spec poitner rather than adding another function argument. Signed-off-by: Jiri Olsa --- pkg/sensors/tracing/generickprobe.go | 6 ++++-- pkg/sensors/tracing/generickprobe_test.go | 16 ++++++++++------ pkg/sensors/tracing/policyhandler.go | 2 +- 3 files changed, 15 insertions(+), 9 deletions(-) diff --git a/pkg/sensors/tracing/generickprobe.go b/pkg/sensors/tracing/generickprobe.go index 5b9dc1dbefb..b45641f6733 100644 --- a/pkg/sensors/tracing/generickprobe.go +++ b/pkg/sensors/tracing/generickprobe.go @@ -495,11 +495,10 @@ func getKprobeSymbols(symbol string, syscall bool, lists []v1alpha1.ListSpec) ([ } func createGenericKprobeSensor( + spec *v1alpha1.TracingPolicySpec, name string, - kprobes []v1alpha1.KProbeSpec, policyID policyfilter.PolicyID, policyName string, - lists []v1alpha1.ListSpec, customHandler eventhandler.Handler, ) (*sensors.Sensor, error) { var progs []*program.Program @@ -508,6 +507,9 @@ func createGenericKprobeSensor( var useMulti bool var selMaps *selectors.KernelSelectorMaps + kprobes := spec.KProbes + lists := spec.Lists + // use multi kprobe only if: // - it's not disabled by user // - there's support detected diff --git a/pkg/sensors/tracing/generickprobe_test.go b/pkg/sensors/tracing/generickprobe_test.go index d6fa7140272..8ff5fb8c90a 100644 --- a/pkg/sensors/tracing/generickprobe_test.go +++ b/pkg/sensors/tracing/generickprobe_test.go @@ -67,17 +67,21 @@ func Test_SensorDestroyHook(t *testing.T) { t.Errorf("genericKprobeTable expected initial length: 0, got: %d", genericKprobeTable.Len()) } + spec := &v1alpha1.TracingPolicySpec{} + + spec.KProbes = []v1alpha1.KProbeSpec{ + { + Call: "test_symbol", + Syscall: false, + }, + } + // we use createGenericKprobeSensor because it's where the DestroyHook is // created. It would be technically more correct if it was added just after // insertion in the table in AddKprobe, but this is done by the caller to // have just DestroyHook that regroups all the potential multiple kprobes // contained in one sensor. - sensor, err := createGenericKprobeSensor("test_sensor", []v1alpha1.KProbeSpec{ - { - Call: "test_symbol", - Syscall: false, - }, - }, 0, "test_policy", nil, nil) + sensor, err := createGenericKprobeSensor(spec, "test_sensor", 0, "test_policy", nil) if err != nil { t.Errorf("createGenericKprobeSensor err expected: nil, got: %s", err) } diff --git a/pkg/sensors/tracing/policyhandler.go b/pkg/sensors/tracing/policyhandler.go index 275d89d288b..7ddb30d3df0 100644 --- a/pkg/sensors/tracing/policyhandler.go +++ b/pkg/sensors/tracing/policyhandler.go @@ -38,7 +38,7 @@ func (h policyHandler) PolicyHandler( if err != nil { return nil, fmt.Errorf("validation failed: %w", err) } - return createGenericKprobeSensor(name, spec.KProbes, policyID, policyName, spec.Lists, handler) + return createGenericKprobeSensor(spec, name, policyID, policyName, handler) } if len(spec.Tracepoints) > 0 { name := fmt.Sprintf("gtp-sensor-%d", atomic.AddUint64(&sensorCounter, 1)) From 01f3ae8daa1d3fd1bb12ee831f4de50983fd5858 Mon Sep 17 00:00:00 2001 From: Jiri Olsa Date: Fri, 20 Oct 2023 07:44:32 +0000 Subject: [PATCH 4/6] tetragon: Add support to pass options for spec Adding support to process options passed in spec for kprobe sensor. At the moment the only supported option is to disable kprobe multi (like with --disable-kprobe-multi option). Signed-off-by: Jiri Olsa --- pkg/sensors/tracing/generickprobe.go | 14 +++++++-- pkg/sensors/tracing/options.go | 47 ++++++++++++++++++++++++++++ 2 files changed, 58 insertions(+), 3 deletions(-) create mode 100644 pkg/sensors/tracing/options.go diff --git a/pkg/sensors/tracing/generickprobe.go b/pkg/sensors/tracing/generickprobe.go index b45641f6733..b67d424fffe 100644 --- a/pkg/sensors/tracing/generickprobe.go +++ b/pkg/sensors/tracing/generickprobe.go @@ -510,11 +510,19 @@ func createGenericKprobeSensor( kprobes := spec.KProbes lists := spec.Lists + options, err := getKprobeOptions(spec.Options) + if err != nil { + return nil, fmt.Errorf("failed to set options: %s", err) + } + // use multi kprobe only if: - // - it's not disabled by user + // - it's not disabled by spec option + // - it's not disabled by command line option // - there's support detected - useMulti = !option.Config.DisableKprobeMulti && - bpf.HasKprobeMulti() + if !options.DisableKprobeMulti { + useMulti = !option.Config.DisableKprobeMulti && + bpf.HasKprobeMulti() + } in := addKprobeIn{ useMulti: useMulti, diff --git a/pkg/sensors/tracing/options.go b/pkg/sensors/tracing/options.go new file mode 100644 index 00000000000..1445d6f68a7 --- /dev/null +++ b/pkg/sensors/tracing/options.go @@ -0,0 +1,47 @@ +// SPDX-License-Identifier: Apache-2.0 +// Copyright Authors of Tetragon + +package tracing + +import ( + "fmt" + "strconv" + + "github.com/cilium/tetragon/pkg/k8s/apis/cilium.io/v1alpha1" + "github.com/cilium/tetragon/pkg/logger" + "github.com/cilium/tetragon/pkg/option" +) + +type kprobeOptions struct { + DisableKprobeMulti bool +} + +type opt struct { + set func(val string, options *kprobeOptions) error +} + +// Allowed kprobe options +var opts = map[string]opt{ + option.KeyDisableKprobeMulti: opt{ + set: func(str string, options *kprobeOptions) (err error) { + options.DisableKprobeMulti, err = strconv.ParseBool(str) + return err + }, + }, +} + +func getKprobeOptions(specs []v1alpha1.OptionSpec) (*kprobeOptions, error) { + options := &kprobeOptions{} + + for _, spec := range specs { + opt, ok := opts[spec.Name] + if ok { + if err := opt.set(spec.Value, options); err != nil { + return nil, fmt.Errorf("failed to set option %s: %s", spec.Name, err) + } + logger.GetLogger().Infof("Set option %s = %s", spec.Name, spec.Value) + } + } + + return options, nil +} From 4135e9dec929ae3517bdca238868ec174757750a Mon Sep 17 00:00:00 2001 From: Jiri Olsa Date: Fri, 20 Oct 2023 08:03:47 +0000 Subject: [PATCH 5/6] tetragon: Switch off kprobe multi via spec in security_ test We can now localy disable kprobe multi, let's use it for security_ override test, which won't work with kprobe multi. Signed-off-by: Jiri Olsa --- pkg/sensors/tracing/kprobe_test.go | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/pkg/sensors/tracing/kprobe_test.go b/pkg/sensors/tracing/kprobe_test.go index 8f9fbb1f08a..38b51be6258 100644 --- a/pkg/sensors/tracing/kprobe_test.go +++ b/pkg/sensors/tracing/kprobe_test.go @@ -2212,10 +2212,6 @@ func TestKprobeOverrideSecurity(t *testing.T) { t.Skip("skipping fmod_ret support is not available") } - if !option.Config.DisableKprobeMulti && bpf.HasKprobeMulti() { - t.Skip("skipping fmod_ret does not work with kprobe multi") - } - pidStr := strconv.Itoa(int(observertesthelper.GetMyPid())) file, err := os.CreateTemp(t.TempDir(), "kprobe-override-") @@ -2230,6 +2226,9 @@ kind: TracingPolicy metadata: name: "sys-openat-override" spec: + options: + - name: "disable-kprobe-multi" + value: "1" kprobes: - call: "security_file_open" syscall: false From c9a76142e0abd553174cbd853df4428658a95552 Mon Sep 17 00:00:00 2001 From: Jiri Olsa Date: Wed, 25 Oct 2023 07:23:10 +0000 Subject: [PATCH 6/6] tetragon: Add documentation for options Signed-off-by: Jiri Olsa --- .../docs/concepts/tracing-policy/options.md | 42 +++++++++++++++++++ 1 file changed, 42 insertions(+) create mode 100644 docs/content/en/docs/concepts/tracing-policy/options.md diff --git a/docs/content/en/docs/concepts/tracing-policy/options.md b/docs/content/en/docs/concepts/tracing-policy/options.md new file mode 100644 index 00000000000..b95428f5e83 --- /dev/null +++ b/docs/content/en/docs/concepts/tracing-policy/options.md @@ -0,0 +1,42 @@ +--- +title: "Options" +icon: "overview" +weight: 3 +description: "Pass options to hook" +--- + +It's possible to pass options through spec file as an array of name and value pairs: + +```yaml +spec: + options: + - name: "option-1" + value: "True" + - name: "option-2" + value: "10" +``` + +Options array is passed and processed by each hook used in the spec file that +supports options. At the moment it's availabe for kprobe hooks. + +- [`Kprobe Options`](#kprobe-options): options for kprobe hooks. + +## Kprobe options + +- [`disable-kprobe-multi`](#disable-kprobe-multi): disable kprobe multi link + +### disable-kprobe-multi + +This option disables kprobe multi link interface for all the kprobes defined in +the spec file. If enabled, all the defined kprobes will be atached through standard +kprobe interface. It stays enabled for another spec file without this option. + +It takes boolean as value, by default it's false. + +Example: + +```yaml + options: + - name: "disable-kprobe-multi" + value: "1" +```