On Sat, Apr 1, 2023 at 12:43 PM Xu Kuohai <xukuohai@xxxxxxxxxxxxxxx> wrote:
>
> From: Xu Kuohai <xukuohai@xxxxxxxxxx>
>
> When BPF_TRAMP_F_CALL_ORIG is set, bpf trampoline uses BLR to jump
> back to the instruction next to call site to call the patched function.
> For BTI-enabled kernel, the instruction next to call site is usually
> PACIASP, in this case, it's safe to jump back with BLR. But when
> the call site is not followed by a PACIASP or bti, a BTI exception
> is triggered.
>
> Here is a fault log:
>
> Unhandled 64-bit el1h sync exception on CPU0, ESR 0x0000000034000002 -- BTI
> CPU: 0 PID: 263 Comm: test_progs Tainted: GF
> Hardware name: linux,dummy-virt (DT)
> pstate: 40400805 (nZcv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=-c)
> pc : bpf_fentry_test1+0xc/0x30
> lr : bpf_trampoline_6442573892_0+0x48/0x1000
> sp : ffff80000c0c3a50
> x29: ffff80000c0c3a90 x28: ffff0000c2e6c080 x27: 0000000000000000
> x26: 0000000000000000 x25: 0000000000000000 x24: 0000000000000050
> x23: 0000000000000000 x22: 0000ffffcfd2a7f0 x21: 000000000000000a
> x20: 0000ffffcfd2a7f0 x19: 0000000000000000 x18: 0000000000000000
> x17: 0000000000000000 x16: 0000000000000000 x15: 0000ffffcfd2a7f0
> x14: 0000000000000000 x13: 0000000000000000 x12: 0000000000000000
> x11: 0000000000000000 x10: ffff80000914f5e4 x9 : ffff8000082a1528
> x8 : 0000000000000000 x7 : 0000000000000000 x6 : 0101010101010101
> x5 : 0000000000000000 x4 : 00000000fffffff2 x3 : 0000000000000001
> x2 : ffff8001f4b82000 x1 : 0000000000000000 x0 : 0000000000000001
> Kernel panic - not syncing: Unhandled exception
> CPU: 0 PID: 263 Comm: test_progs Tainted: GF
> Hardware name: linux,dummy-virt (DT)
> Call trace:
> dump_backtrace+0xec/0x144
> show_stack+0x24/0x7c
> dump_stack_lvl+0x8c/0xb8
> dump_stack+0x18/0x34
> panic+0x1cc/0x3ec
> __el0_error_handler_common+0x0/0x130
> el1h_64_sync_handler+0x60/0xd0
> el1h_64_sync+0x78/0x7c
> bpf_fentry_test1+0xc/0x30
> bpf_fentry_test1+0xc/0x30
> bpf_prog_test_run_tracing+0xdc/0x2a0
> __sys_bpf+0x438/0x22a0
> __arm64_sys_bpf+0x30/0x54
> invoke_syscall+0x78/0x110
> el0_svc_common.constprop.0+0x6c/0x1d0
> do_el0_svc+0x38/0xe0
> el0_svc+0x30/0xd0
> el0t_64_sync_handler+0x1ac/0x1b0
> el0t_64_sync+0x1a0/0x1a4
> Kernel Offset: disabled
> CPU features: 0x0000,00034c24,f994fdab
> Memory Limit: none
>
> And the instruction next to call site of bpf_fentry_test1 is ADD,
> not PACIASP:
>
> <bpf_fentry_test1>:
> bti c
> nop
> nop
> add w0, w0, #0x1
> paciasp
>
> For bpf prog, jit always puts a PACIASP after call site for BTI-enabled
> kernel, so there is no problem.
>
> To fix it, replace BLR with RET to bypass the branch target check.
>
> Fixes: efc9909fdce0 ("bpf, arm64: Add bpf trampoline for arm64")
> Reported-by: Florent Revest <revest@xxxxxxxxxxxx>
> Signed-off-by: Xu Kuohai <xukuohai@xxxxxxxxxx>
Thank you Xu! It does fix the BTI Oops I observed :)
Tested-by: Florent Revest <revest@xxxxxxxxxxxx>
Acked-by: Florent Revest <revest@xxxxxxxxxxxx>
> ---
> arch/arm64/net/bpf_jit.h | 4 ++++
> arch/arm64/net/bpf_jit_comp.c | 3 ++-
> 2 files changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm64/net/bpf_jit.h b/arch/arm64/net/bpf_jit.h
> index a6acb94ea3d6..c2edadb8ec6a 100644
> --- a/arch/arm64/net/bpf_jit.h
> +++ b/arch/arm64/net/bpf_jit.h
> @@ -281,4 +281,8 @@
> /* DMB */
> #define A64_DMB_ISH aarch64_insn_gen_dmb(AARCH64_INSN_MB_ISH)
>
> +/* ADR */
> +#define A64_ADR(Rd, offset) \
> + aarch64_insn_gen_adr(0, offset, Rd, AARCH64_INSN_ADR_TYPE_ADR)
> +
> #endif /* _BPF_JIT_H */
> diff --git a/arch/arm64/net/bpf_jit_comp.c b/arch/arm64/net/bpf_jit_comp.c
> index 62f805f427b7..b26da8efa616 100644
> --- a/arch/arm64/net/bpf_jit_comp.c
> +++ b/arch/arm64/net/bpf_jit_comp.c
> @@ -1900,7 +1900,8 @@ static int prepare_trampoline(struct jit_ctx *ctx, struct bpf_tramp_image *im,
> restore_args(ctx, args_off, nargs);
> /* call original func */
> emit(A64_LDR64I(A64_R(10), A64_SP, retaddr_off), ctx);
> - emit(A64_BLR(A64_R(10)), ctx);
> + emit(A64_ADR(A64_LR, AARCH64_INSN_SIZE * 2), ctx);
> + emit(A64_RET(A64_R(10)), ctx);
> /* store return value */
> emit(A64_STR64I(A64_R(0), A64_SP, retval_off), ctx);
> /* reserve a nop for bpf_tramp_image_put */
> --
> 2.30.2
>
