[PATCH 11/11] x86/fineibt: Add FineIBT+BHI mitigation
From: Peter Zijlstra
Date: Fri Feb 07 2025 - 07:29:05 EST
Due to FineIBT weakness, add an additional mitigation for BHI.
Relies on clang-cfi to emit an additional piece of magic in the kCFI
pre-amble:
https://github.com/llvm/llvm-project/commit/e223485c9b38a5579991b8cebb6a200153eee245
Which encodes the number of argument registers in the target register:
movl 0x12345678, %reg // CFI hash
XXX get Scott to write a blurb about how this works to mitigate BHI
FineIBT FineIBT+BHI
__cfi_foo: __cfi_foo:
endbr64 endbr64
subl $0x12345678, %r10d subl $0x12345678, %r10d
jz 1f call __bhi_args[\reg]
ud2
1: nop
foo: foo:
osp nop3 osp nop3
... ...
Signed-off-by: Peter Zijlstra (Intel) <peterz@xxxxxxxxxxxxx>
---
Makefile | 3 ++
arch/x86/Kconfig | 8 +++++
arch/x86/include/asm/cfi.h | 9 +++++-
arch/x86/kernel/alternative.c | 60 +++++++++++++++++++++++++++++++++++-------
arch/x86/net/bpf_jit_comp.c | 32 +++++++++++++++-------
kernel/bpf/core.c | 4 +-
6 files changed, 94 insertions(+), 22 deletions(-)
--- a/Makefile
+++ b/Makefile
@@ -1014,6 +1014,9 @@ CC_FLAGS_CFI := -fsanitize=kcfi
ifdef CONFIG_CFI_ICALL_NORMALIZE_INTEGERS
CC_FLAGS_CFI += -fsanitize-cfi-icall-experimental-normalize-integers
endif
+ifdef CONFIG_FINEIBT_BHI
+ CC_FLAGS_CFI += -fsanitize-kcfi-arity
+endif
ifdef CONFIG_RUST
# Always pass -Zsanitizer-cfi-normalize-integers as CONFIG_RUST selects
# CONFIG_CFI_ICALL_NORMALIZE_INTEGERS.
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -2473,6 +2473,10 @@ config CC_HAS_RETURN_THUNK
config CC_HAS_ENTRY_PADDING
def_bool $(cc-option,-fpatchable-function-entry=16,16)
+config CC_HAS_KCFI_ARITY
+ def_bool $(cc-option,-fsanitize=kcfi -fsanitize-kcfi-arity)
+ depends on CC_IS_CLANG && !RUST
+
config FUNCTION_PADDING_CFI
int
default 59 if FUNCTION_ALIGNMENT_64B
@@ -2498,6 +2502,10 @@ config FINEIBT
depends on X86_KERNEL_IBT && CFI_CLANG && MITIGATION_RETPOLINE
select CALL_PADDING
+config FINEIBT_BHI
+ def_bool y
+ depends on FINEIBT && CC_HAS_KCFI_ARITY
+
config HAVE_CALL_THUNKS
def_bool y
depends on CC_HAS_ENTRY_PADDING && MITIGATION_RETHUNK && OBJTOOL
--- a/arch/x86/include/asm/cfi.h
+++ b/arch/x86/include/asm/cfi.h
@@ -72,7 +72,7 @@
* __cfi_foo:
* endbr64
* subl 0x12345678, %r10d
- * jz foo
+ * jz foo # call __bhi_args[\reg] when CONFIG_FINEIBT_BHI
* ud2
* nop
* foo:
@@ -97,6 +97,7 @@ enum cfi_mode {
CFI_OFF, /* Taditional / IBT depending on .config */
CFI_KCFI, /* Optionally CALL_PADDING, IBT, RETPOLINE */
CFI_FINEIBT, /* see arch/x86/kernel/alternative.c */
+ CFI_FINEIBT_BHI,
};
extern enum cfi_mode cfi_mode;
@@ -116,6 +117,7 @@ static inline int cfi_get_offset(void)
{
switch (cfi_mode) {
case CFI_FINEIBT:
+ case CFI_FINEIBT_BHI:
return 16;
case CFI_KCFI:
if (IS_ENABLED(CONFIG_CALL_PADDING))
@@ -128,6 +130,7 @@ static inline int cfi_get_offset(void)
#define cfi_get_offset cfi_get_offset
extern u32 cfi_get_func_hash(void *func);
+extern int cfi_get_func_arity(void *func);
#else
static inline enum bug_trap_type handle_cfi_failure(struct pt_regs *regs)
@@ -140,6 +143,10 @@ static inline u32 cfi_get_func_hash(void
{
return 0;
}
+static inline int cfi_get_func_arity(void *func)
+{
+ return 0;
+}
#endif /* CONFIG_CFI_CLANG */
#if HAS_KERNEL_IBT == 1
--- a/arch/x86/kernel/alternative.c
+++ b/arch/x86/kernel/alternative.c
@@ -962,6 +962,7 @@ u32 cfi_get_func_hash(void *func)
func -= cfi_get_offset();
switch (cfi_mode) {
case CFI_FINEIBT:
+ case CFI_FINEIBT_BHI:
func += 7;
break;
case CFI_KCFI:
@@ -976,6 +977,21 @@ u32 cfi_get_func_hash(void *func)
return hash;
}
+
+int cfi_get_func_arity(void *func)
+{
+ bhi_thunk *target;
+ s32 disp;
+
+ if (cfi_mode != CFI_FINEIBT_BHI)
+ return 0;
+
+ if (get_kernel_nofault(disp, func-4))
+ return 0;
+
+ target = func + disp;
+ return target - __bhi_args;
+}
#endif
#ifdef CONFIG_FINEIBT
@@ -1020,6 +1036,8 @@ static __init int cfi_parse_cmdline(char
cfi_mode = CFI_KCFI;
} else if (!strcmp(str, "fineibt")) {
cfi_mode = CFI_FINEIBT;
+ } else if (IS_ENABLED(CONFIG_FINEIBT_BHI) && !strcmp(str, "fineibt+bhi")) {
+ cfi_mode = CFI_FINEIBT_BHI;
} else if (!strcmp(str, "norand")) {
cfi_rand = false;
} else {
@@ -1064,6 +1082,7 @@ asm( ".pushsection .rodata \n"
"fineibt_preamble_start: \n"
" endbr64 \n"
" subl $0x12345678, %r10d \n"
+ "fineibt_preamble_bhi_call: \n"
" je fineibt_preamble_end \n"
" ud2 \n"
" nop \n"
@@ -1072,10 +1091,12 @@ asm( ".pushsection .rodata \n"
);
extern u8 fineibt_preamble_start[];
+extern u8 fineibt_preamble_bhi_call[];
extern u8 fineibt_preamble_end[];
#define fineibt_preamble_size (fineibt_preamble_end - fineibt_preamble_start)
#define fineibt_preamble_hash 7
+#define fineibt_preamble_bhi (fineibt_preamble_bhi_call - fineibt_preamble_start)
asm( ".pushsection .rodata \n"
"fineibt_caller_start: \n"
@@ -1094,13 +1115,16 @@ extern u8 fineibt_caller_end[];
#define fineibt_caller_jmp (fineibt_caller_size - 2)
-static u32 decode_preamble_hash(void *addr)
+static u32 decode_preamble_hash(void *addr, int *reg)
{
u8 *p = addr;
- /* b8 78 56 34 12 mov $0x12345678,%eax */
- if (p[0] == 0xb8)
+ /* b8+reg 78 56 34 12 movl $0x12345678,\reg */
+ if (p[0] >= 0xb8 && p[0] < 0xc0) {
+ if (reg)
+ *reg = p[0] - 0xb8;
return *(u32 *)(addr + 1);
+ }
return 0; /* invalid hash value */
}
@@ -1109,7 +1133,7 @@ static u32 decode_caller_hash(void *addr
{
u8 *p = addr;
- /* 41 ba 78 56 34 12 mov $0x12345678,%r10d */
+ /* 41 ba 78 56 34 12 movl $(-0x12345678),%r10d */
if (p[0] == 0x41 && p[1] == 0xba)
return -*(u32 *)(addr + 2);
@@ -1178,7 +1202,7 @@ static int cfi_rand_preamble(s32 *start,
void *addr = (void *)s + *s;
u32 hash;
- hash = decode_preamble_hash(addr);
+ hash = decode_preamble_hash(addr, NULL);
if (WARN(!hash, "no CFI hash found at: %pS %px %*ph\n",
addr, addr, 5, addr))
return -EINVAL;
@@ -1197,6 +1221,7 @@ static int cfi_rewrite_preamble(s32 *sta
for (s = start; s < end; s++) {
void *addr = (void *)s + *s;
u32 hash;
+ int reg;
/*
* When the function doesn't start with ENDBR the compiler will
@@ -1206,7 +1231,7 @@ static int cfi_rewrite_preamble(s32 *sta
if (!is_endbr(addr + 16))
continue;
- hash = decode_preamble_hash(addr);
+ hash = decode_preamble_hash(addr, ®);
if (WARN(!hash, "no CFI hash found at: %pS %px %*ph\n",
addr, addr, 5, addr))
return -EINVAL;
@@ -1214,6 +1239,19 @@ static int cfi_rewrite_preamble(s32 *sta
text_poke_early(addr, fineibt_preamble_start, fineibt_preamble_size);
WARN_ON(*(u32 *)(addr + fineibt_preamble_hash) != 0x12345678);
text_poke_early(addr + fineibt_preamble_hash, &hash, 4);
+
+ WARN_ONCE(!IS_ENABLED(CONFIG_FINEIBT_BHI) && reg,
+ "kCFI preamble has wrong register at: %pS %px %*ph\n",
+ addr, addr, 5, addr);
+
+ if (cfi_mode != CFI_FINEIBT_BHI || !reg)
+ continue;
+
+ text_poke_early(addr + fineibt_preamble_bhi,
+ text_gen_insn(CALL_INSN_OPCODE,
+ addr + fineibt_preamble_bhi,
+ __bhi_args[reg]),
+ CALL_INSN_SIZE);
}
return 0;
@@ -1330,6 +1368,7 @@ static void __apply_fineibt(s32 *start_r
return;
case CFI_FINEIBT:
+ case CFI_FINEIBT_BHI:
/* place the FineIBT preamble at func()-16 */
ret = cfi_rewrite_preamble(start_cfi, end_cfi);
if (ret)
@@ -1343,8 +1382,10 @@ static void __apply_fineibt(s32 *start_r
/* now that nobody targets func()+0, remove ENDBR there */
cfi_rewrite_endbr(start_cfi, end_cfi);
- if (builtin)
- pr_info("Using FineIBT CFI\n");
+ if (builtin) {
+ pr_info("Using FineIBT%s CFI\n",
+ cfi_mode == CFI_FINEIBT_BHI ? "+BHI" : "");
+ }
return;
default:
@@ -1372,6 +1413,7 @@ static void poison_cfi(void *addr)
*/
switch (cfi_mode) {
case CFI_FINEIBT:
+ case CFI_FINEIBT_BHI:
/*
* FineIBT prefix should start with an ENDBR.
*/
@@ -1394,7 +1436,7 @@ static void poison_cfi(void *addr)
/*
* kCFI prefix should start with a valid hash.
*/
- if (!decode_preamble_hash(addr))
+ if (!decode_preamble_hash(addr, NULL))
break;
/*
--- a/arch/x86/net/bpf_jit_comp.c
+++ b/arch/x86/net/bpf_jit_comp.c
@@ -410,16 +410,21 @@ static void emit_nops(u8 **pprog, int le
* Emit the various CFI preambles, see asm/cfi.h and the comments about FineIBT
* in arch/x86/kernel/alternative.c
*/
+static int emit_call(u8 **pprog, void *func, void *ip);
-static void emit_fineibt(u8 **pprog, u32 hash)
+static void emit_fineibt(u8 **pprog, u8 *ip, u32 hash, int reg)
{
u8 *prog = *pprog;
EMIT_ENDBR();
EMIT3_off32(0x41, 0x81, 0xea, hash); /* subl $hash, %r10d */
- EMIT2(0x74, 0x07); /* jz.d8 +7 */
- EMIT2(0x0f, 0x0b); /* ud2 */
- EMIT1(0x90); /* nop */
+ if (cfi_mode == CFI_FINEIBT_BHI && reg) {
+ emit_call(&prog, __bhi_args[reg], ip);
+ } else {
+ EMIT2(0x74, 0x02); /* jz.d8 +2 */
+ EMIT2(0x0f, 0x0b); /* ud2 */
+ EMIT1(0x90); /* nop */
+ }
EMIT_ENDBR_POISON();
*pprog = prog;
@@ -448,13 +453,14 @@ static void emit_kcfi(u8 **pprog, u32 ha
*pprog = prog;
}
-static void emit_cfi(u8 **pprog, u32 hash)
+static void emit_cfi(u8 **pprog, u8 *ip, u32 hash, int reg)
{
u8 *prog = *pprog;
switch (cfi_mode) {
case CFI_FINEIBT:
- emit_fineibt(&prog, hash);
+ case CFI_FINEIBT_BHI:
+ emit_fineibt(&prog, ip, hash, reg);
break;
case CFI_KCFI:
@@ -505,13 +511,17 @@ static void emit_prologue_tail_call(u8 *
* bpf_tail_call helper will skip the first X86_TAIL_CALL_OFFSET bytes
* while jumping to another program
*/
-static void emit_prologue(u8 **pprog, u32 stack_depth, bool ebpf_from_cbpf,
+static void emit_prologue(u8 **pprog, u8 *ip, u32 stack_depth, bool ebpf_from_cbpf,
bool tail_call_reachable, bool is_subprog,
bool is_exception_cb)
{
u8 *prog = *pprog;
- emit_cfi(&prog, is_subprog ? cfi_bpf_subprog_hash : cfi_bpf_hash);
+ if (is_subprog) {
+ emit_cfi(&prog, ip, cfi_bpf_subprog_hash, 5);
+ } else {
+ emit_cfi(&prog, ip, cfi_bpf_hash, 1);
+ }
/* BPF trampoline can be made to work without these nops,
* but let's waste 5 bytes for now and optimize later
*/
@@ -1480,7 +1490,7 @@ static int do_jit(struct bpf_prog *bpf_p
detect_reg_usage(insn, insn_cnt, callee_regs_used);
- emit_prologue(&prog, stack_depth,
+ emit_prologue(&prog, image, stack_depth,
bpf_prog_was_classic(bpf_prog), tail_call_reachable,
bpf_is_subprog(bpf_prog), bpf_prog->aux->exception_cb);
/* Exception callback will clobber callee regs for its own use, and
@@ -3047,7 +3057,9 @@ static int __arch_prepare_bpf_trampoline
/*
* Indirect call for bpf_struct_ops
*/
- emit_cfi(&prog, cfi_get_func_hash(func_addr));
+ emit_cfi(&prog, image,
+ cfi_get_func_hash(func_addr),
+ cfi_get_func_arity(func_addr));
} else {
/*
* Direct-call fentry stub, as such it needs accounting for the
--- a/kernel/bpf/core.c
+++ b/kernel/bpf/core.c
@@ -707,7 +707,7 @@ void bpf_prog_kallsyms_add(struct bpf_pr
* When FineIBT, code in the __cfi_foo() symbols can get executed
* and hence unwinder needs help.
*/
- if (cfi_mode != CFI_FINEIBT)
+ if (cfi_mode != CFI_FINEIBT && cfi_mode != CFI_FINEIBT_BHI)
return;
snprintf(fp->aux->ksym_prefix.name, KSYM_NAME_LEN,
@@ -727,7 +727,7 @@ void bpf_prog_kallsyms_del(struct bpf_pr
bpf_ksym_del(&fp->aux->ksym);
#ifdef CONFIG_FINEIBT
- if (cfi_mode != CFI_FINEIBT)
+ if (cfi_mode != CFI_FINEIBT && cfi_mode != CFI_FINEIBT_BHI)
return;
bpf_ksym_del(&fp->aux->ksym_prefix);
#endif