Skip to content

Commit 24c8147

Browse files
author
Peter Zijlstra
committed
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: 931ab63 ("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>
1 parent a0cb371 commit 24c8147

4 files changed

Lines changed: 34 additions & 24 deletions

File tree

arch/x86/include/asm/cfi.h

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -111,6 +111,12 @@ extern bhi_thunk __bhi_args_end[];
111111

112112
struct pt_regs;
113113

114+
#ifdef CONFIG_CALL_PADDING
115+
#define CFI_OFFSET (CONFIG_FUNCTION_PADDING_CFI+5)
116+
#else
117+
#define CFI_OFFSET 5
118+
#endif
119+
114120
#ifdef CONFIG_CFI
115121
enum bug_trap_type handle_cfi_failure(struct pt_regs *regs);
116122
#define __bpfcall
@@ -119,11 +125,9 @@ static inline int cfi_get_offset(void)
119125
{
120126
switch (cfi_mode) {
121127
case CFI_FINEIBT:
122-
return 16;
128+
return /* fineibt_prefix_size */ 16;
123129
case CFI_KCFI:
124-
if (IS_ENABLED(CONFIG_CALL_PADDING))
125-
return 16;
126-
return 5;
130+
return CFI_OFFSET;
127131
default:
128132
return 0;
129133
}

arch/x86/include/asm/linkage.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,7 @@
6868
* Depending on -fpatchable-function-entry=N,N usage (CONFIG_CALL_PADDING) the
6969
* CFI symbol layout changes.
7070
*
71-
* Without CALL_THUNKS:
71+
* Without CALL_PADDING:
7272
*
7373
* .align FUNCTION_ALIGNMENT
7474
* __cfi_##name:
@@ -77,7 +77,7 @@
7777
* .long __kcfi_typeid_##name
7878
* name:
7979
*
80-
* With CALL_THUNKS:
80+
* With CALL_PADDING:
8181
*
8282
* .align FUNCTION_ALIGNMENT
8383
* __cfi_##name:

arch/x86/kernel/alternative.c

Lines changed: 22 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1182,7 +1182,7 @@ void __init_or_module noinline apply_seal_endbr(s32 *start, s32 *end)
11821182

11831183
poison_endbr(addr);
11841184
if (IS_ENABLED(CONFIG_FINEIBT))
1185-
poison_cfi(addr - 16);
1185+
poison_cfi(addr - CFI_OFFSET);
11861186
}
11871187
}
11881188

@@ -1389,6 +1389,8 @@ extern u8 fineibt_preamble_end[];
13891389
#define fineibt_preamble_ud 0x13
13901390
#define fineibt_preamble_hash 5
13911391

1392+
#define fineibt_prefix_size (fineibt_preamble_size - ENDBR_INSN_SIZE)
1393+
13921394
/*
13931395
* <fineibt_caller_start>:
13941396
* 0: b8 78 56 34 12 mov $0x12345678, %eax
@@ -1634,14 +1636,23 @@ static int cfi_rewrite_preamble(s32 *start, s32 *end)
16341636
* have determined there are no indirect calls to it and we
16351637
* don't need no CFI either.
16361638
*/
1637-
if (!is_endbr(addr + 16))
1639+
if (!is_endbr(addr + CFI_OFFSET))
16381640
continue;
16391641

16401642
hash = decode_preamble_hash(addr, &arity);
16411643
if (WARN(!hash, "no CFI hash found at: %pS %px %*ph\n",
16421644
addr, addr, 5, addr))
16431645
return -EINVAL;
16441646

1647+
/*
1648+
* FineIBT relies on being at func-16, so if the preamble is
1649+
* actually larger than that, place it the tail end.
1650+
*
1651+
* NOTE: this is possible with things like DEBUG_CALL_THUNKS
1652+
* and DEBUG_FORCE_FUNCTION_ALIGN_64B.
1653+
*/
1654+
addr += CFI_OFFSET - fineibt_prefix_size;
1655+
16451656
text_poke_early(addr, fineibt_preamble_start, fineibt_preamble_size);
16461657
WARN_ON(*(u32 *)(addr + fineibt_preamble_hash) != 0x12345678);
16471658
text_poke_early(addr + fineibt_preamble_hash, &hash, 4);
@@ -1664,10 +1675,10 @@ static void cfi_rewrite_endbr(s32 *start, s32 *end)
16641675
for (s = start; s < end; s++) {
16651676
void *addr = (void *)s + *s;
16661677

1667-
if (!exact_endbr(addr + 16))
1678+
if (!exact_endbr(addr + CFI_OFFSET))
16681679
continue;
16691680

1670-
poison_endbr(addr + 16);
1681+
poison_endbr(addr + CFI_OFFSET);
16711682
}
16721683
}
16731684

@@ -1772,7 +1783,8 @@ static void __apply_fineibt(s32 *start_retpoline, s32 *end_retpoline,
17721783
if (FINEIBT_WARN(fineibt_preamble_size, 20) ||
17731784
FINEIBT_WARN(fineibt_preamble_bhi + fineibt_bhi1_size, 20) ||
17741785
FINEIBT_WARN(fineibt_caller_size, 14) ||
1775-
FINEIBT_WARN(fineibt_paranoid_size, 20))
1786+
FINEIBT_WARN(fineibt_paranoid_size, 20) ||
1787+
WARN_ON_ONCE(CFI_OFFSET < fineibt_prefix_size))
17761788
return;
17771789

17781790
if (cfi_mode == CFI_AUTO) {
@@ -1885,6 +1897,11 @@ static void poison_cfi(void *addr)
18851897
*/
18861898
switch (cfi_mode) {
18871899
case CFI_FINEIBT:
1900+
/*
1901+
* FineIBT preamble is at func-16.
1902+
*/
1903+
addr += CFI_OFFSET - fineibt_prefix_size;
1904+
18881905
/*
18891906
* FineIBT prefix should start with an ENDBR.
18901907
*/
@@ -1923,8 +1940,6 @@ static void poison_cfi(void *addr)
19231940
}
19241941
}
19251942

1926-
#define fineibt_prefix_size (fineibt_preamble_size - ENDBR_INSN_SIZE)
1927-
19281943
/*
19291944
* When regs->ip points to a 0xD6 byte in the FineIBT preamble,
19301945
* return true and fill out target and type.

arch/x86/net/bpf_jit_comp.c

Lines changed: 2 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -438,17 +438,8 @@ static void emit_kcfi(u8 **pprog, u32 hash)
438438

439439
EMIT1_off32(0xb8, hash); /* movl $hash, %eax */
440440
#ifdef CONFIG_CALL_PADDING
441-
EMIT1(0x90);
442-
EMIT1(0x90);
443-
EMIT1(0x90);
444-
EMIT1(0x90);
445-
EMIT1(0x90);
446-
EMIT1(0x90);
447-
EMIT1(0x90);
448-
EMIT1(0x90);
449-
EMIT1(0x90);
450-
EMIT1(0x90);
451-
EMIT1(0x90);
441+
for (int i = 0; i < CONFIG_FUNCTION_PADDING_CFI; i++)
442+
EMIT1(0x90);
452443
#endif
453444
EMIT_ENDBR();
454445

0 commit comments

Comments
 (0)