Skip to content

Commit 0fce646

Browse files
committed
perf trace: Fix BPF loading failure (-E2BIG)
JIRA: https://issues.redhat.com/browse/RHEL-77936 upstream ======== commit 013eb04 Author: Howard Chu <[email protected]> Date: Thu Dec 12 18:30:47 2024 -0800 description =========== As reported by Namhyung Kim and acknowledged by Qiao Zhao (link: https://lore.kernel.org/linux-perf-users/[email protected]/), on certain machines, perf trace failed to load the BPF program into the kernel. The verifier runs perf trace's BPF program for up to 1 million instructions, returning an E2BIG error, whereas the perf trace BPF program should be much less complex than that. This patch aims to fix the issue described above. The E2BIG problem from clang-15 to clang-16 is cause by this line: } else if (size < 0 && size >= -6) { /* buffer */ Specifically this check: size < 0. seems like clang generates a cool optimization to this sign check that breaks things. Making 'size' s64, and use } else if ((int)size < 0 && size >= -6) { /* buffer */ Solves the problem. This is some Hogwarts magic. And the unbounded access of clang-12 and clang-14 (clang-13 works this time) is fixed by making variable 'aug_size' s64. As for this: -if (aug_size > TRACE_AUG_MAX_BUF) - aug_size = TRACE_AUG_MAX_BUF; +aug_size = args->args[index] > TRACE_AUG_MAX_BUF ? TRACE_AUG_MAX_BUF : args->args[index]; This makes the BPF skel generated by clang-18 work. Yes, new clangs introduce problems too. Sorry, I only know that it works, but I don't know how it works. I'm not an expert in the BPF verifier. I really hope this is not a kernel version issue, as that would make the test case (kernel_nr) * (clang_nr), a true horror story. I will test it on more kernel versions in the future. Fixes: 395d384: ("perf trace augmented_raw_syscalls: Add more check s to pass the verifier") Reported-by: Namhyung Kim <[email protected]> Signed-off-by: Howard Chu <[email protected]> Tested-by: Namhyung Kim <[email protected]> Link: https://lore.kernel.org/r/[email protected] Signed-off-by: Namhyung Kim <[email protected]> Signed-off-by: Michael Petlan <[email protected]>
1 parent 0df25d6 commit 0fce646

File tree

1 file changed

+4
-7
lines changed

1 file changed

+4
-7
lines changed

tools/perf/util/bpf_skel/augmented_raw_syscalls.bpf.c

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -431,9 +431,9 @@ static bool pid_filter__has(struct pids_filtered *pids, pid_t pid)
431431
static int augment_sys_enter(void *ctx, struct syscall_enter_args *args)
432432
{
433433
bool augmented, do_output = false;
434-
int zero = 0, size, aug_size, index,
435-
value_size = sizeof(struct augmented_arg) - offsetof(struct augmented_arg, value);
434+
int zero = 0, index, value_size = sizeof(struct augmented_arg) - offsetof(struct augmented_arg, value);
436435
u64 output = 0; /* has to be u64, otherwise it won't pass the verifier */
436+
s64 aug_size, size;
437437
unsigned int nr, *beauty_map;
438438
struct beauty_payload_enter *payload;
439439
void *arg, *payload_offset;
@@ -484,14 +484,11 @@ static int augment_sys_enter(void *ctx, struct syscall_enter_args *args)
484484
} else if (size > 0 && size <= value_size) { /* struct */
485485
if (!bpf_probe_read_user(((struct augmented_arg *)payload_offset)->value, size, arg))
486486
augmented = true;
487-
} else if (size < 0 && size >= -6) { /* buffer */
487+
} else if ((int)size < 0 && size >= -6) { /* buffer */
488488
index = -(size + 1);
489489
barrier_var(index); // Prevent clang (noticed with v18) from removing the &= 7 trick.
490490
index &= 7; // Satisfy the bounds checking with the verifier in some kernels.
491-
aug_size = args->args[index];
492-
493-
if (aug_size > TRACE_AUG_MAX_BUF)
494-
aug_size = TRACE_AUG_MAX_BUF;
491+
aug_size = args->args[index] > TRACE_AUG_MAX_BUF ? TRACE_AUG_MAX_BUF : args->args[index];
495492

496493
if (aug_size > 0) {
497494
if (!bpf_probe_read_user(((struct augmented_arg *)payload_offset)->value, aug_size, arg))

0 commit comments

Comments
 (0)