Skip to content

Commit aba0370

Browse files
committed
bpf, arm64: Remove garbage frame for struct_ops trampoline
JIRA: https://issues.redhat.com/browse/RHEL-85485 commit 87cb58a Author: Xu Kuohai <xukuohai@huawei.com> Date: Fri Oct 25 16:52:20 2024 +0800 bpf, arm64: Remove garbage frame for struct_ops trampoline The callsite layout for arm64 fentry is: mov x9, lr nop When a bpf prog is attached, the nop instruction is patched to a call to bpf trampoline: mov x9, lr bl <bpf trampoline> So two return addresses are passed to bpf trampoline: the return address for the traced function/prog, stored in x9, and the return address for the bpf trampoline itself, stored in lr. To obtain a full and accurate call stack, the bpf trampoline constructs two fake function frames using x9 and lr. However, struct_ops progs are invoked directly as function callbacks, meaning that x9 is not set as it is in the fentry callsite. In this case, the frame constructed using x9 is garbage. The following stack trace for struct_ops, captured by perf sampling, illustrates this issue, where tcp_ack+0x404 is a garbage frame: ffffffc0801a04b4 bpf_prog_50992e55a0f655a9_bpf_cubic_cong_avoid+0x98 (bpf_prog_50992e55a0f655a9_bpf_cubic_cong_avoid) ffffffc0801a228c [unknown] ([kernel.kallsyms]) // bpf trampoline ffffffd08d362590 tcp_ack+0x798 ([kernel.kallsyms]) // caller for bpf trampoline ffffffd08d3621fc tcp_ack+0x404 ([kernel.kallsyms]) // garbage frame ffffffd08d36452c tcp_rcv_established+0x4ac ([kernel.kallsyms]) ffffffd08d375c58 tcp_v4_do_rcv+0x1f0 ([kernel.kallsyms]) ffffffd08d378630 tcp_v4_rcv+0xeb8 ([kernel.kallsyms]) To fix it, construct only one frame using lr for struct_ops. The above stack trace also indicates that there is no kernel symbol for struct_ops bpf trampoline. This will be addressed in a follow-up patch. Fixes: efc9909 ("bpf, arm64: Add bpf trampoline for arm64") Signed-off-by: Xu Kuohai <xukuohai@huawei.com> Acked-by: Puranjay Mohan <puranjay@kernel.org> Tested-by: Puranjay Mohan <puranjay@kernel.org> Link: https://lore.kernel.org/r/20241025085220.533949-1-xukuohai@huaweicloud.com Signed-off-by: Alexei Starovoitov <ast@kernel.org> Signed-off-by: Viktor Malik <vmalik@redhat.com>
1 parent 17c9105 commit aba0370

File tree

1 file changed

+31
-16
lines changed

1 file changed

+31
-16
lines changed

arch/arm64/net/bpf_jit_comp.c

Lines changed: 31 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -2074,6 +2074,12 @@ static void restore_args(struct jit_ctx *ctx, int args_off, int nregs)
20742074
}
20752075
}
20762076

2077+
static bool is_struct_ops_tramp(const struct bpf_tramp_links *fentry_links)
2078+
{
2079+
return fentry_links->nr_links == 1 &&
2080+
fentry_links->links[0]->link.type == BPF_LINK_TYPE_STRUCT_OPS;
2081+
}
2082+
20772083
/* Based on the x86's implementation of arch_prepare_bpf_trampoline().
20782084
*
20792085
* bpf prog and function entry before bpf trampoline hooked:
@@ -2103,6 +2109,7 @@ static int prepare_trampoline(struct jit_ctx *ctx, struct bpf_tramp_image *im,
21032109
struct bpf_tramp_links *fmod_ret = &tlinks[BPF_TRAMP_MODIFY_RETURN];
21042110
bool save_ret;
21052111
__le32 **branches = NULL;
2112+
bool is_struct_ops = is_struct_ops_tramp(fentry);
21062113

21072114
/* trampoline stack layout:
21082115
* [ parent ip ]
@@ -2171,11 +2178,14 @@ static int prepare_trampoline(struct jit_ctx *ctx, struct bpf_tramp_image *im,
21712178
*/
21722179
emit_bti(A64_BTI_JC, ctx);
21732180

2174-
/* frame for parent function */
2175-
emit(A64_PUSH(A64_FP, A64_R(9), A64_SP), ctx);
2176-
emit(A64_MOV(1, A64_FP, A64_SP), ctx);
2181+
/* x9 is not set for struct_ops */
2182+
if (!is_struct_ops) {
2183+
/* frame for parent function */
2184+
emit(A64_PUSH(A64_FP, A64_R(9), A64_SP), ctx);
2185+
emit(A64_MOV(1, A64_FP, A64_SP), ctx);
2186+
}
21772187

2178-
/* frame for patched function */
2188+
/* frame for patched function for tracing, or caller for struct_ops */
21792189
emit(A64_PUSH(A64_FP, A64_LR, A64_SP), ctx);
21802190
emit(A64_MOV(1, A64_FP, A64_SP), ctx);
21812191

@@ -2269,19 +2279,24 @@ static int prepare_trampoline(struct jit_ctx *ctx, struct bpf_tramp_image *im,
22692279
/* reset SP */
22702280
emit(A64_MOV(1, A64_SP, A64_FP), ctx);
22712281

2272-
/* pop frames */
2273-
emit(A64_POP(A64_FP, A64_LR, A64_SP), ctx);
2274-
emit(A64_POP(A64_FP, A64_R(9), A64_SP), ctx);
2275-
2276-
if (flags & BPF_TRAMP_F_SKIP_FRAME) {
2277-
/* skip patched function, return to parent */
2278-
emit(A64_MOV(1, A64_LR, A64_R(9)), ctx);
2279-
emit(A64_RET(A64_R(9)), ctx);
2282+
if (is_struct_ops) {
2283+
emit(A64_POP(A64_FP, A64_LR, A64_SP), ctx);
2284+
emit(A64_RET(A64_LR), ctx);
22802285
} else {
2281-
/* return to patched function */
2282-
emit(A64_MOV(1, A64_R(10), A64_LR), ctx);
2283-
emit(A64_MOV(1, A64_LR, A64_R(9)), ctx);
2284-
emit(A64_RET(A64_R(10)), ctx);
2286+
/* pop frames */
2287+
emit(A64_POP(A64_FP, A64_LR, A64_SP), ctx);
2288+
emit(A64_POP(A64_FP, A64_R(9), A64_SP), ctx);
2289+
2290+
if (flags & BPF_TRAMP_F_SKIP_FRAME) {
2291+
/* skip patched function, return to parent */
2292+
emit(A64_MOV(1, A64_LR, A64_R(9)), ctx);
2293+
emit(A64_RET(A64_R(9)), ctx);
2294+
} else {
2295+
/* return to patched function */
2296+
emit(A64_MOV(1, A64_R(10), A64_LR), ctx);
2297+
emit(A64_MOV(1, A64_LR, A64_R(9)), ctx);
2298+
emit(A64_RET(A64_R(10)), ctx);
2299+
}
22852300
}
22862301

22872302
kfree(branches);

0 commit comments

Comments
 (0)