tetragon: assorted fixes

bpf/process/bpf_generic_kprobe.c

@@ -100,7 +100,7 @@ generic_kprobe_process_filter(void *ctx)
 	if (ret == PFILTER_CONTINUE)
 		tail_call(ctx, &kprobe_calls, TAIL_CALL_FILTER);
 	else if (ret == PFILTER_ACCEPT)
-		tail_call(ctx, &kprobe_calls, 0);
+		tail_call(ctx, &kprobe_calls, TAIL_CALL_SETUP);
 	/* If filter does not accept drop it. Ideally we would
 	 * log error codes for later review, TBD.
 	 */

bpf/process/bpf_generic_lsm_core.c

@@ -71,7 +71,7 @@ generic_lsm_process_filter(void *ctx)
 	if (ret == PFILTER_CONTINUE)
 		tail_call(ctx, &lsm_calls, TAIL_CALL_FILTER);
 	else if (ret == PFILTER_ACCEPT)
-		tail_call(ctx, &lsm_calls, 0);
+		tail_call(ctx, &lsm_calls, TAIL_CALL_SETUP);
 	return PFILTER_REJECT;
 }
 

bpf/process/bpf_generic_uprobe.c

@@ -74,7 +74,7 @@ generic_uprobe_process_filter(void *ctx)
 	if (ret == PFILTER_CONTINUE)
 		tail_call(ctx, &uprobe_calls, TAIL_CALL_FILTER);
 	else if (ret == PFILTER_ACCEPT)
-		tail_call(ctx, &uprobe_calls, 0);
+		tail_call(ctx, &uprobe_calls, TAIL_CALL_SETUP);
 	/* If filter does not accept drop it. Ideally we would
 	 * log error codes for later review, TBD.
 	 */

bpf/process/generic_calls.h

@@ -216,20 +216,43 @@ generic_process_event_and_setup(struct pt_regs *ctx, struct bpf_map_def *tailcal
 	return generic_process_event(ctx, tailcals);
 }
 
+#if defined GENERIC_KPROBE || defined GENERIC_LSM
+FUNC_INLINE void
+do_override_action(__s32 error)
+{
+	__s32 *error_p;
+	__u64 id;
+
+	id = get_current_pid_tgid();
+
+	/*
+	 * TODO: this should not happen, it means that the override
+	 * program was not executed for some reason, we should do
+	 * warning in here
+	 */
+	error_p = map_lookup_elem(&override_tasks, &id);
+	if (error_p)
+		*error_p = error;
+	else
+		map_update_elem(&override_tasks, &id, &error, BPF_ANY);
+}
+#else
+#define do_override_action(error)
+#endif
+
 FUNC_LOCAL __u32
 do_action(void *ctx, __u32 i, struct selector_action *actions, bool *post)
 {
 	int signal __maybe_unused = FGS_SIGKILL;
 	int action = actions->act[i];
 	struct msg_generic_kprobe *e;
-	__s32 error, *error_p;
+	__s32 error __maybe_unused;
 	int fdi, namei;
 	int newfdi, oldfdi;
 	int socki;
 	int argi __maybe_unused;
 	int err = 0;
 	int zero = 0;
-	__u64 id;
 
 	e = map_lookup_elem(&process_call_heap, &zero);
 	if (!e)
@@ -293,18 +316,7 @@ do_action(void *ctx, __u32 i, struct selector_action *actions, bool *post)
 		break;
 	case ACTION_OVERRIDE:
 		error = actions->act[++i];
-		id = get_current_pid_tgid();
-
-		/*
-		 * TODO: this should not happen, it means that the override
-		 * program was not executed for some reason, we should do
-		 * warning in here
-		 */
-		error_p = map_lookup_elem(&override_tasks, &id);
-		if (error_p)
-			*error_p = error;
-		else
-			map_update_elem(&override_tasks, &id, &error, BPF_ANY);
+		do_override_action(error);
 		break;
 	case ACTION_GETURL:
 	case ACTION_DNSLOOKUP:
@@ -558,59 +570,57 @@ FUNC_INLINE int generic_retkprobe(void *ctx, struct bpf_map_def *calls, unsigned
 // msg_generic_hdr structure.
 FUNC_INLINE int generic_process_filter(void)
 {
+	int selectors, pass, curr, zero = 0;
 	struct execve_map_value *enter;
 	struct msg_generic_kprobe *msg;
 	struct msg_execve_key *current;
 	struct msg_selector_data *sel;
-	int curr, zero = 0;
 	bool walker = 0;
-	__u32 ppid;
+	__u32 ppid, *f;
 
 	msg = map_lookup_elem(&process_call_heap, &zero);
 	if (!msg)
 		return 0;
 
 	enter = event_find_curr(&ppid, &walker);
-	if (enter) {
-		int selectors, pass;
-		__u32 *f = map_lookup_elem(&filter_map, &msg->idx);
+	if (!enter)
+		return PFILTER_CURR_NOT_FOUND;
 
-		if (!f)
-			return PFILTER_ERROR;
+	f = map_lookup_elem(&filter_map, &msg->idx);
+	if (!f)
+		return PFILTER_ERROR;
 
-		sel = &msg->sel;
-		current = &msg->current;
+	sel = &msg->sel;
+	current = &msg->current;
 
-		curr = sel->curr;
-		if (curr > MAX_SELECTORS)
-			return process_filter_done(sel, enter, current);
+	curr = sel->curr;
+	if (curr > MAX_SELECTORS)
+		return process_filter_done(sel, enter, current);
 
-		selectors = f[0];
-		/* If no selectors accept process */
-		if (!selectors) {
-			sel->pass = true;
-			return process_filter_done(sel, enter, current);
-		}
+	selectors = f[0];
+	/* If no selectors accept process */
+	if (!selectors) {
+		sel->pass = true;
+		return process_filter_done(sel, enter, current);
+	}
 
-		/* If we get here with reference to uninitialized selector drop */
-		if (selectors <= curr)
-			return process_filter_done(sel, enter, current);
-
-		pass = selector_process_filter(f, curr, enter, msg);
-		if (pass) {
-			/* Verify lost that msg is not null here so recheck */
-			asm volatile("%[curr] &= 0x1f;\n"
-				     : [curr] "+r"(curr));
-			sel->active[curr] = true;
-			sel->active[SELECTORS_ACTIVE] = true;
-			sel->pass |= true;
-		}
-		sel->curr++;
-		if (sel->curr > selectors)
-			return process_filter_done(sel, enter, current);
-		return PFILTER_CONTINUE; /* will iterate to the next selector */
+	/* If we get here with reference to uninitialized selector drop */
+	if (selectors <= curr)
+		return process_filter_done(sel, enter, current);
+
+	pass = selector_process_filter(f, curr, enter, msg);
+	if (pass) {
+		/* Verify lost that msg is not null here so recheck */
+		asm volatile("%[curr] &= 0x1f;\n"
+			     : [curr] "+r"(curr));
+		sel->active[curr] = true;
+		sel->active[SELECTORS_ACTIVE] = true;
+		sel->pass |= true;
 	}
-	return PFILTER_CURR_NOT_FOUND;
+	sel->curr++;
+	if (sel->curr > selectors)
+		return process_filter_done(sel, enter, current);
+	return PFILTER_CONTINUE; /* will iterate to the next selector */
 }
 
 FUNC_INLINE int filter_args(struct msg_generic_kprobe *e, int selidx, bool is_entry)

bpf/process/pfilter.h

@@ -409,7 +409,7 @@ selector_process_filter(__u32 *f, __u32 index, struct execve_map_value *enter,
 	__u64 i;
 
 	/* Do binary filter first for selector index */
-	if (!match_binaries(index))
+	if (!match_binaries(index, enter))
 		return 0;
 
 	/* Find selector offset byte index */

bpf/process/types/basic.h

@@ -117,6 +117,7 @@ enum {
 };
 
 enum {
+	TAIL_CALL_SETUP = 0,
 	TAIL_CALL_PROCESS = 1,
 	TAIL_CALL_FILTER = 2,
 	TAIL_CALL_ARGS = 3,
@@ -1542,11 +1543,9 @@ struct {
 		});
 } tg_mb_paths SEC(".maps");
 
-FUNC_INLINE int match_binaries(__u32 selidx)
+FUNC_INLINE int match_binaries(__u32 selidx, struct execve_map_value *current)
 {
-	struct execve_map_value *current;
-	__u32 ppid;
-	bool walker, match = 0;
+	bool match = 0;
 	void *path_map;
 	__u8 *found_key;
 #ifdef __LARGE_BPF_PROG
@@ -1566,12 +1565,6 @@ FUNC_INLINE int match_binaries(__u32 selidx)
 		if (selector_options->op == op_filter_none)
 			return 1; // matchBinaries selector is empty <=> match
 
-		current = event_find_curr(&ppid, &walker);
-		if (!current) {
-			// this should not happen, it means that the process was missed when
-			// scanning /proc for process that started before and after tetragon
-			return 0;
-		}
 		if (current->bin.path_length < 0) {
 			// something wrong happened when copying the filename to execve_map
 			return 0;

pkg/observer/observer.go

@@ -26,6 +26,7 @@ import (
 	"github.com/cilium/tetragon/pkg/reader/notify"
 	"github.com/cilium/tetragon/pkg/sensors"
 	"github.com/cilium/tetragon/pkg/sensors/config/confmap"
+	"github.com/cilium/tetragon/pkg/strutils"
 	"github.com/prometheus/client_golang/prometheus"
 	dto "github.com/prometheus/client_model/go"
 
@@ -158,18 +159,6 @@ func perfBufferSize(perCPUBuffer int) int {
 	return nPages * pageSize
 }
 
-func sizeWithSuffix(size int) string {
-	suffix := [4]string{"", "K", "M", "G"}
-
-	i := 0
-	for size > 1024 && i < 3 {
-		size = size / 1024
-		i++
-	}
-
-	return fmt.Sprintf("%d%s", size, suffix[i])
-}
-
 func (k *Observer) getRBSize(cpus int) int {
 	var size int
 
@@ -184,8 +173,8 @@ func (k *Observer) getRBSize(cpus int) int {
 	cpuSize := perfBufferSize(size)
 	totalSize := cpuSize * cpus
 
-	k.log.WithField("percpu", sizeWithSuffix(cpuSize)).
-		WithField("total", sizeWithSuffix(totalSize)).
+	k.log.WithField("percpu", strutils.SizeWithSuffix(cpuSize)).
+		WithField("total", strutils.SizeWithSuffix(totalSize)).
 		Info("Perf ring buffer size (bytes)")
 	return size
 }
@@ -195,7 +184,7 @@ func (k *Observer) getRBQueueSize() int {
 	if size == 0 {
 		size = 65535
 	}
-	k.log.WithField("size", sizeWithSuffix(size)).
+	k.log.WithField("size", strutils.SizeWithSuffix(size)).
 		Info("Perf ring buffer events queue size (events)")
 	return size
 }

pkg/sensors/program/map.go

@@ -159,7 +159,11 @@ func DeleteGlobMap(name string) {
 //	...
 //	p.PinMap["mapX"] = &mapX
 func mapBuilder(name string, ty MapType, owner bool, lds ...*Program) *Map {
-	m := &Map{name, "", lds[0], Idle(), nil, MaxEntries{0, false}, MaxEntries{0, false}, ty, owner}
+	var prog *Program
+	if len(lds) != 0 {
+		prog = lds[0]
+	}
+	m := &Map{name, "", prog, Idle(), nil, MaxEntries{0, false}, MaxEntries{0, false}, ty, owner}
 	for _, ld := range lds {
 		ld.PinMap[name] = m
 	}

pkg/strutils/strutls.go

@@ -4,6 +4,7 @@
 package strutils
 
 import (
+	"fmt"
 	"strconv"
 	"strings"
 )
@@ -50,3 +51,15 @@ func ParseSize(str string) (int, error) {
 	// never reached
 	return 0, nil
 }
+
+func SizeWithSuffix(size int) string {
+	suffix := [4]string{"", "K", "M", "G"}
+
+	i := 0
+	for size > 1024 && i < 3 {
+		size = size / 1024
+		i++
+	}
+
+	return fmt.Sprintf("%d%s", size, suffix[i])
+}