Re: [PATCH 00/11] x86/ibt: FineIBT-BHI

[Peter Zijlstra <peterz@infradead.org>]

On Thu, Feb 13, 2025 at 11:48:02AM +0100, Peter Zijlstra wrote:

> > One thing I realized is that CONFIG_CFI_PERMISSIVE doesn't actually do
> > anything when FineIBT is used since we lose track of CFI trap
> > locations. I'm not sure if that's worth fixing, but perhaps we could
> > disable FineIBT when permissive mode is enabled to avoid confusion?
> 
> Hmm, yeah, that's one thing I keep forgetting about. Let me try and fix
> it and see how ugly it gets before offering an opinion :-)

Completely untested and on top of this series minus the last two patches
-- basically what I just pushed into:

  git://git.kernel.org/pub/scm/linux/kernel/git/peterz/queue.git x86/core

WDYT?

---

diff --git a/arch/x86/kernel/alternative.c b/arch/x86/kernel/alternative.c
index e285933506e9..1fec1e445a25 100644
--- a/arch/x86/kernel/alternative.c
+++ b/arch/x86/kernel/alternative.c
@@ -1410,6 +1410,36 @@ static void poison_cfi(void *addr)
 	}
 }
 
+bool decode_fineibt_insn(struct pt_regs *regs, unsigned long *target, u32 *type)
+{
+	/*
+	 * FineIBT preamble:
+	 *
+	 *  __cfi_foo:
+	 *     endbr64
+	 *     subl $0x12345678, %r10d
+	 *     jz   1f
+	 *     ud2
+	 *  1: nop
+	 *  foo:
+	 *
+	 * regs->ip points to the UD2 instruction.
+	 */
+	unsigned long addr = regs->ip - (4+7+2);
+	u32 hash;
+
+	if (!is_endbr((void *)addr)) {
+Efault:
+		return false;
+	}
+
+	*target = addr + fineibt_preamble_size;
+
+	__get_kernel_nofault(&hash, *(u32 *)(addr + fineibt_preamble_hash), u32, Efault);
+	*type = (u32)regs->r10 + hash;
+	return true;
+}
+
 #else
 
 static void __apply_fineibt(s32 *start_retpoline, s32 *end_retpoline,
@@ -1421,6 +1451,11 @@ static void __apply_fineibt(s32 *start_retpoline, s32 *end_retpoline,
 static void poison_cfi(void *addr) { }
 #endif
 
+bool decode_fineibt_insn(struct pt_regs *regs, unsigned long *target, u32 *type)
+{
+	return false;
+}
+
 #endif
 
 void apply_fineibt(s32 *start_retpoline, s32 *end_retpoline,
diff --git a/arch/x86/kernel/cfi.c b/arch/x86/kernel/cfi.c
index e6bf78fac146..4c682076c656 100644
--- a/arch/x86/kernel/cfi.c
+++ b/arch/x86/kernel/cfi.c
@@ -61,6 +61,8 @@ static bool decode_cfi_insn(struct pt_regs *regs, unsigned long *target,
 	return true;
 }
 
+extern bool decode_fineibt_insn(struct pt_regs *regs, unsigned long *target, u32 *type);
+
 /*
  * Checks if a ud2 trap is because of a CFI failure, and handles the trap
  * if needed. Returns a bug_trap_type value similarly to report_bug.
@@ -70,11 +72,25 @@ enum bug_trap_type handle_cfi_failure(struct pt_regs *regs)
 	unsigned long target;
 	u32 type;
 
-	if (!is_cfi_trap(regs->ip))
-		return BUG_TRAP_TYPE_NONE;
+	switch (cfi_mode) {
+	case CFI_KCFI:
+		if (!is_cfi_trap(regs->ip))
+			return BUG_TRAP_TYPE_NONE;
+
+		if (!decode_cfi_insn(regs, &target, &type))
+			return report_cfi_failure_noaddr(regs, regs->ip);
+
+		break;
 
-	if (!decode_cfi_insn(regs, &target, &type))
-		return report_cfi_failure_noaddr(regs, regs->ip);
+	case CFI_FINEIBT:
+		if (!decode_fineibt_insn(regs, &target, &type))
+			return BUG_TRAP_TYPE_NONE;
+
+		break;
+
+	default:
+		return BUG_TRAP_TYPE_NONE;
+	}
 
 	return report_cfi_failure(regs, regs->ip, &target, type);
 }