matchBinaries improvements

bpf/lib/generic.h

@@ -70,7 +70,7 @@ struct names_map_key {
 
 struct {
 	__uint(type, BPF_MAP_TYPE_HASH);
-	__uint(max_entries, 64);
+	__uint(max_entries, 256); /* maximum number of binary names for all matchBinary selectors */
 	__type(key, struct names_map_key);
 	__type(value, __u32);
 } names_map SEC(".maps");

bpf/lib/process.h

@@ -104,6 +104,7 @@
 #define __ASM_ARGSBUFFER 976
 #define ARGSBUFFERMASK	 (ARGSBUFFER - 1)
 #define MAXARGMASK	 (MAXARG - 1)
+#define PATHNAME_SIZE	 256
 
 /* Task flags */
 #ifndef PF_KTHREAD
@@ -316,7 +317,7 @@ struct {
 
 struct execve_heap {
 	union {
-		char pathname[256];
+		char pathname[PATHNAME_SIZE];
 		char maxpath[4096];
 	};
 };

bpf/process/bpf_execve_event.c

@@ -94,8 +94,7 @@ event_args_builder(void *ctx, struct msg_execve_event *event)
 }
 
 static inline __attribute__((always_inline)) uint32_t
-event_filename_builder(void *ctx, struct msg_process *curr, __u32 curr_pid,
-		       __u32 flags, __u32 bin, void *filename)
+event_filename_builder(void *ctx, struct msg_process *curr, __u32 curr_pid, __u32 flags, void *filename)
 {
 	struct execve_heap *heap;
 	int64_t size = 0;
@@ -131,15 +130,20 @@ event_filename_builder(void *ctx, struct msg_process *curr, __u32 curr_pid,
 	curr->ktime = ktime_get_ns();
 	curr->size = size + offsetof(struct msg_process, args);
 
+	// skip binaries check for long (> 255) filenames for now
+	if (flags & EVENT_DATA_FILENAME)
+		return 0;
+
 	heap = map_lookup_elem(&execve_heap, &zero);
 	if (!heap)
-		return bin;
+		return 0;
 
-	probe_read_str(heap->pathname, 255, filename);
+	memset(heap->pathname, 0, PATHNAME_SIZE);
+	probe_read_str(heap->pathname, size, filename);
 	value = map_lookup_elem(&names_map, heap->pathname);
 	if (value)
 		return *value;
-	return bin;
+	return 0;
 }
 
 __attribute__((section("tracepoint/sys_execve"), used)) int
@@ -149,7 +153,6 @@ event_execve(struct sched_execve_args *ctx)
 	struct msg_execve_event *event;
 	struct execve_map_value *parent;
 	struct msg_process *execve;
-	uint32_t binary = 0;
 	bool walker = 0;
 	__u32 zero = 0;
 	__u32 pid;
@@ -172,16 +175,13 @@ event_execve(struct sched_execve_args *ctx)
 	parent = event_find_parent();
 	if (parent) {
 		event->parent = parent->key;
-		binary = parent->binary;
 	} else {
 		event_minimal_parent(event, task);
 	}
 
 	execve = &event->process;
 	fileoff = ctx->filename & 0xFFFF;
-	binary = event_filename_builder(ctx, execve, pid, EVENT_EXECVE, binary,
-					(char *)ctx + fileoff);
-	event->binary = binary;
+	event->binary = event_filename_builder(ctx, execve, pid, EVENT_EXECVE, (char *)ctx + fileoff);
 
 	event_args_builder(ctx, event);
 	compiler_barrier();

bpf/process/types/basic.h

@@ -83,12 +83,6 @@ struct selector_action {
 	__u32 act[];
 };
 
-struct selector_binary_filter {
-	__u32 arglen;
-	__u32 op;
-	__u32 index[4];
-};
-
 struct selector_arg_filter {
 	__u32 arglen;
 	__u32 index;
@@ -930,14 +924,33 @@ static inline __attribute__((always_inline)) size_t type_to_min_size(int type,
 
 #define INDEX_MASK 0x3ff
 
+/*
+ * For matchBinaries we use two maps:
+ * 1. names_map: global (for all sensors) keeps a mapping from names -> ids
+ * 2. sel_names_map: per-sensor: keeps a mapping from id -> selector val
+ *
+ * At exec time, we check names_map and set ->binary in execve_map equal to
+ * the id stored in names_map. Assuming the binary name exists in the map,
+ * otherwise binary is 0.
+ *
+ * When we check the selectors, use ->binary to index sel_names_map and decide
+ * whether the selector matches or not.
+ */
+struct {
+	__uint(type, BPF_MAP_TYPE_HASH);
+	__uint(max_entries, 256);
+	__type(key, __u32);
+	__type(value, __u32);
+} sel_names_map SEC(".maps");
+
 static inline __attribute__((always_inline)) int
 selector_arg_offset(__u8 *f, struct msg_generic_kprobe *e, __u32 selidx)
 {
 	struct selector_arg_filter *filter;
-	struct selector_binary_filter *binary;
 	long seloff, argoff, pass;
-	__u32 index;
+	__u32 index, *op;
 	char *args;
+	__u32 max = 0xffffffff; // UINT32_MAX
 
 	seloff = 4; /* start of the relative offsets */
 	seloff += (selidx * 4); /* relative offset for this selector */
@@ -958,42 +971,37 @@ selector_arg_offset(__u8 *f, struct msg_generic_kprobe *e, __u32 selidx)
 	/* skip the matchCapabilityChanges by reading its length */
 	seloff += *(__u32 *)((__u64)f + (seloff & INDEX_MASK));
 
-	/* seloff must leave space for verifier to walk strings
-	 * so we set inside 4k maximum. Advance to binary matches.
-	 */
-	seloff &= INDEX_MASK;
-	binary = (struct selector_binary_filter *)&f[seloff];
-
-	/* Run binary name filters
-	 */
-	if (binary->op == op_filter_in) {
+	op = map_lookup_elem(&sel_names_map, &max);
+	if (op) {
 		struct execve_map_value *execve;
 		bool walker = 0;
-		__u32 ppid;
+		__u32 ppid, bin_key, *bin_val;
 
 		execve = event_find_curr(&ppid, &walker);
 		if (!execve)
 			return 0;
-		if (binary->index[0] != execve->binary &&
-		    binary->index[1] != execve->binary &&
-		    binary->index[2] != execve->binary &&
-		    binary->index[3] != execve->binary)
-			return 0;
-	}
 
-	/* Advance to matchArgs we use fixed size binary filters for now. It helps
-	 * the verifier and its still unclear how many entries are needed. At any
-	 * rate each entry is a uint32 now and we should really be able to pack
-	 * an entry into a byte which would give us 4x more entries.
-	 */
-	seloff += sizeof(struct selector_binary_filter);
-	if (seloff > 3800) {
-		return 0;
+		bin_key = execve->binary;
+		bin_val = map_lookup_elem(&sel_names_map, &bin_key);
+
+		/*
+		 * The following things may happen:
+		 * binary is not part of names_map, execve_map->binary will be `0` and `bin_val` will always be `0`
+		 * binary is part of `names_map`:
+		 *  if binary is not part of this selector, bin_val will be`0`
+		 *  if binary is part of this selector: `bin_val will be `!0`
+		 */
+		if (*op == op_filter_in) {
+			if (!bin_val)
+				return 0;
+		} else if (*op == op_filter_notin) {
+			if (bin_val)
+				return 0;
+		}
 	}
 
 	/* Making binary selectors fixes size helps on some kernels */
-	asm volatile("%[seloff] &= 0xfff;\n" ::[seloff] "+r"(seloff)
-		     :);
+	seloff &= INDEX_MASK;
 	filter = (struct selector_arg_filter *)&f[seloff];
 
 	if (filter->arglen <= 4) // no filters

pkg/selectors/kernel.go

@@ -632,37 +632,33 @@ func parseMatchCapabilityChanges(k *KernelSelectorState, actions []v1alpha1.Capa
 	return nil
 }
 
-func parseMatchBinary(k *KernelSelectorState, index uint32, b *v1alpha1.BinarySelector) error {
+func parseMatchBinary(k *KernelSelectorState, b *v1alpha1.BinarySelector) error {
 	op, err := selectorOp(b.Operator)
 	if err != nil {
-		return fmt.Errorf("matchpid error: %w", err)
+		return fmt.Errorf("matchBinary error: %w", err)
+	}
+	if op != selectorOpIn && op != selectorOpNotIn {
+		return fmt.Errorf("matchBinary error: Only In and NotIn operators are supported")
+	}
+	k.SetBinaryOp(op)
+	for _, s := range b.Values {
+		if len(s) > 255 {
+			return fmt.Errorf("matchBinary error: Binary names > 255 chars do not supported")
+		}
+		k.AddBinaryName(s)
 	}
-	WriteSelectorUint32(k, op)
-	WriteSelectorUint32(k, index)
-	WriteSelectorUint32(k, index)
-	WriteSelectorUint32(k, index)
-	WriteSelectorUint32(k, index)
 	return nil
 }
 
 func parseMatchBinaries(k *KernelSelectorState, binarys []v1alpha1.BinarySelector) error {
-	loff := AdvanceSelectorLength(k)
 	if len(binarys) > 1 {
 		return fmt.Errorf("Only support single binary selector")
-	} else if len(binarys) == 0 {
-		// To aid verifier we always zero in binary fields to allow
-		// BPF to assume the values exist.
-		WriteSelectorUint32(k, 0)
-		WriteSelectorUint32(k, 0)
-		WriteSelectorUint32(k, 0)
-		WriteSelectorUint32(k, 0)
-		WriteSelectorUint32(k, 0)
-	} else {
-		if err := parseMatchBinary(k, 1, &binarys[0]); err != nil {
+	}
+	for _, s := range binarys {
+		if err := parseMatchBinary(k, &s); err != nil {
 			return err
 		}
 	}
-	WriteSelectorLength(k, loff)
 	return nil
 }
 
@@ -715,7 +711,6 @@ func parseSelector(
 //	[matchCapabilities]
 //	[matchNamespaceChanges]
 //	[matchCapabilityChanges]
-//	[matchBinaries]
 //	[matchArgs]
 //	[matchActions]
 //
@@ -724,7 +719,6 @@ func parseSelector(
 // matchCapabilities := [length][CAx][CAy]...[CAn]
 // matchNamespaceChanges := [length][NCx][NCy]...[NCn]
 // matchCapabilityChanges := [length][CAx][CAy]...[CAn]
-// matchBinaries := [length][op][Index]...[Index]
 // matchArgs := [length][ARGx][ARGy]...[ARGn]
 // PIDn := [op][flags][nValues][v1]...[vn]
 // Argn := [index][op][valueGen]

pkg/selectors/kernel_test.go

@@ -397,11 +397,11 @@ func TestMultipleSelectorsExample(t *testing.T) {
 		expectedLen += 4
 	}
 
-	// vaue               absolute offset    explanation
+	// value               absolute offset    explanation
 	expU32Push(2)               // off: 0       number of selectors
 	expU32Push(8)               // off: 4       relative ofset of 1st selector (4 + 8 = 12)
-	expU32Push(104)             // off: 8       relative ofset of 2nd selector (8 + 104 = 112)
-	expU32Push(100)             // off: 12      selector1: length (100 + 12 = 112)
+	expU32Push(80)              // off: 8       relative ofset of 2nd selector (8 + 80 = 88)
+	expU32Push(76)              // off: 12      selector1: length (76 + 12 = 112)
 	expU32Push(24)              // off: 16      selector1: MatchPIDs: len
 	expU32Push(selectorOpNotIn) // off: 20      selector1: MatchPIDs[0]: op
 	expU32Push(0)               // off: 24      selector1: MatchPIDs[0]: flags
@@ -412,12 +412,6 @@ func TestMultipleSelectorsExample(t *testing.T) {
 	expU32Push(4)               // off: 44      selector1: MatchCapabilities: len
 	expU32Push(4)               // off: 48      selector1: MatchNamespaceChanges: len
 	expU32Push(4)               // off: 52      selector1: MatchCapabilityChanges: len
-	expU32Push(4 + 5*4)         // off: 56      selector1: matchBinaries: len
-	expU32Push(0)               // off: 60      selector1: matchBinaries: 0
-	expU32Push(0)               // off: 64      selector1: matchBinaries: 1
-	expU32Push(0)               // off: 68      selector1: matchBinaries: 2
-	expU32Push(0)               // off: 72      selector1: matchBinaries: 3
-	expU32Push(0)               // off: 76      selector1: matchBinaries: 4
 	expU32Push(28)              // off: 80      selector1: matchArgs: len
 	expU32Push(1)               // off: 84      selector1: matchArgs: arg0: index
 	expU32Push(selectorOpEQ)    // off: 88      selector1: matchArgs: arg0: operator
@@ -426,7 +420,7 @@ func TestMultipleSelectorsExample(t *testing.T) {
 	expU32Push(10)              // off: 100     selector1: matchArgs: arg0: val0: 10
 	expU32Push(20)              // off: 104     selector1: matchArgs: arg0: val1: 20
 	expU32Push(4)               // off: 108     selector1: matchActions: length
-	expU32Push(100)             // off: 112     selector2: length
+	expU32Push(76)              // off: 112     selector2: length
 	// ... everything else should be the same as selector1 ...
 
 	if bytes.Equal(expected[:expectedLen], b[:expectedLen]) == false {
@@ -443,11 +437,11 @@ func TestInitKernelSelectors(t *testing.T) {
 	}
 
 	expected_selsize_small := []byte{
-		0xfe, 0x00, 0x00, 0x00, // size = pids + binarys + args + actions + namespaces + capabilities  + 4
+		0xe6, 0x00, 0x00, 0x00, // size = pids + args + actions + namespaces + capabilities  + 4
 	}
 
 	expected_selsize_large := []byte{
-		0x1a, 0x01, 0x00, 0x00, // size = pids + binarys + args + actions + namespaces + namespacesChanges + capabilities + capabilityChanges + 4
+		0x02, 0x01, 0x00, 0x00, // size = pids + args + actions + namespaces + namespacesChanges + capabilities + capabilityChanges + 4
 	}
 
 	expected_filters := []byte{
@@ -531,17 +525,6 @@ func TestInitKernelSelectors(t *testing.T) {
 	}
 
 	expected_last := []byte{
-		// binaryNames header
-		24, 0x00, 0x00, 0x00, // size = sizeof(uint32) * 4
-
-		// binaryNames Ids, always has 4 to ease verify complexity and
-		// zeroes unused entries.
-		0x00, 0x00, 0x00, 0x00, // op
-		0x00, 0x00, 0x00, 0x00, // index0
-		0x00, 0x00, 0x00, 0x00, // index1
-		0x00, 0x00, 0x00, 0x00, // index2
-		0x00, 0x00, 0x00, 0x00, // index3
-
 		// arg header
 		54, 0x00, 0x00, 0x00, // size = sizeof(arg2) + sizeof(arg1) + 4