From c8feb914092f6b98d7132c739d2e9b4e295f6064 Mon Sep 17 00:00:00 2001 From: Anastasios Papagiannis Date: Wed, 11 Oct 2023 15:39:18 +0000 Subject: [PATCH] bugfix: Use the same selector maps in kprobe-multi The previous commit shows a test that we have a bug in the case of kprobe-multi, that uses matchArgs where both of these use maps to do equals/prefix/postfix operations. The problem is due to overriding string maps after processing each kprobe. To fix that we use shared maps for all selectors for all attach points in te case of kprobe-multi. Non kprobe-multi cases and tracepoints/uprobes work exactly as before. Signed-off-by: Anastasios Papagiannis --- pkg/selectors/kernel.go | 6 +-- pkg/selectors/kernel_test.go | 4 +- pkg/selectors/selectors.go | 50 ++++++++++++++---------- pkg/sensors/tracing/generickprobe.go | 15 +++++-- pkg/sensors/tracing/generictracepoint.go | 2 +- pkg/sensors/tracing/genericuprobe.go | 2 +- 6 files changed, 48 insertions(+), 31 deletions(-) diff --git a/pkg/selectors/kernel.go b/pkg/selectors/kernel.go index f94b23b92c4..dc7ce8077da 100644 --- a/pkg/selectors/kernel.go +++ b/pkg/selectors/kernel.go @@ -1258,7 +1258,7 @@ func parseSelector( // // For some examples, see kernel_test.go func InitKernelSelectors(selectors []v1alpha1.KProbeSelector, args []v1alpha1.KProbeArg, actionArgTable *idtable.Table) ([4096]byte, error) { - kernelSelectors, err := InitKernelSelectorState(selectors, args, actionArgTable, nil) + kernelSelectors, err := InitKernelSelectorState(selectors, args, actionArgTable, nil, nil) if err != nil { return [4096]byte{}, err } @@ -1266,8 +1266,8 @@ func InitKernelSelectors(selectors []v1alpha1.KProbeSelector, args []v1alpha1.KP } func InitKernelSelectorState(selectors []v1alpha1.KProbeSelector, args []v1alpha1.KProbeArg, - actionArgTable *idtable.Table, listReader ValueReader) (*KernelSelectorState, error) { - kernelSelectors := NewKernelSelectorState(listReader) + actionArgTable *idtable.Table, listReader ValueReader, maps *KernelSelectorMaps) (*KernelSelectorState, error) { + kernelSelectors := NewKernelSelectorState(listReader, maps) WriteSelectorUint32(kernelSelectors, uint32(len(selectors))) soff := make([]uint32, len(selectors)) diff --git a/pkg/selectors/kernel_test.go b/pkg/selectors/kernel_test.go index 82a11bafcc0..fed50e564c1 100644 --- a/pkg/selectors/kernel_test.go +++ b/pkg/selectors/kernel_test.go @@ -221,7 +221,7 @@ func TestParseMatchArg(t *testing.T) { } arg1 := &v1alpha1.ArgSelector{Index: 1, Operator: "Equal", Values: []string{"foobar"}} - k := &KernelSelectorState{off: 0} + k := NewKernelSelectorState(nil, nil) expected1 := []byte{ 0x01, 0x00, 0x00, 0x00, // Index == 1 0x03, 0x00, 0x00, 0x00, // operator == equal @@ -318,7 +318,7 @@ func TestParseMatchArg(t *testing.T) { expected3 := append(length, expected1[:]...) expected3 = append(expected3, expected2[:]...) arg12 := []v1alpha1.ArgSelector{*arg1, *arg2} - ks := &KernelSelectorState{off: 0} + ks := NewKernelSelectorState(nil, nil) if err := ParseMatchArgs(ks, arg12, sig); err != nil || bytes.Equal(expected3, ks.e[0:ks.off]) == false { t.Errorf("parseMatchArgs: error %v expected:\n%v\nbytes:\n%v\nparsing %v\n", err, expected3, ks.e[0:k.off], arg3) } diff --git a/pkg/selectors/selectors.go b/pkg/selectors/selectors.go index 8f92b550b1f..6d6de5c3ba2 100644 --- a/pkg/selectors/selectors.go +++ b/pkg/selectors/selectors.go @@ -75,6 +75,15 @@ type KernelLPMTrieStringPostfix struct { data [StringPostfixMaxLength]byte } +type KernelSelectorMaps struct { + // stringMaps are used to populate string and char buf matches + stringMaps StringMapLists + // stringPrefixMaps are used to populate string and char buf prefix matches + stringPrefixMaps []map[KernelLPMTrieStringPrefix]struct{} + // stringPostfixMaps are used to populate string and char buf postfix matches + stringPostfixMaps []map[KernelLPMTrieStringPostfix]struct{} +} + type KernelSelectorState struct { off uint32 // offset into encoding e [4096]byte // kernel encoding of selectors @@ -92,20 +101,19 @@ type KernelSelectorState struct { newBinVals map[uint32]string // these should be added in the names_map listReader ValueReader - // stringMaps are used to populate string and char buf matches - stringMaps StringMapLists - // stringPrefixMaps are used to populate string and char buf prefix matches - stringPrefixMaps []map[KernelLPMTrieStringPrefix]struct{} - // stringPostfixMaps are used to populate string and char buf postfix matches - stringPostfixMaps []map[KernelLPMTrieStringPostfix]struct{} + maps *KernelSelectorMaps } -func NewKernelSelectorState(listReader ValueReader) *KernelSelectorState { +func NewKernelSelectorState(listReader ValueReader, maps *KernelSelectorMaps) *KernelSelectorState { + if maps == nil { + maps = &KernelSelectorMaps{} + } return &KernelSelectorState{ matchBinaries: make(map[int]*MatchBinariesMappings), newBinVals: make(map[uint32]string), listReader: listReader, + maps: maps, } } @@ -165,15 +173,15 @@ func (k *KernelSelectorState) Addr6Maps() []map[KernelLPMTrie6]struct{} { } func (k *KernelSelectorState) StringMaps(subMap int) []map[[MaxStringMapsSize]byte]struct{} { - return k.stringMaps[subMap] + return k.maps.stringMaps[subMap] } func (k *KernelSelectorState) StringPrefixMaps() []map[KernelLPMTrieStringPrefix]struct{} { - return k.stringPrefixMaps + return k.maps.stringPrefixMaps } func (k *KernelSelectorState) StringPostfixMaps() []map[KernelLPMTrieStringPostfix]struct{} { - return k.stringPostfixMaps + return k.maps.stringPostfixMaps } // ValueMapsMaxEntries returns the maximum entries over all maps @@ -212,7 +220,7 @@ func (k *KernelSelectorState) Addr6MapsMaxEntries() int { // StringMapsMaxEntries returns the maximum entries over all maps inside a particular map of map func (k *KernelSelectorState) StringMapsMaxEntries(subMap int) int { maxEntries := 1 - for _, vm := range k.stringMaps[subMap] { + for _, vm := range k.maps.stringMaps[subMap] { if l := len(vm); l > maxEntries { maxEntries = l } @@ -223,7 +231,7 @@ func (k *KernelSelectorState) StringMapsMaxEntries(subMap int) int { // StringPrefixMapsMaxEntries returns the maximum entries over all maps func (k *KernelSelectorState) StringPrefixMapsMaxEntries() int { maxEntries := 1 - for _, vm := range k.stringPrefixMaps { + for _, vm := range k.maps.stringPrefixMaps { if l := len(vm); l > maxEntries { maxEntries = l } @@ -234,7 +242,7 @@ func (k *KernelSelectorState) StringPrefixMapsMaxEntries() int { // StringPostfixMapsMaxEntries returns the maximum entries over all maps func (k *KernelSelectorState) StringPostfixMapsMaxEntries() int { maxEntries := 1 - for _, vm := range k.stringPostfixMaps { + for _, vm := range k.maps.stringPostfixMaps { if l := len(vm); l > maxEntries { maxEntries = l } @@ -402,8 +410,8 @@ func (k *KernelSelectorState) insertStringMaps(stringMaps SelectorStringMaps) [S for subMap := 0; subMap < StringMapsNumSubMaps; subMap++ { if len(stringMaps[subMap]) > 0 { - mapid = uint32(len(k.stringMaps[subMap])) - k.stringMaps[subMap] = append(k.stringMaps[subMap], stringMaps[subMap]) + mapid = uint32(len(k.maps.stringMaps[subMap])) + k.maps.stringMaps[subMap] = append(k.maps.stringMaps[subMap], stringMaps[subMap]) } else { mapid = 0xffffffff } @@ -414,13 +422,13 @@ func (k *KernelSelectorState) insertStringMaps(stringMaps SelectorStringMaps) [S } func (k *KernelSelectorState) newStringPrefixMap() (uint32, map[KernelLPMTrieStringPrefix]struct{}) { - mapid := len(k.stringPrefixMaps) - k.stringPrefixMaps = append(k.stringPrefixMaps, map[KernelLPMTrieStringPrefix]struct{}{}) - return uint32(mapid), k.stringPrefixMaps[mapid] + mapid := len(k.maps.stringPrefixMaps) + k.maps.stringPrefixMaps = append(k.maps.stringPrefixMaps, map[KernelLPMTrieStringPrefix]struct{}{}) + return uint32(mapid), k.maps.stringPrefixMaps[mapid] } func (k *KernelSelectorState) newStringPostfixMap() (uint32, map[KernelLPMTrieStringPostfix]struct{}) { - mapid := len(k.stringPostfixMaps) - k.stringPostfixMaps = append(k.stringPostfixMaps, map[KernelLPMTrieStringPostfix]struct{}{}) - return uint32(mapid), k.stringPostfixMaps[mapid] + mapid := len(k.maps.stringPostfixMaps) + k.maps.stringPostfixMaps = append(k.maps.stringPostfixMaps, map[KernelLPMTrieStringPostfix]struct{}{}) + return uint32(mapid), k.maps.stringPostfixMaps[mapid] } diff --git a/pkg/sensors/tracing/generickprobe.go b/pkg/sensors/tracing/generickprobe.go index 57bfed8a76f..d7b956d6067 100644 --- a/pkg/sensors/tracing/generickprobe.go +++ b/pkg/sensors/tracing/generickprobe.go @@ -518,6 +518,7 @@ func createGenericKprobeSensor( } addedKprobeIndices := []int{} + selMaps := &selectors.KernelSelectorMaps{} for i := range kprobes { syms, syscall, err := getKprobeSymbols(kprobes[i].Call, kprobes[i].Syscall, lists) if err != nil { @@ -528,7 +529,12 @@ func createGenericKprobeSensor( kprobes[i].Syscall = syscall for idx := range syms { - out, err := addKprobe(syms[idx], &kprobes[i], &in) + // if we use kprobe-multi, we need to share the maps. + // Otherwise, we create new ones. + if !useMulti { + selMaps = &selectors.KernelSelectorMaps{} + } + out, err := addKprobe(syms[idx], &kprobes[i], &in, selMaps) if err != nil { return nil, err } @@ -581,7 +587,7 @@ func createGenericKprobeSensor( // addKprobe will, amongst other things, create a generic kprobe entry and add // it to the genericKprobeTable. The caller should make sure that this entry is // properly removed on kprobe unload. -func addKprobe(funcName string, f *v1alpha1.KProbeSpec, in *addKprobeIn) (out *addKprobeOut, err error) { +func addKprobe(funcName string, f *v1alpha1.KProbeSpec, in *addKprobeIn, selMaps *selectors.KernelSelectorMaps) (out *addKprobeOut, err error) { var argSigPrinters []argPrinters var argReturnPrinters []argPrinters var setRetprobe bool @@ -747,7 +753,10 @@ func addKprobe(funcName string, f *v1alpha1.KProbeSpec, in *addKprobeIn) (out *a } // Parse Filters into kernel filter logic - kprobeEntry.loadArgs.selectors, err = selectors.InitKernelSelectorState(f.Selectors, f.Args, &kprobeEntry.actionArgs, nil) + // If we use kprobe-multi and this is the first kprobe, we initialize new selector maps. + // In all the other cases we use shared maps for all attach points. In the case of + // non-kprobe-multi each kprobe has it's own maps. + kprobeEntry.loadArgs.selectors, err = selectors.InitKernelSelectorState(f.Selectors, f.Args, &kprobeEntry.actionArgs, nil, selMaps) if err != nil { return nil, err } diff --git a/pkg/sensors/tracing/generictracepoint.go b/pkg/sensors/tracing/generictracepoint.go index f8ecf9a84a7..5af8b3b96ff 100644 --- a/pkg/sensors/tracing/generictracepoint.go +++ b/pkg/sensors/tracing/generictracepoint.go @@ -508,7 +508,7 @@ func (tp *genericTracepoint) InitKernelSelectors(lists []v1alpha1.ListSpec) erro } } - selectors, err := selectors.InitKernelSelectorState(selSelectors, selArgs, &tp.actionArgs, &listReader{lists}) + selectors, err := selectors.InitKernelSelectorState(selSelectors, selArgs, &tp.actionArgs, &listReader{lists}, nil) if err != nil { return err } diff --git a/pkg/sensors/tracing/genericuprobe.go b/pkg/sensors/tracing/genericuprobe.go index e5df0626f67..1264a7af34a 100644 --- a/pkg/sensors/tracing/genericuprobe.go +++ b/pkg/sensors/tracing/genericuprobe.go @@ -200,7 +200,7 @@ func createGenericUprobeSensor( } // Parse Filters into kernel filter logic - uprobeSelectorState, err := selectors.InitKernelSelectorState(spec.Selectors, args, nil, nil) + uprobeSelectorState, err := selectors.InitKernelSelectorState(spec.Selectors, args, nil, nil, nil) if err != nil { return nil, err }