lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<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

Powered by Openwall GNU/*/Linux Powered by OpenVZ