Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix x86 verification issues with custom labels #2886

Merged
merged 4 commits into from
Jun 20, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions bpf/unwinders/common.h
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
#ifndef __LINUX_PAGE_CONSTANTS_HACK__
#define __LINUX_PAGE_CONSTANTS_HACK__

// see https://gcc.gnu.org/onlinedocs/cpp/Stringizing.html#Stringizing
#define STR_HELPER(x) #x
#define STR(x) STR_HELPER(x)

// Values for x86_64 as of 6.0.18-200.
#define TOP_OF_KERNEL_STACK_PADDING 0
#define THREAD_SIZE_ORDER 2
Expand Down
36 changes: 30 additions & 6 deletions bpf/unwinders/go_runtime.h
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,7 @@ static __always_inline bool get_go_vdso_state(struct bpf_perf_event_data *ctx, s
// curg is nil, then g is either a system stack (called g0) or a signal handler
// g (gsignal). Neither one will ever have labels.
static __always_inline bool get_custom_labels(struct bpf_perf_event_data *ctx, struct go_runtime_offsets *offs, custom_labels_array_t *out) {
bpf_large_memzero((void *)out, sizeof(*out));
long res;
size_t m_ptr_addr = (size_t)get_m_ptr(ctx, offs);
if (!m_ptr_addr) {
Expand Down Expand Up @@ -203,9 +204,6 @@ static __always_inline bool get_custom_labels(struct bpf_perf_event_data *ctx, s
LOG("[warn] failed to read custom label: key too long");
continue;
}
// set the last element to 0 so we don't try to hash leftover unspecified bytes later on.
if (key_len < CUSTOM_LABEL_MAX_KEY_LEN)
lbl->key[key_len / 8] = 0;
res = bpf_probe_read(lbl->key, key_len, map_value->keys[i].str);
if (res) {
LOG("[warn] failed to read key for custom label: %d", res);
Expand All @@ -215,9 +213,35 @@ static __always_inline bool get_custom_labels(struct bpf_perf_event_data *ctx, s
LOG("[warn] failed to read custom label: value too long");
continue;
}
if (val_len < CUSTOM_LABEL_MAX_VAL_LEN)
lbl->val[val_len / 8] = 0;
res = bpf_probe_read(lbl->val, val_len, map_value->values[i].str);
// The following assembly statement is equivalent to:
// if (val_len > CUSTOM_LABEL_MAX_VAL_LEN)
// res = bpf_probe_read(lbl->val, val_len, map_value->values[i].str);
// else
// res = -1;
//
// We need to write this in assembly because the verifier doesn't understand
// that val_len has already been bounds-checked above, apparently
// because clang has spilled it to the stack rather than
// keeping it in a register.
// clang-format off
asm volatile(
// Note: this branch is never taken, but we
// need it to appease the verifier.
"if %2 > " STR(CUSTOM_LABEL_MAX_VAL_LEN) " goto bad%=\n"
"r1 = %1\n"
"r2 = %2\n"
"r3 = %3\n"
"call 4\n"
"%0 = r0\n"
"goto good%=\n"
"bad%=: %0 = -1\n"
"good%=:\n"
: "=r"(res)
: "r"(lbl->val), "r"(val_len), "r"(map_value->values[i].str)
// all r0-r5 are clobbered since we make a function call.
: "r0", "r1", "r2", "r3", "r4", "r5", "memory"
);
// clang-format on
if (res) {
LOG("[warn] failed to read value for custom label: %d", res);
continue;
Expand Down
Loading