bpf: bump prepend_name underlying buffer size 4096

.github/workflows/vmtests.yml

@@ -67,7 +67,7 @@ jobs:
         matrix:
            kernel:
               # renovate: datasource=docker depName=quay.io/lvh-images/kernel-images
-              - 'rhel8-20240620.102250'
+              - 'rhel8.9-20240806.173325'
               # renovate: datasource=docker depName=quay.io/lvh-images/kernel-images
               - 'bpf-next-20240624.013140'
               # renovate: datasource=docker depName=quay.io/lvh-images/kernel-images

Makefile

@@ -264,8 +264,10 @@ test: tester-progs tetragon-bpf ## Run Go tests.
 tester-progs: ## Compile helper programs for unit testing.
 	$(MAKE) -C $(TESTER_PROGS_DIR)
 
+## bpf-test: ## run BPF tests.
+## bpf-test BPFGOTESTFLAGS="-v": ## run BPF tests with verbose.
 .PHONY: bpf-test
-bpf-test: ## Run BPF tests.
+bpf-test:
 	$(MAKE) -C ./bpf run-test
 
 .PHONY: verify

bpf/lib/process.h

@@ -273,12 +273,7 @@ struct msg_k8s {
 #define BINARY_PATH_MAX_LEN 256
 
 struct heap_exe {
-	// because of verifier limitations, this has to be 2 * 256 bytes while 256
-	// should be theoretically sufficient, and actually is, in unit tests.
-	char buf[BINARY_PATH_MAX_LEN * 2];
-	// offset points to the start of the path in the above buffer. Use offset to
-	// read the path in the buffer since it's written from the end.
-	char *off;
+	char buf[BINARY_PATH_MAX_LEN];
 	__u32 len;
 	__u32 error;
 }; // All fields aligned so no 'packed' attribute.

bpf/process/bpf_execve_event.c

@@ -180,11 +180,22 @@ read_exe(struct task_struct *task, struct heap_exe *exe)
 	struct file *file = BPF_CORE_READ(task, mm, exe_file);
 	struct path *path = __builtin_preserve_access_index(&file->f_path);
 
-	exe->len = BINARY_PATH_MAX_LEN;
-	exe->off = (char *)&exe->buf;
-	exe->off = __d_path_local(path, exe->off, (int *)&exe->len, (int *)&exe->error);
-	if (exe->len > 0)
-		exe->len = BINARY_PATH_MAX_LEN - exe->len;
+	// we need to walk the complete 4096 len dentry in order to have an accurate
+	// matching on the prefix operators, even if we only keep a subset of that
+	char *buffer;
+
+	buffer = d_path_local(path, (int *)&exe->len, (int *)&exe->error);
+	if (!buffer)
+		return 0;
+
+	// buffer used by d_path_local can contain up to MAX_BUF_LEN i.e. 4096 we
+	// only keep the first 255 chars for our needs (we sacrifice one char to the
+	// verifier for the > 0 check)
+	if (exe->len > 255)
+		exe->len = 255;
+	asm volatile("%[len] &= 0xff;\n"
+		     : [len] "+r"(exe->len));
+	probe_read(exe->buf, exe->len, buffer);
 
 	return exe->len;
 }
@@ -364,7 +375,7 @@ execve_send(void *ctx)
 #ifdef __LARGE_BPF_PROG
 		// read from proc exe stored at execve time
 		if (event->exe.len <= BINARY_PATH_MAX_LEN) {
-			curr->bin.path_length = probe_read(curr->bin.path, event->exe.len, event->exe.off);
+			curr->bin.path_length = probe_read(curr->bin.path, event->exe.len, event->exe.buf);
 			if (curr->bin.path_length == 0)
 				curr->bin.path_length = event->exe.len;
 		}

bpf/process/bpf_process_event.h

@@ -12,14 +12,14 @@
 #define ENAMETOOLONG 36 /* File name too long */
 
 #define MATCH_BINARIES_PATH_MAX_LENGTH 256
-#define MAX_BUF_LEN		       256
+#define MAX_BUF_LEN		       4096
 
 struct buffer_heap_map_value {
-	// Buffer is twice the needed size because of the verifier. In prepend_name
-	// unit tests, the verifier figures out that 255 is enough and that the
-	// buffer_offset will not overflow, but in the real use-case it looks like
-	// it's forgetting about that.
-	unsigned char buf[MAX_BUF_LEN * 2];
+	// Buffer need a bit more space here  because of the verifier. In
+	// prepend_name unit tests, the verifier figures out that MAX_BUF_LEN is
+	// enough and that the buffer_offset will not overflow, but in the real
+	// use-case it looks like it's forgetting about that.
+	unsigned char buf[MAX_BUF_LEN + 256];
 };
 
 struct {
@@ -119,15 +119,13 @@ prepend_name(char *buf, char **bufptr, int *buflen, const char *name, u32 namele
 
 	*buflen -= (namelen + write_slash);
 
-	// This will not happen as buffer_offset cannot be above 256 and namelen is
-	// bound to 255. Needed to make the verifier happy in older kernels.
 	if (namelen + write_slash > buffer_offset)
 		return -ENAMETOOLONG;
 
 	buffer_offset -= (namelen + write_slash);
 
 	// This will never happen. buffer_offset is the diff of the initial buffer pointer
-	// with the current buffer pointer. This will be at max 256 bytes (similar to the initial
+	// with the current buffer pointer. This will be at max 4096 bytes (similar to the initial
 	// size).
 	// Needed to bound that for probe_read call.
 	if (buffer_offset >= MAX_BUF_LEN)

bpf/tests/prepend_name_test.c

@@ -13,7 +13,7 @@
 
 char _license[] __attribute__((section("license"), used)) = "Dual BSD/GPL";
 
-#define TEST_MAX_BUF_LEN 256
+#define TEST_MAX_BUF_LEN 4096
 #define NAME_MAX	 255
 
 struct test_prepend_name_state_map_value {

bpf/tests/prepend_name_test.go

@@ -18,7 +18,7 @@ import (
 
 const (
 	// those constants must be synchronized with the BPF code
-	MAX_BUF_LEN                 = 256
+	MAX_BUF_LEN                 = 4096
 	NAME_MAX                    = 255
 	testPrependNameStateMapName = "test_prepend_name_state_map"
 	programName                 = "test_prepend_name"
@@ -232,44 +232,67 @@ func Test_PrependName(t *testing.T) {
 		assert.Equal(t, "sr/bin/cat", state.BufferToString())
 	})
 
-	SetupLongDentry := func() string {
-		// length is 239
-		const longDentry = "pizza_tomato_mozzarella_basil_pizza_tomato_mozzarella_basil_pizza_tomato_mozzarella_basil_pizza_tomato_mozzarella_basil_pizza_tomato_mozzarella_basil_pizza_tomato_mozzarella_basil_pizza_tomato_mozzarella_basil_pizza_tomato_mozzarella_basil"
+	// length is 239
+	const longDentry = "pizza_tomato_mozzarella_basil_pizza_tomato_mozzarella_basil_pizza_tomato_mozzarella_basil_pizza_tomato_mozzarella_basil_pizza_tomato_mozzarella_basil_pizza_tomato_mozzarella_basil_pizza_tomato_mozzarella_basil_pizza_tomato_mozzarella_basil"
+
+	t.Run("MaxSizeBufMedium", func(t *testing.T) {
+		const bufsize = 256
+		state.ResetStateWithBuflen(bufsize)
 
 		err = state.UpdateDentry(longDentry)
 		assert.NoError(t, err)
 		code := runPrependName()
 		assert.Equal(t, 0, code)
 		assert.Equal(t, "/"+longDentry, state.BufferToString())
-		return longDentry
-	}
-
-	t.Run("MaxSizeBufFull", func(t *testing.T) {
-		state.ResetStateWithBuflen(MAX_BUF_LEN)
-
-		longDentry := SetupLongDentry()
 
 		// length is 15, so 239 + 15 + 2 slash chars = 256
 		err = state.UpdateDentry("favorite_recipe")
 		assert.NoError(t, err)
-		code := runPrependName()
+		code = runPrependName()
 		assert.Equal(t, 0, code)
 		assert.Equal(t, "/favorite_recipe"+"/"+longDentry, state.BufferToString())
-		assert.Equal(t, MAX_BUF_LEN, len(state.BufferToString()))
+		assert.Equal(t, bufsize, len(state.BufferToString()))
 	})
 
-	t.Run("MaxSizeBufTooSmall", func(t *testing.T) {
+	t.Run("MaxSizeBufFull", func(t *testing.T) {
+		maxDentry := strings.Repeat("a", NAME_MAX)
 		state.ResetStateWithBuflen(MAX_BUF_LEN)
 
-		longDentry := SetupLongDentry()
+		var expectedState string
+		// (len("/") + 255) * 16 = 4096
+		for range 16 {
+			err = state.UpdateDentry(maxDentry)
+			assert.NoError(t, err)
+			code := runPrependName()
+			assert.Equal(t, 0, code)
+			expectedState += "/" + maxDentry
+			assert.Equal(t, expectedState, state.BufferToString())
+		}
+	})
+
+	t.Run("MaxSizeBufTooSmall", func(t *testing.T) {
+		largeDentry := strings.Repeat("a", 240)
+		state.ResetStateWithBuflen(MAX_BUF_LEN)
 
-		// length is 16 with the "s" of "recipes", so 240 + 15 + 2 slash chars = 257
-		err = state.UpdateDentry("favorite_recipes")
+		var expectedState string
+		// (len("/") + 240) * 16 = 3856
+		for range 16 {
+			err = state.UpdateDentry(largeDentry)
+			assert.NoError(t, err)
+			code := runPrependName()
+			assert.Equal(t, 0, code)
+			expectedState = "/" + largeDentry + expectedState
+			assert.Equal(t, expectedState, state.BufferToString())
+		}
+		// at this stage, there should be 240 chars left in the buf which leaves
+		// no space for the remaining root slash character
+		err = state.UpdateDentry(largeDentry)
 		assert.NoError(t, err)
 		code := runPrependName()
 		assert.Equal(t, -int(unix.ENAMETOOLONG), code)
-		assert.Equal(t, "favorite_recipes"+"/"+longDentry, state.BufferToString())
-		assert.Equal(t, MAX_BUF_LEN, len(state.BufferToString()))
+		// note that I intentionally don't add the '/' char
+		expectedState = largeDentry + expectedState
+		assert.Equal(t, expectedState, state.BufferToString())
 	})
 
 	t.Run("MaxSizeBufNormalUse", func(t *testing.T) {

pkg/sensors/tracing/kprobe_test.go

@@ -3886,6 +3886,55 @@ func TestKprobeMatchBinaries(t *testing.T) {
 	})
 }
 
+func TestKprobeMatchBinariesPrefixLargePath(t *testing.T) {
+	if !kernels.EnableLargeProgs() {
+		t.Skip()
+	}
+
+	// create a large temporary directory path
+	tmpDir := t.TempDir()
+	targetBinLargePath := tmpDir
+	// add (255 + 1) * 15 = 3840 chars to the path
+	// max is 4096 and we want to leave some space for the tmpdir + others
+	for range 15 {
+		targetBinLargePath += "/" + strings.Repeat("a", unix.NAME_MAX)
+	}
+	err := os.MkdirAll(targetBinLargePath, 0755)
+	require.NoError(t, err)
+
+	// copy the binary into it
+	targetBinLargePath += "/true"
+	fileExec, err := exec.LookPath("true")
+	require.NoError(t, err)
+	err = exec.Command("cp", fileExec, targetBinLargePath).Run()
+	require.NoError(t, err)
+
+	var doneWG, readyWG sync.WaitGroup
+	defer doneWG.Wait()
+
+	ctx, cancel := context.WithTimeout(context.Background(), tus.Conf().CmdWaitTime)
+	defer cancel()
+
+	createCrdFile(t, getMatchBinariesCrd("Prefix", []string{tmpDir}))
+
+	obs, err := observertesthelper.GetDefaultObserverWithFile(t, ctx, testConfigFile, tus.Conf().TetragonLib, observertesthelper.WithMyPid())
+	if err != nil {
+		t.Fatalf("GetDefaultObserverWithFile error: %s", err)
+	}
+	observertesthelper.LoopEvents(ctx, t, &doneWG, &readyWG, obs)
+	readyWG.Wait()
+
+	if err := exec.Command(targetBinLargePath).Run(); err != nil {
+		t.Fatalf("failed to run true: %s", err)
+	}
+
+	checker := ec.NewUnorderedEventChecker(ec.NewProcessKprobeChecker("").
+		WithProcess(ec.NewProcessChecker().WithBinary(sm.Full(targetBinLargePath))).
+		WithFunctionName(sm.Full("fd_install")))
+	err = jsonchecker.JsonTestCheck(t, checker)
+	assert.NoError(t, err)
+}
+
 // matchBinariesPerfringTest checks that the matchBinaries do correctly
 // filter the events i.e. it checks that no other events appear.
 func matchBinariesPerfringTest(t *testing.T, operator string, values []string) {