[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-Id: <20240927194925.808912874@infradead.org>
Date: Fri, 27 Sep 2024 21:49:10 +0200
From: Peter Zijlstra <peterz@...radead.org>
To: x86@...nel.org
Cc: linux-kernel@...r.kernel.org,
peterz@...radead.org,
alyssa.milburn@...el.com,
scott.d.constable@...el.com,
joao@...rdrivepizza.com,
andrew.cooper3@...rix.com,
jpoimboe@...nel.org,
jose.marchesi@...cle.com,
hjl.tools@...il.com,
ndesaulniers@...gle.com,
samitolvanen@...gle.com,
nathan@...nel.org,
ojeda@...nel.org,
kees@...nel.org,
alexei.starovoitov@...il.com
Subject: [PATCH 14/14] x86/fineibt: Add FineIBT+BHI mitigation
Due to FineIBT weakness, add an additional mitigation for BHI.
Use the 5 bytes of the nop at -1 and the 4 byte poison to squirrel in
a BHI mitigation.
Relies on clang-cfi to emit an additional piece of magic in the kCFI
pre-amble, identifying which function arguments are pointers.
An additional u8 (next to the existing u32) is emitted like:
movl 0x12345678, %eax // CFI hash
movb 0x12, %al // CFI args
This u8 is a bitmask, where BIT(n) indicates the n'th argument is a
pointer, notably the 6 possible argument registers are:
rdi, rsi, rdx, rcx, r8 and r9
Single bit can be inlined, while 2-4 bits have combinatoric stubs with
the required magic in. Anything more will fall back to
__bhi_args_all which additionally poisons %rsp for good measure, in
case things overflowed to the stack.
FineIBT+ FineIBT+BHI
__cfi_foo: __cfi_foo:
endbr64 endbr64
subl $0x12345678, %r10d subl $0x12345678, %r10d
jz foo+4 jz +2
ud2 ud2
nop call __bhi_args_foo
foo: foo+4:
ud1 0x0(%eax), %eax
... ...
Signed-off-by: Peter Zijlstra (Intel) <peterz@...radead.org>
---
arch/x86/include/asm/cfi.h | 1
arch/x86/kernel/alternative.c | 82 ++++++++++++++++++++++++++++++++++++++----
arch/x86/net/bpf_jit_comp.c | 16 ++++++--
tools/objtool/check.c | 16 ++++----
4 files changed, 98 insertions(+), 17 deletions(-)
--- a/arch/x86/include/asm/cfi.h
+++ b/arch/x86/include/asm/cfi.h
@@ -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;
--- a/arch/x86/kernel/alternative.c
+++ b/arch/x86/kernel/alternative.c
@@ -932,7 +932,31 @@ __noendbr bool is_endbr(u32 *val)
if (get_kernel_nofault(endbr, val))
return false;
- return __is_endbr(endbr);
+ if (__is_endbr(endbr))
+ return true;
+
+#if defined(CONFIG_FINEIBT) && defined(CONFIG_X86_KERNEL_IBT_PLUS)
+ if (cfi_mode != CFI_FINEIBT_BHI)
+ return false;
+
+ /*
+ * The BHI clobbers 'replace' the ENDBR poison, but dynamic call
+ * patching (static_call, kprobes, etc..) still need to be able
+ * to identify and skip the foo()+0 'endbr'.
+ */
+
+ /* REX CMOVNE, see bhi_args_1() */
+ if ((endbr & 0xc2fffff9) == 0xc2450f49)
+ return true;
+
+ /* CALL __bhi_args_* */
+ void *dest = (void *)val + 4 + (s32)endbr;
+ if (dest >= (void *)__bhi_args_6c1 &&
+ dest <= (void *)__bhi_args_all)
+ return true;
+#endif
+
+ return false;
}
static void poison_cfi(void *addr);
@@ -1190,6 +1214,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_X86_KERNEL_IBT_PLUS) && !strcmp(str, "fineibt+bhi")) {
+ cfi_mode = CFI_FINEIBT_BHI;
} else if (!strcmp(str, "norand")) {
cfi_rand = false;
} else {
@@ -1208,10 +1234,9 @@ early_param("cfi", cfi_parse_cmdline);
*
* __cfi_\func: __cfi_\func:
* movl $0x12345678,%eax // 5 endbr64 // 4
- * nop subl $0x12345678,%r10d // 7
+ * movb $0x12,%al // 2 subl $0x12345678,%r10d // 7
* nop jz 1f // 2
* nop ud2 // 2
- * nop 1: nop // 1
* nop
* nop
* nop
@@ -1279,6 +1304,17 @@ static u32 decode_preamble_hash(void *ad
return 0; /* invalid hash value */
}
+static u8 decode_preamble_args(void *addr)
+{
+ u8 *p = addr;
+
+ /* b0 12 movb $0x12, %al */
+ if (p[5] == 0xb0)
+ return p[6];
+
+ return 0xff; /* invalid args */
+}
+
static u32 decode_caller_hash(void *addr)
{
u8 *p = addr;
@@ -1371,6 +1407,7 @@ static int cfi_rewrite_preamble(s32 *sta
for (s = start; s < end; s++) {
void *addr = (void *)s + *s;
u32 hash;
+ u8 args;
/*
* When the function doesn't start with ENDBR the compiler will
@@ -1385,11 +1422,25 @@ static int cfi_rewrite_preamble(s32 *sta
addr, addr, 5, addr))
return -EINVAL;
+ args = decode_preamble_args(addr);
+
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);
*(u8 *)(addr + fineibt_preamble_jccd8) += 4;
+
+ if (cfi_mode != CFI_FINEIBT_BHI)
+ continue;
+
+ WARN_ONCE(args == 0xff, "no CFI args found at %pS %px %*ph\n",
+ addr, addr, 7, addr);
+
+ /*
+ * Stash the ARGS byte in the NOP at __cfi_foo+15, see
+ * cfi_rewrite_endbr().
+ */
+ *(u8 *)(addr + fineibt_preamble_size - 1) = args;
}
return 0;
@@ -1401,11 +1452,26 @@ static void cfi_rewrite_endbr(s32 *start
for (s = start; s < end; s++) {
void *addr = (void *)s + *s;
+ u8 args;
if (!is_endbr(addr + 16))
continue;
- poison_endbr(addr + 16);
+ if (cfi_mode != CFI_FINEIBT_BHI) {
+ poison_endbr(addr + 16);
+ continue;
+ }
+
+ /* recover the args byte */
+ args = *(u8 *)(addr + fineibt_preamble_size - 1);
+ *(u8 *)(addr + fineibt_preamble_size - 1) = BYTES_NOP1;
+ if (args) {
+ /* only skip the UD2 */
+ *(u8 *)(addr + fineibt_preamble_jccd8) = 2;
+
+ /* write BHI clobber in the 5 bytes that hold: nop + poison */
+ bhi_args(args, addr + fineibt_preamble_size - 1);
+ }
}
}
@@ -1506,6 +1572,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)
@@ -1519,8 +1586,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:
@@ -1548,6 +1617,7 @@ static void poison_cfi(void *addr)
*/
switch (cfi_mode) {
case CFI_FINEIBT:
+ case CFI_FINEIBT_BHI:
/*
* FineIBT prefix should start with an ENDBR.
*/
--- a/arch/x86/net/bpf_jit_comp.c
+++ b/arch/x86/net/bpf_jit_comp.c
@@ -401,10 +401,17 @@ static void emit_fineibt(u8 **pprog, u32
EMIT_ENDBR();
EMIT3_off32(0x41, 0x81, 0xea, hash); /* subl $hash, %r10d */
- EMIT2(0x74, 0x07); /* jz.d8 +7 */
- EMIT2(0x0f, 0x0b); /* ud2 */
- EMIT1(0x90); /* nop */
- EMIT_ENDBR_POISON();
+ if (cfi_mode == CFI_FINEIBT_BHI) {
+ EMIT2(0x74, 0x02); /* jz.d8 +2 */
+ EMIT2(0x0f, 0x0b); /* ud2 */
+ EMIT1(0x2e); /* cs */
+ EMIT4(0x49, 0x0f, 0x45, 0xfa); /* cmovne %r10, %rdi */
+ } else {
+ EMIT2(0x74, 0x07); /* jz.d8 +7 */
+ EMIT2(0x0f, 0x0b); /* ud2 */
+ EMIT1(0x90); /* nop */
+ EMIT_ENDBR_POISON();
+ }
*pprog = prog;
}
@@ -438,6 +445,7 @@ static void emit_cfi(u8 **pprog, u32 has
switch (cfi_mode) {
case CFI_FINEIBT:
+ case CFI_FINEIBT_BHI:
emit_fineibt(&prog, hash);
break;
--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -4397,11 +4397,9 @@ static int validate_ibt_insn(struct objt
continue;
off = reloc->sym->offset;
- if (reloc_type(reloc) == R_X86_64_PC32 ||
- reloc_type(reloc) == R_X86_64_PLT32)
- off += arch_dest_reloc_offset(reloc_addend(reloc));
- else
- off += reloc_addend(reloc);
+ off += reloc_addend(reloc);
+ if (arch_pc_relative_reloc(reloc))
+ off = arch_dest_reloc_offset(off);
dest = find_insn(file, reloc->sym->sec, off);
if (!dest)
@@ -4456,10 +4454,14 @@ static int validate_ibt_insn(struct objt
static int validate_ibt_data_reloc(struct objtool_file *file,
struct reloc *reloc)
{
+ long offset = reloc->sym->offset;
struct instruction *dest;
- dest = find_insn(file, reloc->sym->sec,
- reloc->sym->offset + reloc_addend(reloc));
+ offset += reloc_addend(reloc);
+ if (reloc_type(reloc) == R_X86_64_PLT32) // the fuck ?!
+ offset = arch_dest_reloc_offset(offset);
+
+ dest = find_insn(file, reloc->sym->sec, offset);
if (!dest)
return 0;
Powered by blists - more mailing lists