Skip to content

Commit e427054

Browse files
author
Alexei Starovoitov
committed
Merge branch 'x86-fgraph-bpf-fix-orc-stack-unwind-from-return-probe'
Jiri Olsa says: ==================== x86/fgraph,bpf: Fix ORC stack unwind from return probe sending fix for ORC stack unwind issue reported in here [1], where the ORC unwinder won't go pass the return_to_handler function and we get no stacktrace. Sending fix for that together with unrelated stacktrace fix (patch 1), so the attached test can work properly. It's based on: https://git.kernel.org/pub/scm/linux/kernel/git/trace/linux-trace.git probes/for-next v1: https://lore.kernel.org/bpf/[email protected]/ v2: https://lore.kernel.org/bpf/[email protected]/ v3 changes: - fix assert condition in test thanks, jirka [1] https://lore.kernel.org/bpf/aObSyt3qOnS_BMcy@krava/ ==================== Acked-by: Steven Rostedt (Google) <[email protected]> Link: https://patch.msgid.link/[email protected] Signed-off-by: Alexei Starovoitov <[email protected]>
2 parents 44e8f13 + 3490d29 commit e427054

File tree

7 files changed

+251
-7
lines changed

7 files changed

+251
-7
lines changed

‎arch/x86/events/core.c‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2789,13 +2789,13 @@ perf_callchain_kernel(struct perf_callchain_entry_ctx *entry, struct pt_regs *re
27892789
return;
27902790
}
27912791

2792-
if (perf_callchain_store(entry, regs->ip))
2793-
return;
2794-
2795-
if (perf_hw_regs(regs))
2792+
if (perf_hw_regs(regs)) {
2793+
if (perf_callchain_store(entry, regs->ip))
2794+
return;
27962795
unwind_start(&state, current, regs, NULL);
2797-
else
2796+
} else {
27982797
unwind_start(&state, current, NULL, (void *)regs->sp);
2798+
}
27992799

28002800
for (; !unwind_done(&state); unwind_next_frame(&state)) {
28012801
addr = unwind_get_return_address(&state);

‎arch/x86/include/asm/ftrace.h‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,11 @@ arch_ftrace_get_regs(struct ftrace_regs *fregs)
5656
return &arch_ftrace_regs(fregs)->regs;
5757
}
5858

59+
#define arch_ftrace_partial_regs(regs) do { \
60+
regs->flags &= ~X86_EFLAGS_FIXED; \
61+
regs->cs = __KERNEL_CS; \
62+
} while (0)
63+
5964
#define arch_ftrace_fill_perf_regs(fregs, _regs) do { \
6065
(_regs)->ip = arch_ftrace_regs(fregs)->regs.ip; \
6166
(_regs)->sp = arch_ftrace_regs(fregs)->regs.sp; \

‎arch/x86/kernel/ftrace_64.S‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -354,12 +354,17 @@ SYM_CODE_START(return_to_handler)
354354
UNWIND_HINT_UNDEFINED
355355
ANNOTATE_NOENDBR
356356

357+
/* Restore return_to_handler value that got eaten by previous ret instruction. */
358+
subq $8, %rsp
359+
UNWIND_HINT_FUNC
360+
357361
/* Save ftrace_regs for function exit context */
358362
subq $(FRAME_SIZE), %rsp
359363

360364
movq %rax, RAX(%rsp)
361365
movq %rdx, RDX(%rsp)
362366
movq %rbp, RBP(%rsp)
367+
movq %rsp, RSP(%rsp)
363368
movq %rsp, %rdi
364369

365370
call ftrace_return_to_handler
@@ -368,7 +373,8 @@ SYM_CODE_START(return_to_handler)
368373
movq RDX(%rsp), %rdx
369374
movq RAX(%rsp), %rax
370375

371-
addq $(FRAME_SIZE), %rsp
376+
addq $(FRAME_SIZE) + 8, %rsp
377+
372378
/*
373379
* Jump back to the old return address. This cannot be JMP_NOSPEC rdi
374380
* since IBT would demand that contain ENDBR, which simply isn't so for

‎include/linux/ftrace.h‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -193,6 +193,10 @@ static __always_inline struct pt_regs *ftrace_get_regs(struct ftrace_regs *fregs
193193
#if !defined(CONFIG_HAVE_DYNAMIC_FTRACE_WITH_ARGS) || \
194194
defined(CONFIG_HAVE_FTRACE_REGS_HAVING_PT_REGS)
195195

196+
#ifndef arch_ftrace_partial_regs
197+
#define arch_ftrace_partial_regs(regs) do {} while (0)
198+
#endif
199+
196200
static __always_inline struct pt_regs *
197201
ftrace_partial_regs(struct ftrace_regs *fregs, struct pt_regs *regs)
198202
{
@@ -202,7 +206,11 @@ ftrace_partial_regs(struct ftrace_regs *fregs, struct pt_regs *regs)
202206
* Since arch_ftrace_get_regs() will check some members and may return
203207
* NULL, we can not use it.
204208
*/
205-
return &arch_ftrace_regs(fregs)->regs;
209+
regs = &arch_ftrace_regs(fregs)->regs;
210+
211+
/* Allow arch specific updates to regs. */
212+
arch_ftrace_partial_regs(regs);
213+
return regs;
206214
}
207215

208216
#endif /* !CONFIG_HAVE_DYNAMIC_FTRACE_WITH_ARGS || CONFIG_HAVE_FTRACE_REGS_HAVING_PT_REGS */

‎tools/testing/selftests/bpf/prog_tests/stacktrace_ips.c‎

Lines changed: 150 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,150 @@
1+
// SPDX-License-Identifier: GPL-2.0
2+
#include <test_progs.h>
3+
#include "stacktrace_ips.skel.h"
4+
5+
#ifdef __x86_64__
6+
static int check_stacktrace_ips(int fd, __u32 key, int cnt, ...)
7+
{
8+
__u64 ips[PERF_MAX_STACK_DEPTH];
9+
struct ksyms *ksyms = NULL;
10+
int i, err = 0;
11+
va_list args;
12+
13+
/* sorted by addr */
14+
ksyms = load_kallsyms_local();
15+
if (!ASSERT_OK_PTR(ksyms, "load_kallsyms_local"))
16+
return -1;
17+
18+
/* unlikely, but... */
19+
if (!ASSERT_LT(cnt, PERF_MAX_STACK_DEPTH, "check_max"))
20+
return -1;
21+
22+
err = bpf_map_lookup_elem(fd, &key, ips);
23+
if (err)
24+
goto out;
25+
26+
/*
27+
* Compare all symbols provided via arguments with stacktrace ips,
28+
* and their related symbol addresses.t
29+
*/
30+
va_start(args, cnt);
31+
32+
for (i = 0; i < cnt; i++) {
33+
unsigned long val;
34+
struct ksym *ksym;
35+
36+
val = va_arg(args, unsigned long);
37+
ksym = ksym_search_local(ksyms, ips[i]);
38+
if (!ASSERT_OK_PTR(ksym, "ksym_search_local"))
39+
break;
40+
ASSERT_EQ(ksym->addr, val, "stack_cmp");
41+
}
42+
43+
va_end(args);
44+
45+
out:
46+
free_kallsyms_local(ksyms);
47+
return err;
48+
}
49+
50+
static void test_stacktrace_ips_kprobe_multi(bool retprobe)
51+
{
52+
LIBBPF_OPTS(bpf_kprobe_multi_opts, opts,
53+
.retprobe = retprobe
54+
);
55+
LIBBPF_OPTS(bpf_test_run_opts, topts);
56+
struct stacktrace_ips *skel;
57+
58+
skel = stacktrace_ips__open_and_load();
59+
if (!ASSERT_OK_PTR(skel, "stacktrace_ips__open_and_load"))
60+
return;
61+
62+
if (!skel->kconfig->CONFIG_UNWINDER_ORC) {
63+
test__skip();
64+
goto cleanup;
65+
}
66+
67+
skel->links.kprobe_multi_test = bpf_program__attach_kprobe_multi_opts(
68+
skel->progs.kprobe_multi_test,
69+
"bpf_testmod_stacktrace_test", &opts);
70+
if (!ASSERT_OK_PTR(skel->links.kprobe_multi_test, "bpf_program__attach_kprobe_multi_opts"))
71+
goto cleanup;
72+
73+
trigger_module_test_read(1);
74+
75+
load_kallsyms();
76+
77+
check_stacktrace_ips(bpf_map__fd(skel->maps.stackmap), skel->bss->stack_key, 4,
78+
ksym_get_addr("bpf_testmod_stacktrace_test_3"),
79+
ksym_get_addr("bpf_testmod_stacktrace_test_2"),
80+
ksym_get_addr("bpf_testmod_stacktrace_test_1"),
81+
ksym_get_addr("bpf_testmod_test_read"));
82+
83+
cleanup:
84+
stacktrace_ips__destroy(skel);
85+
}
86+
87+
static void test_stacktrace_ips_raw_tp(void)
88+
{
89+
__u32 info_len = sizeof(struct bpf_prog_info);
90+
LIBBPF_OPTS(bpf_test_run_opts, topts);
91+
struct bpf_prog_info info = {};
92+
struct stacktrace_ips *skel;
93+
__u64 bpf_prog_ksym = 0;
94+
int err;
95+
96+
skel = stacktrace_ips__open_and_load();
97+
if (!ASSERT_OK_PTR(skel, "stacktrace_ips__open_and_load"))
98+
return;
99+
100+
if (!skel->kconfig->CONFIG_UNWINDER_ORC) {
101+
test__skip();
102+
goto cleanup;
103+
}
104+
105+
skel->links.rawtp_test = bpf_program__attach_raw_tracepoint(
106+
skel->progs.rawtp_test,
107+
"bpf_testmod_test_read");
108+
if (!ASSERT_OK_PTR(skel->links.rawtp_test, "bpf_program__attach_raw_tracepoint"))
109+
goto cleanup;
110+
111+
/* get bpf program address */
112+
info.jited_ksyms = ptr_to_u64(&bpf_prog_ksym);
113+
info.nr_jited_ksyms = 1;
114+
err = bpf_prog_get_info_by_fd(bpf_program__fd(skel->progs.rawtp_test),
115+
&info, &info_len);
116+
if (!ASSERT_OK(err, "bpf_prog_get_info_by_fd"))
117+
goto cleanup;
118+
119+
trigger_module_test_read(1);
120+
121+
load_kallsyms();
122+
123+
check_stacktrace_ips(bpf_map__fd(skel->maps.stackmap), skel->bss->stack_key, 2,
124+
bpf_prog_ksym,
125+
ksym_get_addr("bpf_trace_run2"));
126+
127+
cleanup:
128+
stacktrace_ips__destroy(skel);
129+
}
130+
131+
static void __test_stacktrace_ips(void)
132+
{
133+
if (test__start_subtest("kprobe_multi"))
134+
test_stacktrace_ips_kprobe_multi(false);
135+
if (test__start_subtest("kretprobe_multi"))
136+
test_stacktrace_ips_kprobe_multi(true);
137+
if (test__start_subtest("raw_tp"))
138+
test_stacktrace_ips_raw_tp();
139+
}
140+
#else
141+
static void __test_stacktrace_ips(void)
142+
{
143+
test__skip();
144+
}
145+
#endif
146+
147+
void test_stacktrace_ips(void)
148+
{
149+
__test_stacktrace_ips();
150+
}

‎tools/testing/selftests/bpf/progs/stacktrace_ips.c‎

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
// SPDX-License-Identifier: GPL-2.0
2+
// Copyright (c) 2018 Facebook
3+
4+
#include <vmlinux.h>
5+
#include <bpf/bpf_helpers.h>
6+
#include <bpf/bpf_tracing.h>
7+
8+
#ifndef PERF_MAX_STACK_DEPTH
9+
#define PERF_MAX_STACK_DEPTH 127
10+
#endif
11+
12+
typedef __u64 stack_trace_t[PERF_MAX_STACK_DEPTH];
13+
14+
struct {
15+
__uint(type, BPF_MAP_TYPE_STACK_TRACE);
16+
__uint(max_entries, 16384);
17+
__type(key, __u32);
18+
__type(value, stack_trace_t);
19+
} stackmap SEC(".maps");
20+
21+
extern bool CONFIG_UNWINDER_ORC __kconfig __weak;
22+
23+
/*
24+
* This function is here to have CONFIG_UNWINDER_ORC
25+
* used and added to object BTF.
26+
*/
27+
int unused(void)
28+
{
29+
return CONFIG_UNWINDER_ORC ? 0 : 1;
30+
}
31+
32+
__u32 stack_key;
33+
34+
SEC("kprobe.multi")
35+
int kprobe_multi_test(struct pt_regs *ctx)
36+
{
37+
stack_key = bpf_get_stackid(ctx, &stackmap, 0);
38+
return 0;
39+
}
40+
41+
SEC("raw_tp/bpf_testmod_test_read")
42+
int rawtp_test(void *ctx)
43+
{
44+
/* Skip ebpf program entry in the stack. */
45+
stack_key = bpf_get_stackid(ctx, &stackmap, 0);
46+
return 0;
47+
}
48+
49+
char _license[] SEC("license") = "GPL";

‎tools/testing/selftests/bpf/test_kmods/bpf_testmod.c‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -417,6 +417,30 @@ noinline int bpf_testmod_fentry_test11(u64 a, void *b, short c, int d,
417417
return a + (long)b + c + d + (long)e + f + g + h + i + j + k;
418418
}
419419

420+
noinline void bpf_testmod_stacktrace_test(void)
421+
{
422+
/* used for stacktrace test as attach function */
423+
asm volatile ("");
424+
}
425+
426+
noinline void bpf_testmod_stacktrace_test_3(void)
427+
{
428+
bpf_testmod_stacktrace_test();
429+
asm volatile ("");
430+
}
431+
432+
noinline void bpf_testmod_stacktrace_test_2(void)
433+
{
434+
bpf_testmod_stacktrace_test_3();
435+
asm volatile ("");
436+
}
437+
438+
noinline void bpf_testmod_stacktrace_test_1(void)
439+
{
440+
bpf_testmod_stacktrace_test_2();
441+
asm volatile ("");
442+
}
443+
420444
int bpf_testmod_fentry_ok;
421445

422446
noinline ssize_t
@@ -497,6 +521,8 @@ bpf_testmod_test_read(struct file *file, struct kobject *kobj,
497521
21, 22, 23, 24, 25, 26) != 231)
498522
goto out;
499523

524+
bpf_testmod_stacktrace_test_1();
525+
500526
bpf_testmod_fentry_ok = 1;
501527
out:
502528
return -EIO; /* always fail */

0 commit comments

Comments
 (0)