diff --git a/bpf/process/string_maps.h b/bpf/process/string_maps.h index 1a908a29dbd..a27d6eff0de 100644 --- a/bpf/process/string_maps.h +++ b/bpf/process/string_maps.h @@ -138,7 +138,7 @@ struct { __uint(value_size, 512); } string_maps_ro_zero SEC(".maps"); -#define STRING_PREFIX_MAX_LENGTH 128 +#define STRING_PREFIX_MAX_LENGTH 256 struct string_prefix_lpm_trie { __u32 prefixlen; @@ -171,7 +171,7 @@ struct { #ifdef __LARGE_BPF_PROG #define STRING_POSTFIX_MAX_MATCH_LENGTH STRING_POSTFIX_MAX_LENGTH #else -#define STRING_POSTFIX_MAX_MATCH_LENGTH 96 +#define STRING_POSTFIX_MAX_MATCH_LENGTH 95 #endif struct string_postfix_lpm_trie { diff --git a/bpf/process/types/basic.h b/bpf/process/types/basic.h index 676321c2159..4b2d49cf293 100644 --- a/bpf/process/types/basic.h +++ b/bpf/process/types/basic.h @@ -775,17 +775,25 @@ filter_char_buf_prefix(struct selector_arg_filter *filter, char *arg_str, uint a int zero = 0; addrmap = map_lookup_elem(&string_prefix_maps, &map_idx); - if (!addrmap) + if (!addrmap || !arg_len) return 0; - if (arg_len > STRING_PREFIX_MAX_LENGTH || !arg_len) - return 0; + // If the string to check is longer than the prefix map allows, then only check the longest + // substring that the map allows. + if (arg_len >= STRING_PREFIX_MAX_LENGTH) + arg_len = STRING_PREFIX_MAX_LENGTH - 1; arg = (struct string_prefix_lpm_trie *)map_lookup_elem(&string_prefix_maps_heap, &zero); if (!arg) return 0; arg->prefixlen = arg_len * 8; // prefix is in bits + + // Force the verifier to recheck the arg_len after register spilling on 4.19. + asm volatile("%[arg_len] &= %[mask] ;\n" + : [arg_len] "+r"(arg_len) + : [mask] "i"(STRING_PREFIX_MAX_LENGTH - 1)); + probe_read(arg->data, arg_len & (STRING_PREFIX_MAX_LENGTH - 1), arg_str); __u8 *pass = map_lookup_elem(addrmap, arg); @@ -793,8 +801,11 @@ filter_char_buf_prefix(struct selector_arg_filter *filter, char *arg_str, uint a return !!pass; } +// Define a mask for the maximum path length on Linux. +#define PATH_MASK (4096 - 1) + static inline __attribute__((always_inline)) void -copy_reverse(__u8 *dest, uint len, __u8 *src) +copy_reverse(__u8 *dest, uint len, __u8 *src, uint offset) { uint i; @@ -813,8 +824,8 @@ copy_reverse(__u8 *dest, uint len, __u8 *src) // Alternative (prettier) fixes resulted in a confused verifier // unfortunately. for (i = 0; i < (STRING_POSTFIX_MAX_MATCH_LENGTH - 1); i++) { - dest[i & STRING_POSTFIX_MAX_MASK] = src[(len - 1 - i) & STRING_POSTFIX_MAX_MASK]; - if (len == (i + 1)) + dest[i & STRING_POSTFIX_MAX_MASK] = src[(len + offset - 1 - i) & PATH_MASK]; + if (len + offset == (i + 1)) return; } } @@ -825,21 +836,22 @@ filter_char_buf_postfix(struct selector_arg_filter *filter, char *arg_str, uint void *addrmap; __u32 map_idx = *(__u32 *)&filter->value; struct string_postfix_lpm_trie *arg; + uint orig_len = arg_len; int zero = 0; addrmap = map_lookup_elem(&string_postfix_maps, &map_idx); - if (!addrmap) + if (!addrmap || !arg_len) return 0; - if (arg_len > STRING_POSTFIX_MAX_LENGTH || !arg_len) - return 0; + if (arg_len >= STRING_POSTFIX_MAX_MATCH_LENGTH) + arg_len = STRING_POSTFIX_MAX_MATCH_LENGTH - 1; arg = (struct string_postfix_lpm_trie *)map_lookup_elem(&string_postfix_maps_heap, &zero); if (!arg) return 0; arg->prefixlen = arg_len * 8; // prefix is in bits - copy_reverse(arg->data, arg_len, (__u8 *)arg_str); + copy_reverse(arg->data, arg_len, (__u8 *)arg_str, orig_len - arg_len); __u8 *pass = map_lookup_elem(addrmap, arg); diff --git a/pkg/btf/validation.go b/pkg/btf/validation.go index 6989caf6529..9b9c6a74946 100644 --- a/pkg/btf/validation.go +++ b/pkg/btf/validation.go @@ -219,6 +219,16 @@ func typesCompatible(specTy string, kernelTy string) bool { case "struct module *": return true } + case "sock": + switch kernelTy { + case "struct sock *": + return true + } + case "skb": + switch kernelTy { + case "struct sk_buff *": + return true + } } return false diff --git a/pkg/selectors/selectors.go b/pkg/selectors/selectors.go index 6d6de5c3ba2..eac2f2fbda0 100644 --- a/pkg/selectors/selectors.go +++ b/pkg/selectors/selectors.go @@ -49,7 +49,7 @@ const ( stringMapsKeyIncSize = 24 StringMapsNumSubMaps = 6 MaxStringMapsSize = 6*stringMapsKeyIncSize + 1 - StringPrefixMaxLength = 128 + StringPrefixMaxLength = 256 StringPostfixMaxLength = 128 ) diff --git a/pkg/sensors/tracing/kprobe_test.go b/pkg/sensors/tracing/kprobe_test.go index 38b51be6258..8244980b12e 100644 --- a/pkg/sensors/tracing/kprobe_test.go +++ b/pkg/sensors/tracing/kprobe_test.go @@ -1040,6 +1040,25 @@ func TestKprobeObjectFilterPrefixOpen(t *testing.T) { testKprobeObjectFiltered(t, readHook, getOpenatChecker(t, dir), false, dir, false, syscall.O_RDWR, 0x770) } +func TestKprobeObjectFilterPrefixOpenSuperLong(t *testing.T) { + pidStr := strconv.Itoa(int(observertesthelper.GetMyPid())) + dir := t.TempDir() + readHook := testKprobeObjectFilterPrefixOpenHook(pidStr, dir) + firstDir := dir + "/testfoo" + longDir := firstDir + "/1234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890" + + "1234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890" + + "123456789012345678901234567890123456789012345678901234567890" + if err := os.Mkdir(firstDir, 0755); err != nil { + t.Logf("Mkdir %s failed: %s\n", firstDir, err) + t.Skip() + } + if err := os.Mkdir(longDir, 0755); err != nil { + t.Logf("Mkdir %s failed: %s\n", longDir, err) + t.Skip() + } + testKprobeObjectFiltered(t, readHook, getOpenatChecker(t, longDir), false, longDir, false, syscall.O_RDWR, 0x770) +} + func TestKprobeObjectFilterPrefixOpenMount(t *testing.T) { pidStr := strconv.Itoa(int(observertesthelper.GetMyPid())) dir := t.TempDir() @@ -1227,6 +1246,50 @@ func TestKprobeObjectPostfixOpenWithNull(t *testing.T) { testKprobeObjectPostfixOpen(t, true) } +func TestKprobeObjectPostfixOpenSuperLong(t *testing.T) { + pidStr := strconv.Itoa(int(observertesthelper.GetMyPid())) + dir := t.TempDir() + readHook := ` +apiVersion: cilium.io/v1alpha1 +kind: TracingPolicy +metadata: + name: "sys-read" +spec: + kprobes: + - call: "sys_openat" + return: false + syscall: true + args: + - index: 0 + type: int + - index: 1 + type: "string" + - index: 2 + type: "int" + selectors: + - matchPIDs: + - operator: In + followForks: true + values: + - ` + pidStr + ` + matchArgs: + - index: 1 + operator: "Postfix" + values: + - "` + testKprobeObjectPostfixOpenFileName(false) + `" +` + + longDir := dir + "/1234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890" + + "1234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890" + + "123456789012345678901234567890123456789012345678901234567890" + if err := os.Mkdir(longDir, 0755); err != nil { + t.Logf("Mkdir %s failed: %s\n", longDir, err) + t.Skip() + } + + testKprobeObjectFiltered(t, readHook, getOpenatChecker(t, longDir), false, longDir, false, syscall.O_RDWR, 0x770) +} + func testKprobeObjectFilterModeOpenHook(pidStr string, mode int, valueFmt string) string { return ` apiVersion: cilium.io/v1alpha1 diff --git a/pkg/sensors/tracing/selectors.go b/pkg/sensors/tracing/selectors.go index 770238a6200..f374a2da5ab 100644 --- a/pkg/sensors/tracing/selectors.go +++ b/pkg/sensors/tracing/selectors.go @@ -412,7 +412,7 @@ func populateStringPrefixFilterMap( innerSpec := &ebpf.MapSpec{ Name: innerName, Type: ebpf.LPMTrie, - KeySize: 4 + selectors.StringPrefixMaxLength, // NB: KernelLpmTrieStringPrefix consists of 32bit prefix and 128 byte data + KeySize: 4 + selectors.StringPrefixMaxLength, // NB: KernelLpmTrieStringPrefix consists of 32bit prefix and 256 byte data ValueSize: uint32(1), MaxEntries: maxEntries, Flags: bpf.BPF_F_NO_PREALLOC,