Skip to content

Commit

Permalink
fix(epbf): fix behavior of has_prefix() and add strncmp()
Browse files Browse the repository at this point in the history
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()`.
  • Loading branch information
oshaked1 committed Nov 24, 2024
1 parent 6d174d0 commit a0f80bd
Show file tree
Hide file tree
Showing 3 changed files with 45 additions and 12 deletions.
2 changes: 1 addition & 1 deletion pkg/ebpf/c/capture_filtering.h
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ statfunc bool filter_file_path(void *ctx, void *filter_map, struct file *file)

has_filter = true;

if (has_prefix(filter_p->path, (char *) &path_buf->buf, MAX_PATH_PREF_SIZE)) {
if (has_prefix(filter_p->path, (char *) &path_buf->buf, MAX_PATH_PREF_SIZE-1)) {
filter_match = true;
break;
}
Expand Down
37 changes: 35 additions & 2 deletions pkg/ebpf/c/common/common.h
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,8 @@ statfunc const char *get_device_name(struct device *dev)
({ \
int rc = 1; \
char *pre = p, *str = s; \
_Pragma("unroll") for (int z = 0; z < n; pre++, str++, z++) \
int z; \
_Pragma("unroll") for (z = 0; z < n; pre++, str++, z++) \
{ \
if (!*pre) { \
rc = 1; \
Expand All @@ -44,6 +45,23 @@ statfunc const char *get_device_name(struct device *dev)
break; \
} \
} \
/* if prefix is longer than n, return 0 */ \
if (z == n && *pre) \
rc = 0; \
rc; \
})

#define strncmp(str1, str2, n) \
({ \
int rc = 0; \
char *s1 = str1, *s2 = str2; \
_Pragma("unroll") for (int z = 0; z < n; s1++, s2++, z++) \
{ \
if (*s1 != *s2 || *s1 == '\0' || *s2 == '\0') { \
rc = (unsigned char) *s1 - (unsigned char) *s2; \
break; \
} \
} \
rc; \
})

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

// prefix is too long
// if prefix is longer than n, return 0
if (i == n && *prefix)
return 0;

// prefix and string are identical
return 1;
}

static __inline int strncmp(char *str1, char *str2, int n)
{
int i;
#pragma unroll
for (i = 0; i < n; str1++, str2++, i++) {
if (*str1 != *str2 || *str1 == '\0' || *str2 == '\0')
return (unsigned char) *str1 - (unsigned char) *str2;
}
return 0;
}

Expand Down
18 changes: 9 additions & 9 deletions pkg/ebpf/c/tracee.bpf.c
Original file line number Diff line number Diff line change
Expand Up @@ -1925,7 +1925,7 @@ send_bpf_perf_attach(program_data_t *p, struct file *bpf_prog_file, struct file
bpf_probe_read_kernel_str(
&class_system, REQUIRED_SYSTEM_LENGTH, BPF_CORE_READ(tp_class, system));
class_system[REQUIRED_SYSTEM_LENGTH - 1] = '\0';
if (has_prefix("syscalls", class_system, REQUIRED_SYSTEM_LENGTH)) {
if (strncmp("syscalls", class_system, REQUIRED_SYSTEM_LENGTH-1) == 0) {
is_syscall_tracepoint = true;
}

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

Expand Down Expand Up @@ -6329,16 +6329,16 @@ statfunc int net_l7_is_http(struct __sk_buff *skb, u32 l7_off)
}

// check if HTTP response
if (has_prefix("HTTP/", http_min_str, 6)) {
if (strncmp("HTTP/", http_min_str, 5) == 0) {
return proto_http_resp;
}

// check if HTTP request
if (has_prefix("GET ", http_min_str, 5) ||
has_prefix("POST ", http_min_str, 6) ||
has_prefix("PUT ", http_min_str, 5) ||
has_prefix("DELETE ", http_min_str, 8) ||
has_prefix("HEAD ", http_min_str, 6)) {
if (strncmp("GET ", http_min_str, 4) == 0 ||
strncmp("POST ", http_min_str, 5) == 0 ||
strncmp("PUT ", http_min_str, 4) == 0 ||
strncmp("DELETE ", http_min_str, 7) == 0 ||
strncmp("HEAD ", http_min_str, 5) == 0) {
return proto_http_req;
}

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

// Submit all test events
Expand Down

0 comments on commit a0f80bd

Please sign in to comment.