Skip to content

Commit 4f9087f

Browse files
Peter ZijlstraAlexei Starovoitov
Peter Zijlstra
authored and
Alexei Starovoitov
committed
x86/cfi,bpf: Fix BPF JIT call
The current BPF call convention is __nocfi, except when it calls !JIT things, then it calls regular C functions. It so happens that with FineIBT the __nocfi and C calling conventions are incompatible. Specifically __nocfi will call at func+0, while FineIBT will have endbr-poison there, which is not a valid indirect target. Causing #CP. Notably this only triggers on IBT enabled hardware, which is probably why this hasn't been reported (also, most people will have JIT on anyway). Implement proper CFI prologues for the BPF JIT codegen and drop __nocfi for x86. Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org> Link: https://lore.kernel.org/r/20231215092707.345270396@infradead.org Signed-off-by: Alexei Starovoitov <ast@kernel.org>
1 parent 4382159 commit 4f9087f

File tree

6 files changed

+269
-14
lines changed

6 files changed

+269
-14
lines changed

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

Lines changed: 110 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,15 +9,125 @@
99
*/
1010
#include <linux/bug.h>
1111

12+
/*
13+
* An overview of the various calling conventions...
14+
*
15+
* Traditional:
16+
*
17+
* foo:
18+
* ... code here ...
19+
* ret
20+
*
21+
* direct caller:
22+
* call foo
23+
*
24+
* indirect caller:
25+
* lea foo(%rip), %r11
26+
* ...
27+
* call *%r11
28+
*
29+
*
30+
* IBT:
31+
*
32+
* foo:
33+
* endbr64
34+
* ... code here ...
35+
* ret
36+
*
37+
* direct caller:
38+
* call foo / call foo+4
39+
*
40+
* indirect caller:
41+
* lea foo(%rip), %r11
42+
* ...
43+
* call *%r11
44+
*
45+
*
46+
* kCFI:
47+
*
48+
* __cfi_foo:
49+
* movl $0x12345678, %eax
50+
* # 11 nops when CONFIG_CALL_PADDING
51+
* foo:
52+
* endbr64 # when IBT
53+
* ... code here ...
54+
* ret
55+
*
56+
* direct call:
57+
* call foo # / call foo+4 when IBT
58+
*
59+
* indirect call:
60+
* lea foo(%rip), %r11
61+
* ...
62+
* movl $(-0x12345678), %r10d
63+
* addl -4(%r11), %r10d # -15 when CONFIG_CALL_PADDING
64+
* jz 1f
65+
* ud2
66+
* 1:call *%r11
67+
*
68+
*
69+
* FineIBT (builds as kCFI + CALL_PADDING + IBT + RETPOLINE and runtime patches into):
70+
*
71+
* __cfi_foo:
72+
* endbr64
73+
* subl 0x12345678, %r10d
74+
* jz foo
75+
* ud2
76+
* nop
77+
* foo:
78+
* osp nop3 # was endbr64
79+
* ... code here ...
80+
* ret
81+
*
82+
* direct caller:
83+
* call foo / call foo+4
84+
*
85+
* indirect caller:
86+
* lea foo(%rip), %r11
87+
* ...
88+
* movl $0x12345678, %r10d
89+
* subl $16, %r11
90+
* nop4
91+
* call *%r11
92+
*
93+
*/
94+
enum cfi_mode {
95+
CFI_DEFAULT, /* FineIBT if hardware has IBT, otherwise kCFI */
96+
CFI_OFF, /* Taditional / IBT depending on .config */
97+
CFI_KCFI, /* Optionally CALL_PADDING, IBT, RETPOLINE */
98+
CFI_FINEIBT, /* see arch/x86/kernel/alternative.c */
99+
};
100+
101+
extern enum cfi_mode cfi_mode;
102+
12103
struct pt_regs;
13104

14105
#ifdef CONFIG_CFI_CLANG
15106
enum bug_trap_type handle_cfi_failure(struct pt_regs *regs);
107+
#define __bpfcall
108+
extern u32 cfi_bpf_hash;
109+
110+
static inline int cfi_get_offset(void)
111+
{
112+
switch (cfi_mode) {
113+
case CFI_FINEIBT:
114+
return 16;
115+
case CFI_KCFI:
116+
if (IS_ENABLED(CONFIG_CALL_PADDING))
117+
return 16;
118+
return 5;
119+
default:
120+
return 0;
121+
}
122+
}
123+
#define cfi_get_offset cfi_get_offset
124+
16125
#else
17126
static inline enum bug_trap_type handle_cfi_failure(struct pt_regs *regs)
18127
{
19128
return BUG_TRAP_TYPE_NONE;
20129
}
130+
#define cfi_bpf_hash 0U
21131
#endif /* CONFIG_CFI_CLANG */
22132

23133
#endif /* _ASM_X86_CFI_H */

‎arch/x86/kernel/alternative.c‎

Lines changed: 39 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@
3030
#include <asm/fixmap.h>
3131
#include <asm/paravirt.h>
3232
#include <asm/asm-prototypes.h>
33+
#include <asm/cfi.h>
3334

3435
int __read_mostly alternatives_patched;
3536

@@ -832,15 +833,43 @@ void __init_or_module apply_seal_endbr(s32 *start, s32 *end) { }
832833
#endif /* CONFIG_X86_KERNEL_IBT */
833834

834835
#ifdef CONFIG_FINEIBT
836+
#define __CFI_DEFAULT CFI_DEFAULT
837+
#elif defined(CONFIG_CFI_CLANG)
838+
#define __CFI_DEFAULT CFI_KCFI
839+
#else
840+
#define __CFI_DEFAULT CFI_OFF
841+
#endif
835842

836-
enum cfi_mode {
837-
CFI_DEFAULT,
838-
CFI_OFF,
839-
CFI_KCFI,
840-
CFI_FINEIBT,
841-
};
843+
enum cfi_mode cfi_mode __ro_after_init = __CFI_DEFAULT;
844+
845+
#ifdef CONFIG_CFI_CLANG
846+
struct bpf_insn;
847+
848+
/* Must match bpf_func_t / DEFINE_BPF_PROG_RUN() */
849+
extern unsigned int __bpf_prog_runX(const void *ctx,
850+
const struct bpf_insn *insn);
851+
852+
/*
853+
* Force a reference to the external symbol so the compiler generates
854+
* __kcfi_typid.
855+
*/
856+
__ADDRESSABLE(__bpf_prog_runX);
857+
858+
/* u32 __ro_after_init cfi_bpf_hash = __kcfi_typeid___bpf_prog_runX; */
859+
asm (
860+
" .pushsection .data..ro_after_init,\"aw\",@progbits \n"
861+
" .type cfi_bpf_hash,@object \n"
862+
" .globl cfi_bpf_hash \n"
863+
" .p2align 2, 0x0 \n"
864+
"cfi_bpf_hash: \n"
865+
" .long __kcfi_typeid___bpf_prog_runX \n"
866+
" .size cfi_bpf_hash, 4 \n"
867+
" .popsection \n"
868+
);
869+
#endif
870+
871+
#ifdef CONFIG_FINEIBT
842872

843-
static enum cfi_mode cfi_mode __ro_after_init = CFI_DEFAULT;
844873
static bool cfi_rand __ro_after_init = true;
845874
static u32 cfi_seed __ro_after_init;
846875

@@ -1149,8 +1178,10 @@ static void __apply_fineibt(s32 *start_retpoline, s32 *end_retpoline,
11491178
goto err;
11501179

11511180
if (cfi_rand) {
1152-
if (builtin)
1181+
if (builtin) {
11531182
cfi_seed = get_random_u32();
1183+
cfi_bpf_hash = cfi_rehash(cfi_bpf_hash);
1184+
}
11541185

11551186
ret = cfi_rand_preamble(start_cfi, end_cfi);
11561187
if (ret)

‎arch/x86/net/bpf_jit_comp.c‎

Lines changed: 78 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
#include <asm/nospec-branch.h>
1818
#include <asm/text-patching.h>
1919
#include <asm/unwind.h>
20+
#include <asm/cfi.h>
2021

2122
static bool all_callee_regs_used[4] = {true, true, true, true};
2223

@@ -51,9 +52,11 @@ static u8 *emit_code(u8 *ptr, u32 bytes, unsigned int len)
5152
do { EMIT4(b1, b2, b3, b4); EMIT(off, 4); } while (0)
5253

5354
#ifdef CONFIG_X86_KERNEL_IBT
54-
#define EMIT_ENDBR() EMIT(gen_endbr(), 4)
55+
#define EMIT_ENDBR() EMIT(gen_endbr(), 4)
56+
#define EMIT_ENDBR_POISON() EMIT(gen_endbr_poison(), 4)
5557
#else
5658
#define EMIT_ENDBR()
59+
#define EMIT_ENDBR_POISON()
5760
#endif
5861

5962
static bool is_imm8(int value)
@@ -304,6 +307,69 @@ static void pop_callee_regs(u8 **pprog, bool *callee_regs_used)
304307
*pprog = prog;
305308
}
306309

310+
/*
311+
* Emit the various CFI preambles, see asm/cfi.h and the comments about FineIBT
312+
* in arch/x86/kernel/alternative.c
313+
*/
314+
315+
static void emit_fineibt(u8 **pprog)
316+
{
317+
u8 *prog = *pprog;
318+
319+
EMIT_ENDBR();
320+
EMIT3_off32(0x41, 0x81, 0xea, cfi_bpf_hash); /* subl $hash, %r10d */
321+
EMIT2(0x74, 0x07); /* jz.d8 +7 */
322+
EMIT2(0x0f, 0x0b); /* ud2 */
323+
EMIT1(0x90); /* nop */
324+
EMIT_ENDBR_POISON();
325+
326+
*pprog = prog;
327+
}
328+
329+
static void emit_kcfi(u8 **pprog)
330+
{
331+
u8 *prog = *pprog;
332+
333+
EMIT1_off32(0xb8, cfi_bpf_hash); /* movl $hash, %eax */
334+
#ifdef CONFIG_CALL_PADDING
335+
EMIT1(0x90);
336+
EMIT1(0x90);
337+
EMIT1(0x90);
338+
EMIT1(0x90);
339+
EMIT1(0x90);
340+
EMIT1(0x90);
341+
EMIT1(0x90);
342+
EMIT1(0x90);
343+
EMIT1(0x90);
344+
EMIT1(0x90);
345+
EMIT1(0x90);
346+
#endif
347+
EMIT_ENDBR();
348+
349+
*pprog = prog;
350+
}
351+
352+
static void emit_cfi(u8 **pprog)
353+
{
354+
u8 *prog = *pprog;
355+
356+
switch (cfi_mode) {
357+
case CFI_FINEIBT:
358+
emit_fineibt(&prog);
359+
break;
360+
361+
case CFI_KCFI:
362+
emit_kcfi(&prog);
363+
break;
364+
365+
default:
366+
EMIT_ENDBR();
367+
break;
368+
}
369+
370+
*pprog = prog;
371+
}
372+
307373
/*
308374
* Emit x86-64 prologue code for BPF program.
309375
* bpf_tail_call helper will skip the first X86_TAIL_CALL_OFFSET bytes
@@ -315,10 +381,10 @@ static void emit_prologue(u8 **pprog, u32 stack_depth, bool ebpf_from_cbpf,
315381
{
316382
u8 *prog = *pprog;
317383

384+
emit_cfi(&prog);
318385
/* BPF trampoline can be made to work without these nops,
319386
* but let's waste 5 bytes for now and optimize later
320387
*/
321-
EMIT_ENDBR();
322388
memcpy(prog, x86_nops[5], X86_PATCH_SIZE);
323389
prog += X86_PATCH_SIZE;
324390
if (!ebpf_from_cbpf) {
@@ -3013,9 +3079,16 @@ struct bpf_prog *bpf_int_jit_compile(struct bpf_prog *prog)
30133079
jit_data->header = header;
30143080
jit_data->rw_header = rw_header;
30153081
}
3016-
prog->bpf_func = (void *)image;
3082+
/*
3083+
* ctx.prog_offset is used when CFI preambles put code *before*
3084+
* the function. See emit_cfi(). For FineIBT specifically this code
3085+
* can also be executed and bpf_prog_kallsyms_add() will
3086+
* generate an additional symbol to cover this, hence also
3087+
* decrement proglen.
3088+
*/
3089+
prog->bpf_func = (void *)image + cfi_get_offset();
30173090
prog->jited = 1;
3018-
prog->jited_len = proglen;
3091+
prog->jited_len = proglen - cfi_get_offset();
30193092
} else {
30203093
prog = orig_prog;
30213094
}
@@ -3070,6 +3143,7 @@ void bpf_jit_free(struct bpf_prog *prog)
30703143
kvfree(jit_data->addrs);
30713144
kfree(jit_data);
30723145
}
3146+
prog->bpf_func = (void *)prog->bpf_func - cfi_get_offset();
30733147
hdr = bpf_jit_binary_pack_hdr(prog);
30743148
bpf_jit_binary_pack_free(hdr, NULL);
30753149
WARN_ON_ONCE(!bpf_prog_kallsyms_verify_off(prog));

‎include/linux/bpf.h‎

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@
2929
#include <linux/rcupdate_trace.h>
3030
#include <linux/static_call.h>
3131
#include <linux/memcontrol.h>
32+
#include <linux/cfi.h>
3233

3334
struct bpf_verifier_env;
3435
struct bpf_verifier_log;
@@ -1211,7 +1212,11 @@ struct bpf_dispatcher {
12111212
#endif
12121213
};
12131214

1214-
static __always_inline __nocfi unsigned int bpf_dispatcher_nop_func(
1215+
#ifndef __bpfcall
1216+
#define __bpfcall __nocfi
1217+
#endif
1218+
1219+
static __always_inline __bpfcall unsigned int bpf_dispatcher_nop_func(
12151220
const void *ctx,
12161221
const struct bpf_insn *insnsi,
12171222
bpf_func_t bpf_func)
@@ -1303,7 +1308,7 @@ int arch_prepare_bpf_dispatcher(void *image, void *buf, s64 *funcs, int num_func
13031308

13041309
#define DEFINE_BPF_DISPATCHER(name) \
13051310
__BPF_DISPATCHER_SC(name); \
1306-
noinline __nocfi unsigned int bpf_dispatcher_##name##_func( \
1311+
noinline __bpfcall unsigned int bpf_dispatcher_##name##_func( \
13071312
const void *ctx, \
13081313
const struct bpf_insn *insnsi, \
13091314
bpf_func_t bpf_func) \
@@ -1453,6 +1458,9 @@ struct bpf_prog_aux {
14531458
struct bpf_kfunc_desc_tab *kfunc_tab;
14541459
struct bpf_kfunc_btf_tab *kfunc_btf_tab;
14551460
u32 size_poke_tab;
1461+
#ifdef CONFIG_FINEIBT
1462+
struct bpf_ksym ksym_prefix;
1463+
#endif
14561464
struct bpf_ksym ksym;
14571465
const struct bpf_prog_ops *ops;
14581466
struct bpf_map **used_maps;

‎include/linux/cfi.h‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,13 @@
1111
#include <linux/module.h>
1212
#include <asm/cfi.h>
1313

14+
#ifndef cfi_get_offset
15+
static inline int cfi_get_offset(void)
16+
{
17+
return 0;
18+
}
19+
#endif
20+
1421
#ifdef CONFIG_CFI_CLANG
1522
enum bug_trap_type report_cfi_failure(struct pt_regs *regs, unsigned long addr,
1623
unsigned long *target, u32 type);

0 commit comments

Comments
 (0)