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 <tasos.papagiannnis@gmail.com>

Author: tpapagian

pkg/selectors/kernel.go

@@ -1258,16 +1258,16 @@ 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
 	}
 	return kernelSelectors.e, nil
 }
 
 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))

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)
 		}

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]
 }

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,7 @@ 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)
+	kprobeEntry.loadArgs.selectors, err = selectors.InitKernelSelectorState(f.Selectors, f.Args, &kprobeEntry.actionArgs, nil, selMaps)
 	if err != nil {
 		return nil, err
 	}

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
 	}

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
 		}