Skip to content

Commit 2a66416

Browse files
committed
fix(epbf): fix behavior of has_prefix() and add strncmp()
There are 2 implementations for `has_prefix()`, one as a macro and one as a function (depending on the compiler version). They both behave differently in cases where no difference was found in `n` iterations - the macro returns 1 (true), which is an issue if the prefix is longer than `n`, while the function returns 0 (false) which is wrong if the prefix and string are identical. An additional check was added to `has_prefix()` to account for prefixes longer than `n`, while still returning true when the strings are equal. Additionally, an `strncmp()` macro and function were added, because most uses of `has_prefix()` become clearer when using `strncmp()`. Only a single usage of `has_prefix()` (in `filter_file_path()`) cannot use `strncmp()` because the prefix length can only be determined at runtime which makes usage of an unrolled loop impossible. Instead, it must use `has_prefix()` which accounts for prefixes shorter than the string, unlike `strncmp()`.
1 parent 6d174d0 commit 2a66416

File tree

3 files changed

+45
-12
lines changed

3 files changed

+45
-12
lines changed

pkg/ebpf/c/capture_filtering.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,7 @@ statfunc bool filter_file_path(void *ctx, void *filter_map, struct file *file)
6262

6363
has_filter = true;
6464

65-
if (has_prefix(filter_p->path, (char *) &path_buf->buf, MAX_PATH_PREF_SIZE)) {
65+
if (has_prefix(filter_p->path, (char *) &path_buf->buf, MAX_PATH_PREF_SIZE - 1)) {
6666
filter_match = true;
6767
break;
6868
}

pkg/ebpf/c/common/common.h

Lines changed: 35 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,8 @@ statfunc const char *get_device_name(struct device *dev)
3434
({ \
3535
int rc = 1; \
3636
char *pre = p, *str = s; \
37-
_Pragma("unroll") for (int z = 0; z < n; pre++, str++, z++) \
37+
int z; \
38+
_Pragma("unroll") for (z = 0; z < n; pre++, str++, z++) \
3839
{ \
3940
if (!*pre) { \
4041
rc = 1; \
@@ -44,6 +45,23 @@ statfunc const char *get_device_name(struct device *dev)
4445
break; \
4546
} \
4647
} \
48+
/* if prefix is longer than n, return 0 */ \
49+
if (z == n && *pre) \
50+
rc = 0; \
51+
rc; \
52+
})
53+
54+
#define strncmp(str1, str2, n) \
55+
({ \
56+
int rc = 0; \
57+
char *s1 = str1, *s2 = str2; \
58+
_Pragma("unroll") for (int z = 0; z < n; s1++, s2++, z++) \
59+
{ \
60+
if (*s1 != *s2 || *s1 == '\0' || *s2 == '\0') { \
61+
rc = (unsigned char) *s1 - (unsigned char) *s2; \
62+
break; \
63+
} \
64+
} \
4765
rc; \
4866
})
4967

@@ -61,7 +79,22 @@ static __inline int has_prefix(char *prefix, char *str, int n)
6179
}
6280
}
6381

64-
// prefix is too long
82+
// if prefix is longer than n, return 0
83+
if (i == n && *prefix)
84+
return 0;
85+
86+
// prefix and string are identical
87+
return 1;
88+
}
89+
90+
static __inline int strncmp(char *str1, char *str2, int n)
91+
{
92+
int i;
93+
#pragma unroll
94+
for (i = 0; i < n; str1++, str2++, i++) {
95+
if (*str1 != *str2 || *str1 == '\0' || *str2 == '\0')
96+
return (unsigned char) *str1 - (unsigned char) *str2;
97+
}
6598
return 0;
6699
}
67100

pkg/ebpf/c/tracee.bpf.c

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1925,7 +1925,7 @@ send_bpf_perf_attach(program_data_t *p, struct file *bpf_prog_file, struct file
19251925
bpf_probe_read_kernel_str(
19261926
&class_system, REQUIRED_SYSTEM_LENGTH, BPF_CORE_READ(tp_class, system));
19271927
class_system[REQUIRED_SYSTEM_LENGTH - 1] = '\0';
1928-
if (has_prefix("syscalls", class_system, REQUIRED_SYSTEM_LENGTH)) {
1928+
if (strncmp("syscalls", class_system, REQUIRED_SYSTEM_LENGTH - 1) == 0) {
19291929
is_syscall_tracepoint = true;
19301930
}
19311931

@@ -3140,7 +3140,7 @@ statfunc int capture_file_write(struct pt_regs *ctx, u32 event_id, bool is_buf)
31403140
// otherwise the capture will overwrite itself.
31413141
int pid = 0;
31423142
void *path_buf = get_path_str_cached(file);
3143-
if (path_buf != NULL && has_prefix("/dev/null", (char *) path_buf, 10)) {
3143+
if (path_buf != NULL && strncmp("/dev/null", (char *) path_buf, 9) == 0) {
31443144
pid = p.event->context.task.pid;
31453145
}
31463146

@@ -6329,16 +6329,16 @@ statfunc int net_l7_is_http(struct __sk_buff *skb, u32 l7_off)
63296329
}
63306330

63316331
// check if HTTP response
6332-
if (has_prefix("HTTP/", http_min_str, 6)) {
6332+
if (strncmp("HTTP/", http_min_str, 5) == 0) {
63336333
return proto_http_resp;
63346334
}
63356335

63366336
// check if HTTP request
6337-
if (has_prefix("GET ", http_min_str, 5) ||
6338-
has_prefix("POST ", http_min_str, 6) ||
6339-
has_prefix("PUT ", http_min_str, 5) ||
6340-
has_prefix("DELETE ", http_min_str, 8) ||
6341-
has_prefix("HEAD ", http_min_str, 6)) {
6337+
if (strncmp("GET ", http_min_str, 4) == 0 ||
6338+
strncmp("POST ", http_min_str, 5) == 0 ||
6339+
strncmp("PUT ", http_min_str, 4) == 0 ||
6340+
strncmp("DELETE ", http_min_str, 7) == 0 ||
6341+
strncmp("HEAD ", http_min_str, 5) == 0) {
63426342
return proto_http_req;
63436343
}
63446344

@@ -6901,7 +6901,7 @@ int tracepoint__exec_test(struct bpf_raw_tracepoint_args *ctx)
69016901
return -1;
69026902
struct file *file = get_file_ptr_from_bprm(bprm);
69036903
void *file_path = get_path_str(__builtin_preserve_access_index(&file->f_path));
6904-
if (file_path == NULL || !has_prefix("/tmp/test", file_path, 9))
6904+
if (file_path == NULL || strncmp("/tmp/test", file_path, 9) != 0)
69056905
return 0;
69066906

69076907
// Submit all test events

0 commit comments

Comments
 (0)