tetragon: fixup generic tracepoint sensor create

[Y-dc]

tracepoint sensor creation not as expected.
source code
use &conf cause all genericTracepoint.Spec as same as the last element in genericTracepointTable, should use &confs[i] instead.

fix #569
Signed-off-by: dechengyuan dechengyuan@tencent.com

[olsajiri]

could you please be more specific what's failing, best with crd example and description of expected behavior? would be great to have automated test for that if possible, thanks

[anjmao]

@Y-dc I see that golanglint-ci is used https://github.com/cilium/tetragon/blob/main/.golangci.yml
But it doesn't use exportloopref check which should catch such bugs.

I suggest to enable all linters which covers bugs section https://golangci-lint.run/usage/linters/

[Y-dc]

could you please be more specific what's failing, best with crd example and description of expected behavior? would be great to have automated test for that if possible, thanks

@olsajiri I added some logs before and after the loops.

// createGenericTracepointSensor will create a sensor that can be loaded based on a generic tracepoint configuration
func createGenericTracepointSensor(name string, confs []GenericTracepointConf) (*sensors.Sensor, error) {
	tracepoints := make([]*genericTracepoint, 0, len(confs))
        // ----------add log
	js,_ := json.Marshal(genericTracepointTable.arr)
	fmt.Println("before creating genericTracepointTable: ",string(js))
	js,_ = json.Marshal(confs)
	fmt.Println("before creating confs: ",string(js))
	// -------------

	for _, conf := range confs {
		tp, err := createGenericTracepoint(name, &conf)
		if err != nil {
			return nil, err
		}

                 // ------------add log
		js,_ = json.Marshal(tp.Spec)
		fmt.Println("creating TracepointSpec:",string(js))
		tracepoints = append(tracepoints, tp)
                 // --------------------

	}
        // ------------add log
	js,_ = json.Marshal(genericTracepointTable.arr)
	fmt.Println("after creating genericTracepointTable: ",string(js))
        // --------------------

	//......
}

add tracingpolicy:

apiVersion: cilium.io/v1alpha1
kind: TracingPolicy
metadata:
  name: http
spec:
  tracepoints:
  - args:
    - index: 5
      returnCopy: false
      type: fd
    - index: 6
      returnCopy: false
      sizeArgIndex: 8
      type: char_buf
    - index: 7
      returnCopy: false
      type: size_t
    event: sys_enter_write
    selectors:
    - matchArgs:
      - index: 6
        operator: Prefix
        values:
        - HTTP
    subsystem: syscalls
  - args:
    - index: 5
      returnCopy: false
      type: fd
    - index: 6
      returnCopy: false
      sizeArgIndex: 8
      type: char_buf
    - index: 7
      returnCopy: false
      type: size_t
    event: sys_enter_read
    selectors:
    - {}
    subsystem: syscalls

then output:

time="2022-11-30T10:25:45Z" level=info msg="tracing policy added" policy="{[] [{syscalls sys_enter_write [{5 fd 0 false} {6 char_buf 8 false} {7 size_t 0 false}] [{[] [{6 Prefix [HTTP]}] [] [] [] [] [] [] []}]} {syscalls sys_enter_read [{5 fd 0 false} {6 char_buf 8 false} {7 size_t 0 false}] [{[] [] [] [] [] [] [] [] []}]}]}"
before creating genericTracepointTable:  null
before creating confs:  [{"subsystem":"syscalls","event":"sys_enter_write","args":[{"index":5,"type":"fd","sizeArgIndex":0,"returnCopy":false},{"index":6,"type":"char_buf","sizeArgIndex":8,"returnCopy":false},{"index":7,"type":"size_t","sizeArgIndex":0,"returnCopy":false}],"selectors":[{"matchPIDs":null,"matchArgs":[{"index":6,"operator":"Prefix","values":["HTTP"]}],"matchActions":null,"matchReturnArgs":null,"matchBinaries":null,"matchNamespaces":null,"matchNamespaceChanges":null,"matchCapabilities":null,"matchCapabilityChanges":null}]},{"subsystem":"syscalls","event":"sys_enter_read","args":[{"index":5,"type":"fd","sizeArgIndex":0,"returnCopy":false},{"index":6,"type":"char_buf","sizeArgIndex":8,"returnCopy":false},{"index":7,"type":"size_t","sizeArgIndex":0,"returnCopy":false}],"selectors":[{"matchPIDs":null,"matchArgs":null,"matchActions":null,"matchReturnArgs":null,"matchBinaries":null,"matchNamespaces":null,"matchNamespaceChanges":null,"matchCapabilities":null,"matchCapabilityChanges":null}]}]
creating TracepointSpec: {"subsystem":"syscalls","event":"sys_enter_write","args":[{"index":5,"type":"fd","sizeArgIndex":0,"returnCopy":false},{"index":6,"type":"char_buf","sizeArgIndex":8,"returnCopy":false},{"index":7,"type":"size_t","sizeArgIndex":0,"returnCopy":false}],"selectors":[{"matchPIDs":null,"matchArgs":[{"index":6,"operator":"Prefix","values":["HTTP"]}],"matchActions":null,"matchReturnArgs":null,"matchBinaries":null,"matchNamespaces":null,"matchNamespaceChanges":null,"matchCapabilities":null,"matchCapabilityChanges":null}]}
creating TracepointSpec: {"subsystem":"syscalls","event":"sys_enter_read","args":[{"index":5,"type":"fd","sizeArgIndex":0,"returnCopy":false},{"index":6,"type":"char_buf","sizeArgIndex":8,"returnCopy":false},{"index":7,"type":"size_t","sizeArgIndex":0,"returnCopy":false}],"selectors":[{"matchPIDs":null,"matchArgs":null,"matchActions":null,"matchReturnArgs":null,"matchBinaries":null,"matchNamespaces":null,"matchNamespaceChanges":null,"matchCapabilities":null,"matchCapabilityChanges":null}]}
after creating genericTracepointTable:  [{"Info":{"Subsys":"syscalls","Event":"sys_enter_write","Format":{"Name":"sys_enter_write","ID":640,"Fields":[{"FieldStr":"unsigned short common_type","Field":null,"Offset":0,"Size":2,"IsSigned":false},{"FieldStr":"unsigned char common_flags","Field":null,"Offset":2,"Size":1,"IsSigned":false},{"FieldStr":"unsigned char common_preempt_count","Field":null,"Offset":3,"Size":1,"IsSigned":false},{"FieldStr":"int common_pid","Field":null,"Offset":4,"Size":4,"IsSigned":true},{"FieldStr":"int __syscall_nr","Field":null,"Offset":8,"Size":4,"IsSigned":true},{"FieldStr":"unsigned int fd","Field":null,"Offset":16,"Size":8,"IsSigned":false},{"FieldStr":"const char * buf","Field":null,"Offset":24,"Size":8,"IsSigned":false},{"FieldStr":"size_t count","Field":null,"Offset":32,"Size":8,"IsSigned":false}]}},"Spec":{"subsystem":"syscalls","event":"sys_enter_read","args":[{"index":5,"type":"fd","sizeArgIndex":0,"returnCopy":false},{"index":6,"type":"char_buf","sizeArgIndex":8,"returnCopy":false},{"index":7,"type":"size_t","sizeArgIndex":0,"returnCopy":false}],"selectors":[{"matchPIDs":null,"matchArgs":null,"matchActions":null,"matchReturnArgs":null,"matchBinaries":null,"matchNamespaces":null,"matchNamespaceChanges":null,"matchCapabilities":null,"matchCapabilityChanges":null}]}},{"Info":{"Subsys":"syscalls","Event":"sys_enter_read","Format":{"Name":"sys_enter_read","ID":642,"Fields":[{"FieldStr":"unsigned short common_type","Field":null,"Offset":0,"Size":2,"IsSigned":false},{"FieldStr":"unsigned char common_flags","Field":null,"Offset":2,"Size":1,"IsSigned":false},{"FieldStr":"unsigned char common_preempt_count","Field":null,"Offset":3,"Size":1,"IsSigned":false},{"FieldStr":"int common_pid","Field":null,"Offset":4,"Size":4,"IsSigned":true},{"FieldStr":"int __syscall_nr","Field":null,"Offset":8,"Size":4,"IsSigned":true},{"FieldStr":"unsigned int fd","Field":null,"Offset":16,"Size":8,"IsSigned":false},{"FieldStr":"char * buf","Field":null,"Offset":24,"Size":8,"IsSigned":false},{"FieldStr":"size_t count","Field":null,"Offset":32,"Size":8,"IsSigned":false}]}},"Spec":{"subsystem":"syscalls","event":"sys_enter_read","args":[{"index":5,"type":"fd","sizeArgIndex":0,"returnCopy":false},{"index":6,"type":"char_buf","sizeArgIndex":8,"returnCopy":false},{"index":7,"type":"size_t","sizeArgIndex":0,"returnCopy":false}],"selectors":[{"matchPIDs":null,"matchArgs":null,"matchActions":null,"matchReturnArgs":null,"matchBinaries":null,"matchNamespaces":null,"matchNamespaceChanges":null,"matchCapabilities":null,"matchCapabilityChanges":null}]}}]
time="2022-11-30T10:25:45Z" level=info msg="BTF file: using metadata file" metadata=/sys/kernel/btf/vmlinux

as you can see, after loops, we got genericTracepointTable.arr:

[{"Info":{"Subsys":"syscalls","Event":"sys_enter_write","Format":{"Name":"sys_enter_write","ID":640,"Fields":[{"FieldStr":"unsigned short common_type","Field":null,"Offset":0,"Size":2,"IsSigned":false},{"FieldStr":"unsigned char common_flags","Field":null,"Offset":2,"Size":1,"IsSigned":false},{"FieldStr":"unsigned char common_preempt_count","Field":null,"Offset":3,"Size":1,"IsSigned":false},{"FieldStr":"int common_pid","Field":null,"Offset":4,"Size":4,"IsSigned":true},{"FieldStr":"int __syscall_nr","Field":null,"Offset":8,"Size":4,"IsSigned":true},{"FieldStr":"unsigned int fd","Field":null,"Offset":16,"Size":8,"IsSigned":false},{"FieldStr":"const char * buf","Field":null,"Offset":24,"Size":8,"IsSigned":false},{"FieldStr":"size_t count","Field":null,"Offset":32,"Size":8,"IsSigned":false}]}},"Spec":{"subsystem":"syscalls","event":"sys_enter_read","args":[{"index":5,"type":"fd","sizeArgIndex":0,"returnCopy":false},{"index":6,"type":"char_buf","sizeArgIndex":8,"returnCopy":false},{"index":7,"type":"size_t","sizeArgIndex":0,"returnCopy":false}],"selectors":[{"matchPIDs":null,"matchArgs":null,"matchActions":null,"matchReturnArgs":null,"matchBinaries":null,"matchNamespaces":null,"matchNamespaceChanges":null,"matchCapabilities":null,"matchCapabilityChanges":null}]}},{"Info":{"Subsys":"syscalls","Event":"sys_enter_read","Format":{"Name":"sys_enter_read","ID":642,"Fields":[{"FieldStr":"unsigned short common_type","Field":null,"Offset":0,"Size":2,"IsSigned":false},{"FieldStr":"unsigned char common_flags","Field":null,"Offset":2,"Size":1,"IsSigned":false},{"FieldStr":"unsigned char common_preempt_count","Field":null,"Offset":3,"Size":1,"IsSigned":false},{"FieldStr":"int common_pid","Field":null,"Offset":4,"Size":4,"IsSigned":true},{"FieldStr":"int __syscall_nr","Field":null,"Offset":8,"Size":4,"IsSigned":true},{"FieldStr":"unsigned int fd","Field":null,"Offset":16,"Size":8,"IsSigned":false},{"FieldStr":"char * buf","Field":null,"Offset":24,"Size":8,"IsSigned":false},{"FieldStr":"size_t count","Field":null,"Offset":32,"Size":8,"IsSigned":false}]}},"Spec":{"subsystem":"syscalls","event":"sys_enter_read","args":[{"index":5,"type":"fd","sizeArgIndex":0,"returnCopy":false},{"index":6,"type":"char_buf","sizeArgIndex":8,"returnCopy":false},{"index":7,"type":"size_t","sizeArgIndex":0,"returnCopy":false}],"selectors":[{"matchPIDs":null,"matchArgs":null,"matchActions":null,"matchReturnArgs":null,"matchBinaries":null,"matchNamespaces":null,"matchNamespaceChanges":null,"matchCapabilities":null,"matchCapabilityChanges":null}]}}]

the Spec of each element of genericTracepointTable.arr becomes sys_enter_read, the last one in tracepoints array.

[Y-dc]

@Y-dc I see that golanglint-ci is used https://github.com/cilium/tetragon/blob/main/.golangci.yml But it doesn't use exportloopref check which should catch such bugs.

I suggest to enable all linters which covers bugs section https://golangci-lint.run/usage/linters/

@anjmao i think that is better!

[Y-dc]

Linters exportloopref: https://github.com/kyoh86/exportloopref

[kkourt]

Nice catch, thanks!

Enabling the linter also makes sense to me!