x86/cfi: Fix CFI rewrite for odd alignments

Rustam reported his clang builds did not boot properly; turns out his
.config has: CONFIG_DEBUG_FORCE_FUNCTION_ALIGN_64B=y set.

Fix up the FineIBT code to deal with this unusual alignment.

Fixes: 931ab63664f0 ("x86/ibt: Implement FineIBT")
Reported-by: Rustam Kovhaev <rkovhaev@gmail.com>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Tested-by: Rustam Kovhaev <rkovhaev@gmail.com>

diff --git a/arch/x86/include/asm/cfi.h b/arch/x86/include/asm/cfi.h
index c40b9ebc1fb40fbe8b4481665d14b38aae9ee81d..ab3fbbd947ed944ce045f8fe7fec1deb660014d5 100644 (file)
--- a/arch/x86/include/asm/cfi.h
+++ b/arch/x86/include/asm/cfi.h
@@ -111,6 +111,12 @@ extern bhi_thunk __bhi_args_end[];
 
 struct pt_regs;
 
+#ifdef CONFIG_CALL_PADDING
+#define CFI_OFFSET (CONFIG_FUNCTION_PADDING_CFI+5)
+#else
+#define CFI_OFFSET 5
+#endif
+
 #ifdef CONFIG_CFI
 enum bug_trap_type handle_cfi_failure(struct pt_regs *regs);
 #define __bpfcall
@@ -119,11 +125,9 @@ static inline int cfi_get_offset(void)
 {
        switch (cfi_mode) {
        case CFI_FINEIBT:
-               return 16;
+               return /* fineibt_prefix_size */ 16;
        case CFI_KCFI:
-               if (IS_ENABLED(CONFIG_CALL_PADDING))
-                       return 16;
-               return 5;
+               return CFI_OFFSET;
        default:
                return 0;
        }

diff --git a/arch/x86/include/asm/linkage.h b/arch/x86/include/asm/linkage.h
index 9d38ae744a2e4af91c78048cd8fb26ab92ac2ba4..a7294656ad908a0574665ea346c98792bb284f01 100644 (file)
--- a/arch/x86/include/asm/linkage.h
+++ b/arch/x86/include/asm/linkage.h
@@ -68,7 +68,7 @@
  * Depending on -fpatchable-function-entry=N,N usage (CONFIG_CALL_PADDING) the
  * CFI symbol layout changes.
  *
- * Without CALL_THUNKS:
+ * Without CALL_PADDING:
  *
  *     .align  FUNCTION_ALIGNMENT
  * __cfi_##name:
@@ -77,7 +77,7 @@
  *     .long   __kcfi_typeid_##name
  * name:
  *
- * With CALL_THUNKS:
+ * With CALL_PADDING:
  *
  *     .align FUNCTION_ALIGNMENT
  * __cfi_##name:

diff --git a/arch/x86/kernel/alternative.c b/arch/x86/kernel/alternative.c
index a888ae0f01fb958946e5268f5da9612dd2023c9f..e87da25d12360b99ae8757ff2d0f4b18c060b65f 100644 (file)
--- a/arch/x86/kernel/alternative.c
+++ b/arch/x86/kernel/alternative.c
@@ -1182,7 +1182,7 @@ void __init_or_module noinline apply_seal_endbr(s32 *start, s32 *end)
 
                poison_endbr(addr);
                if (IS_ENABLED(CONFIG_FINEIBT))
-                       poison_cfi(addr - 16);
+                       poison_cfi(addr - CFI_OFFSET);
        }
 }
 
@@ -1389,6 +1389,8 @@ extern u8 fineibt_preamble_end[];
 #define fineibt_preamble_ud   0x13
 #define fineibt_preamble_hash 5
 
+#define fineibt_prefix_size (fineibt_preamble_size - ENDBR_INSN_SIZE)
+
 /*
  * <fineibt_caller_start>:
  *  0:   b8 78 56 34 12          mov    $0x12345678, %eax
@@ -1634,7 +1636,7 @@ static int cfi_rewrite_preamble(s32 *start, s32 *end)
                 * have determined there are no indirect calls to it and we
                 * don't need no CFI either.
                 */
-               if (!is_endbr(addr + 16))
+               if (!is_endbr(addr + CFI_OFFSET))
                        continue;
 
                hash = decode_preamble_hash(addr, &arity);
@@ -1642,6 +1644,15 @@ static int cfi_rewrite_preamble(s32 *start, s32 *end)
                         addr, addr, 5, addr))
                        return -EINVAL;
 
+               /*
+                * FineIBT relies on being at func-16, so if the preamble is
+                * actually larger than that, place it the tail end.
+                *
+                * NOTE: this is possible with things like DEBUG_CALL_THUNKS
+                * and DEBUG_FORCE_FUNCTION_ALIGN_64B.
+                */
+               addr += CFI_OFFSET - fineibt_prefix_size;
+
                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);
@@ -1664,10 +1675,10 @@ static void cfi_rewrite_endbr(s32 *start, s32 *end)
        for (s = start; s < end; s++) {
                void *addr = (void *)s + *s;
 
-               if (!exact_endbr(addr + 16))
+               if (!exact_endbr(addr + CFI_OFFSET))
                        continue;
 
-               poison_endbr(addr + 16);
+               poison_endbr(addr + CFI_OFFSET);
        }
 }
 
@@ -1772,7 +1783,8 @@ static void __apply_fineibt(s32 *start_retpoline, s32 *end_retpoline,
        if (FINEIBT_WARN(fineibt_preamble_size, 20)                     ||
            FINEIBT_WARN(fineibt_preamble_bhi + fineibt_bhi1_size, 20)  ||
            FINEIBT_WARN(fineibt_caller_size, 14)                       ||
-           FINEIBT_WARN(fineibt_paranoid_size, 20))
+           FINEIBT_WARN(fineibt_paranoid_size, 20)                     ||
+           WARN_ON_ONCE(CFI_OFFSET < fineibt_prefix_size))
                return;
 
        if (cfi_mode == CFI_AUTO) {
@@ -1885,6 +1897,11 @@ static void poison_cfi(void *addr)
         */
        switch (cfi_mode) {
        case CFI_FINEIBT:
+               /*
+                * FineIBT preamble is at func-16.
+                */
+               addr += CFI_OFFSET - fineibt_prefix_size;
+
                /*
                 * FineIBT prefix should start with an ENDBR.
                 */
@@ -1923,8 +1940,6 @@ static void poison_cfi(void *addr)
        }
 }
 
-#define fineibt_prefix_size (fineibt_preamble_size - ENDBR_INSN_SIZE)
-
 /*
  * When regs->ip points to a 0xD6 byte in the FineIBT preamble,
  * return true and fill out target and type.

diff --git a/arch/x86/net/bpf_jit_comp.c b/arch/x86/net/bpf_jit_comp.c
index 8f10080e6fe3fa5a8a1322e06125bb2e244c593b..e9b78040d703ef21f12f25dfc1995895d4fd5fc1 100644 (file)
--- a/arch/x86/net/bpf_jit_comp.c
+++ b/arch/x86/net/bpf_jit_comp.c
@@ -438,17 +438,8 @@ static void emit_kcfi(u8 **pprog, u32 hash)
 
        EMIT1_off32(0xb8, hash);                        /* movl $hash, %eax     */
 #ifdef CONFIG_CALL_PADDING
-       EMIT1(0x90);
-       EMIT1(0x90);
-       EMIT1(0x90);
-       EMIT1(0x90);
-       EMIT1(0x90);
-       EMIT1(0x90);
-       EMIT1(0x90);
-       EMIT1(0x90);
-       EMIT1(0x90);
-       EMIT1(0x90);
-       EMIT1(0x90);
+       for (int i = 0; i < CONFIG_FUNCTION_PADDING_CFI; i++)
+               EMIT1(0x90);
 #endif
        EMIT_ENDBR();