From e66d4fc5ab5860fc905a4318db799708fb2c5286 Mon Sep 17 00:00:00 2001 From: Mahe Tardy Date: Fri, 14 Jun 2024 18:55:59 +0200 Subject: [PATCH 1/3] pkg/sensors: checks for nil pointer dereference Signed-off-by: Mahe Tardy --- pkg/sensors/tracing/generickprobe.go | 4 ++++ pkg/sensors/tracing/generictracepoint.go | 4 ++++ 2 files changed, 8 insertions(+) diff --git a/pkg/sensors/tracing/generickprobe.go b/pkg/sensors/tracing/generickprobe.go index 8a625b5cf41..f738e24dd4d 100644 --- a/pkg/sensors/tracing/generickprobe.go +++ b/pkg/sensors/tracing/generickprobe.go @@ -646,6 +646,10 @@ func addKprobe(funcName string, f *v1alpha1.KProbeSpec, in *addKprobeIn) (id idt return idtable.UninitializedEntryID, err } + if f == nil { + return errFn(errors.New("error adding kprobe, the kprobe spec is nil")) + } + config := &api.EventConfig{} config.PolicyID = uint32(in.policyID) if len(f.ReturnArgAction) > 0 { diff --git a/pkg/sensors/tracing/generictracepoint.go b/pkg/sensors/tracing/generictracepoint.go index 2dcd72a9759..6cfb1bf16e5 100644 --- a/pkg/sensors/tracing/generictracepoint.go +++ b/pkg/sensors/tracing/generictracepoint.go @@ -312,6 +312,10 @@ func createGenericTracepoint( policyName string, customHandler eventhandler.Handler, ) (*genericTracepoint, error) { + if conf == nil { + return nil, errors.New("failed creating generic tracepoint, conf is nil") + } + tp := tracepoint.Tracepoint{ Subsys: conf.Subsystem, Event: conf.Event, From 1bfd8bfeada23d48734fe1efaa781e5f7c953961 Mon Sep 17 00:00:00 2001 From: Mahe Tardy Date: Fri, 14 Jun 2024 12:43:50 +0200 Subject: [PATCH 2/3] pkg/sensors: fix potential issue in stacktrace detection Previous condition was buggy and should have been: hasStackTrace = hasStackTrace || KernelStackTrace || UserStackTrace instead of : hasStackTrace = KernelStackTrace || UserStackTrace Using a function simplify this. Signed-off-by: Mahe Tardy --- pkg/sensors/tracing/generickprobe.go | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/pkg/sensors/tracing/generickprobe.go b/pkg/sensors/tracing/generickprobe.go index f738e24dd4d..4917e480643 100644 --- a/pkg/sensors/tracing/generickprobe.go +++ b/pkg/sensors/tracing/generickprobe.go @@ -785,13 +785,6 @@ func addKprobe(funcName string, f *v1alpha1.KProbeSpec, in *addKprobeIn) (id idt config.Syscall = 0 } - hasStackTrace := false - for _, selector := range f.Selectors { - for _, matchAction := range selector.MatchActions { - hasStackTrace = matchAction.KernelStackTrace || matchAction.UserStackTrace - } - } - // create a new entry on the table, and pass its id to BPF-side // so that we can do the matching at event-generation time kprobeEntry := genericKprobe{ @@ -810,7 +803,7 @@ func addKprobe(funcName string, f *v1alpha1.KProbeSpec, in *addKprobeIn) (id idt customHandler: in.customHandler, message: msgField, tags: tagsField, - hasStackTrace: hasStackTrace, + hasStackTrace: selectorsHaveStackTrace(f.Selectors), hasRatelimit: selectorsHaveRateLimit(f.Selectors), } @@ -1316,3 +1309,14 @@ func selectorsHaveRateLimit(selectors []v1alpha1.KProbeSelector) bool { } return false } + +func selectorsHaveStackTrace(selectors []v1alpha1.KProbeSelector) bool { + for _, selector := range selectors { + for _, matchAction := range selector.MatchActions { + if matchAction.KernelStackTrace || matchAction.UserStackTrace { + return true + } + } + } + return false +} From 2631f9b91d87247770531dfc1582665ebf19935c Mon Sep 17 00:00:00 2001 From: Mahe Tardy Date: Fri, 14 Jun 2024 18:55:15 +0200 Subject: [PATCH 3/3] pkg/sensors: reduce memory footprint of unused fdinstall maps Resize the fdinstall_map if needed to save memory, thus we are saving ~11MB of kernel memory by kprobe that are not using FollowFD, UnfollowFD or CopyFD. Signed-off-by: Mahe Tardy --- bpf/process/types/basic.h | 2 +- pkg/sensors/tracing/generickprobe.go | 48 +++++++++++++++++++++--- pkg/sensors/tracing/generictracepoint.go | 3 ++ 3 files changed, 46 insertions(+), 7 deletions(-) diff --git a/bpf/process/types/basic.h b/bpf/process/types/basic.h index 3c234768e5f..acb78562a2f 100644 --- a/bpf/process/types/basic.h +++ b/bpf/process/types/basic.h @@ -1777,7 +1777,7 @@ struct fdinstall_value { struct { __uint(type, BPF_MAP_TYPE_LRU_HASH); - __uint(max_entries, 32000); + __uint(max_entries, 1); // will be resized by agent when needed __type(key, struct fdinstall_key); __type(value, struct fdinstall_value); } fdinstall_map SEC(".maps"); diff --git a/pkg/sensors/tracing/generickprobe.go b/pkg/sensors/tracing/generickprobe.go index 4917e480643..2d208f72e80 100644 --- a/pkg/sensors/tracing/generickprobe.go +++ b/pkg/sensors/tracing/generickprobe.go @@ -62,6 +62,7 @@ const ( // much kernel memory when enabled. stackTraceMapMaxEntries = 32768 ratelimitMapMaxEntries = 32768 + fdInstallMapMaxEntries = 32000 ) func kprobeCharBufErrorToString(e int32) string { @@ -271,7 +272,7 @@ func filterMaps(load *program.Program, pinPath string, kprobeEntry *genericKprob return maps } -func createMultiKprobeSensor(sensorPath, policyName string, multiIDs []idtable.EntryID) ([]*program.Program, []*program.Map, error) { +func createMultiKprobeSensor(sensorPath, policyName string, multiIDs []idtable.EntryID, enableFDInstall bool) ([]*program.Program, []*program.Map, error) { var multiRetIDs []idtable.EntryID var progs []*program.Program var maps []*program.Map @@ -315,6 +316,9 @@ func createMultiKprobeSensor(sensorPath, policyName string, multiIDs []idtable.E progs = append(progs, load) fdinstall := program.MapBuilderPin("fdinstall_map", sensors.PathJoin(sensorPath, "fdinstall_map"), load) + if enableFDInstall { + fdinstall.SetMaxEntries(fdInstallMapMaxEntries) + } maps = append(maps, fdinstall) configMap := program.MapBuilderPin("config_map", sensors.PathJoin(pinPath, "config_map"), load) @@ -392,6 +396,9 @@ func createMultiKprobeSensor(sensorPath, policyName string, multiIDs []idtable.E maps = append(maps, callHeap) fdinstall := program.MapBuilderPin("fdinstall_map", sensors.PathJoin(sensorPath, "fdinstall_map"), loadret) + if enableFDInstall { + fdinstall.SetMaxEntries(fdInstallMapMaxEntries) + } maps = append(maps, fdinstall) socktrack := program.MapBuilderPin("socktrack_map", sensors.PathJoin(sensorPath, "socktrack_map"), loadret) @@ -578,6 +585,16 @@ func createGenericKprobeSensor( selMaps = &selectors.KernelSelectorMaps{} } + // detect at the policy level if one kprobe uses the fdinstall feature since + // the map is shared amongst all kprobes + oneKprobeHasFDInstall := false + for _, kprobe := range kprobes { + if selectorsHaveFDInstall(kprobe.Selectors) { + oneKprobeHasFDInstall = true + break + } + } + in := addKprobeIn{ useMulti: useMulti, sensorPath: name, @@ -606,9 +623,9 @@ func createGenericKprobeSensor( } if useMulti { - progs, maps, err = createMultiKprobeSensor(in.sensorPath, in.policyName, ids) + progs, maps, err = createMultiKprobeSensor(in.sensorPath, in.policyName, ids, oneKprobeHasFDInstall) } else { - progs, maps, err = createSingleKprobeSensor(in.sensorPath, ids) + progs, maps, err = createSingleKprobeSensor(in.sensorPath, ids, oneKprobeHasFDInstall) } if err != nil { @@ -845,7 +862,7 @@ func addKprobe(funcName string, f *v1alpha1.KProbeSpec, in *addKprobeIn) (id idt } func createKprobeSensorFromEntry(kprobeEntry *genericKprobe, sensorPath string, - progs []*program.Program, maps []*program.Map) ([]*program.Program, []*program.Map) { + progs []*program.Program, maps []*program.Map, enableFDInstall bool) ([]*program.Program, []*program.Map) { loadProgName, loadProgRetName := kernels.GenericKprobeObjs() isSecurityFunc := strings.HasPrefix(kprobeEntry.funcName, "security_") @@ -867,6 +884,9 @@ func createKprobeSensorFromEntry(kprobeEntry *genericKprobe, sensorPath string, progs = append(progs, load) fdinstall := program.MapBuilderPin("fdinstall_map", sensors.PathJoin(sensorPath, "fdinstall_map"), load) + if enableFDInstall { + fdinstall.SetMaxEntries(fdInstallMapMaxEntries) + } maps = append(maps, fdinstall) configMap := program.MapBuilderPin("config_map", sensors.PathJoin(pinPath, "config_map"), load) @@ -958,6 +978,9 @@ func createKprobeSensorFromEntry(kprobeEntry *genericKprobe, sensorPath string, maps = append(maps, callHeap) fdinstall := program.MapBuilderPin("fdinstall_map", sensors.PathJoin(sensorPath, "fdinstall_map"), loadret) + if enableFDInstall { + fdinstall.SetMaxEntries(fdInstallMapMaxEntries) + } maps = append(maps, fdinstall) if kernels.EnableLargeProgs() { @@ -971,7 +994,7 @@ func createKprobeSensorFromEntry(kprobeEntry *genericKprobe, sensorPath string, return progs, maps } -func createSingleKprobeSensor(sensorPath string, ids []idtable.EntryID) ([]*program.Program, []*program.Map, error) { +func createSingleKprobeSensor(sensorPath string, ids []idtable.EntryID, enableFDInstall bool) ([]*program.Program, []*program.Map, error) { var progs []*program.Program var maps []*program.Map @@ -981,7 +1004,7 @@ func createSingleKprobeSensor(sensorPath string, ids []idtable.EntryID) ([]*prog return nil, nil, err } gk.data = &genericKprobeData{} - progs, maps = createKprobeSensorFromEntry(gk, sensorPath, progs, maps) + progs, maps = createKprobeSensorFromEntry(gk, sensorPath, progs, maps, enableFDInstall) } return progs, maps, nil @@ -1320,3 +1343,16 @@ func selectorsHaveStackTrace(selectors []v1alpha1.KProbeSelector) bool { } return false } + +func selectorsHaveFDInstall(sel []v1alpha1.KProbeSelector) bool { + for _, selector := range sel { + for _, matchAction := range selector.MatchActions { + if a := selectors.ActionTypeFromString(matchAction.Action); a == selectors.ActionTypeFollowFd || + a == selectors.ActionTypeUnfollowFd || + a == selectors.ActionTypeCopyFd { + return true + } + } + } + return false +} diff --git a/pkg/sensors/tracing/generictracepoint.go b/pkg/sensors/tracing/generictracepoint.go index 6cfb1bf16e5..fd8a8f79a7b 100644 --- a/pkg/sensors/tracing/generictracepoint.go +++ b/pkg/sensors/tracing/generictracepoint.go @@ -409,6 +409,9 @@ func createGenericTracepointSensor( progs = append(progs, prog0) fdinstall := program.MapBuilderPin("fdinstall_map", sensors.PathJoin(pinPath, "fdinstall_map"), prog0) + if selectorsHaveFDInstall(tp.Spec.Selectors) { + fdinstall.SetMaxEntries(fdInstallMapMaxEntries) + } maps = append(maps, fdinstall) tailCalls := program.MapBuilderPin("tp_calls", sensors.PathJoin(pinPath, "tp_calls"), prog0)