* [PATCH 0/4] x86/ibt: Implement FineIBT
@ 2022-10-27 9:28 Peter Zijlstra
2022-10-27 9:28 ` [PATCH 1/4] objtool: Add --cfi to generate the .cfi_sites section Peter Zijlstra
` (4 more replies)
0 siblings, 5 replies; 15+ messages in thread
From: Peter Zijlstra @ 2022-10-27 9:28 UTC (permalink / raw)
To: x86
Cc: linux-kernel, peterz, Kees Cook, Sami Tolvanen, Joao Moreira,
Josh Poimboeuf, Mark Rutland
Hi all,
Updated FineIBT series; I've (hopefully) incorporated all feedback from last
time with the notable exception of the Kconfig CFI default -- I'm not sure we
want to add to the Kconfig space for this, also what would a distro do with it.
Anyway; please have a look, I'm hoping to merge this soonish so we can make the
next cycle.
^ permalink raw reply [flat|nested] 15+ messages in thread* [PATCH 1/4] objtool: Add --cfi to generate the .cfi_sites section 2022-10-27 9:28 [PATCH 0/4] x86/ibt: Implement FineIBT Peter Zijlstra @ 2022-10-27 9:28 ` Peter Zijlstra 2022-11-02 9:20 ` [tip: x86/core] " tip-bot2 for Peter Zijlstra 2022-10-27 9:28 ` [PATCH 2/4] x86/ibt: Implement FineIBT Peter Zijlstra ` (3 subsequent siblings) 4 siblings, 1 reply; 15+ messages in thread From: Peter Zijlstra @ 2022-10-27 9:28 UTC (permalink / raw) To: x86 Cc: linux-kernel, peterz, Kees Cook, Sami Tolvanen, Joao Moreira, Josh Poimboeuf, Mark Rutland Add the location of all __cfi_##name symbols (as generated by kCFI) to a section such that we might re-write things at kernel boot. Notably; boot time re-hashing and FineIBT are the intended use of this. Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org> --- tools/objtool/builtin-check.c | 1 tools/objtool/check.c | 69 ++++++++++++++++++++++++++++++++ tools/objtool/include/objtool/builtin.h | 1 3 files changed, 71 insertions(+) --- a/tools/objtool/builtin-check.c +++ b/tools/objtool/builtin-check.c @@ -79,6 +79,7 @@ const struct option check_options[] = { OPT_BOOLEAN('s', "stackval", &opts.stackval, "validate frame pointer rules"), OPT_BOOLEAN('t', "static-call", &opts.static_call, "annotate static calls"), OPT_BOOLEAN('u', "uaccess", &opts.uaccess, "validate uaccess rules for SMAP"), + OPT_BOOLEAN(0 , "cfi", &opts.cfi, "annotate kernel control flow integrity (kCFI) function preambles"), OPT_CALLBACK_OPTARG(0, "dump", NULL, NULL, "orc", "dump metadata", parse_dump), OPT_GROUP("Options:"), --- a/tools/objtool/check.c +++ b/tools/objtool/check.c @@ -861,6 +861,68 @@ static int create_ibt_endbr_seal_section return 0; } +static int create_cfi_sections(struct objtool_file *file) +{ + struct section *sec, *s; + struct symbol *sym; + unsigned int *loc; + int idx; + + sec = find_section_by_name(file->elf, ".cfi_sites"); + if (sec) { + INIT_LIST_HEAD(&file->call_list); + WARN("file already has .cfi_sites section, skipping"); + return 0; + } + + idx = 0; + for_each_sec(file, s) { + if (!s->text) + continue; + + list_for_each_entry(sym, &s->symbol_list, list) { + if (sym->type != STT_FUNC) + continue; + + if (strncmp(sym->name, "__cfi_", 6)) + continue; + + idx++; + } + } + + sec = elf_create_section(file->elf, ".cfi_sites", 0, sizeof(unsigned int), idx); + if (!sec) + return -1; + + idx = 0; + for_each_sec(file, s) { + if (!s->text) + continue; + + list_for_each_entry(sym, &s->symbol_list, list) { + if (sym->type != STT_FUNC) + continue; + + if (strncmp(sym->name, "__cfi_", 6)) + continue; + + loc = (unsigned int *)sec->data->d_buf + idx; + memset(loc, 0, sizeof(unsigned int)); + + if (elf_add_reloc_to_insn(file->elf, sec, + idx * sizeof(unsigned int), + R_X86_64_PC32, + s, sym->offset)) + return -1; + + idx++; + } + } + + return 0; +} + static int create_mcount_loc_sections(struct objtool_file *file) { struct section *sec; @@ -4397,6 +4459,13 @@ int check(struct objtool_file *file) if (ret < 0) goto out; warnings += ret; + } + + if (opts.cfi) { + ret = create_cfi_sections(file); + if (ret < 0) + goto out; + warnings += ret; } if (opts.rethunk) { --- a/tools/objtool/include/objtool/builtin.h +++ b/tools/objtool/include/objtool/builtin.h @@ -26,6 +26,7 @@ struct opts { bool stackval; bool static_call; bool uaccess; + bool cfi; /* options: */ bool backtrace; ^ permalink raw reply [flat|nested] 15+ messages in thread
* [tip: x86/core] objtool: Add --cfi to generate the .cfi_sites section 2022-10-27 9:28 ` [PATCH 1/4] objtool: Add --cfi to generate the .cfi_sites section Peter Zijlstra @ 2022-11-02 9:20 ` tip-bot2 for Peter Zijlstra 0 siblings, 0 replies; 15+ messages in thread From: tip-bot2 for Peter Zijlstra @ 2022-11-02 9:20 UTC (permalink / raw) To: linux-tip-commits; +Cc: Peter Zijlstra (Intel), x86, linux-kernel The following commit has been merged into the x86/core branch of tip: Commit-ID: 9a479f766be1dd777e12e3e57b6ee4c3028a40a5 Gitweb: https://git.kernel.org/tip/9a479f766be1dd777e12e3e57b6ee4c3028a40a5 Author: Peter Zijlstra <peterz@infradead.org> AuthorDate: Thu, 27 Oct 2022 11:28:13 +02:00 Committer: Peter Zijlstra <peterz@infradead.org> CommitterDate: Tue, 01 Nov 2022 13:44:10 +01:00 objtool: Add --cfi to generate the .cfi_sites section Add the location of all __cfi_##name symbols (as generated by kCFI) to a section such that we might re-write things at kernel boot. Notably; boot time re-hashing and FineIBT are the intended use of this. Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org> Link: https://lore.kernel.org/r/20221027092842.568039454@infradead.org --- tools/objtool/builtin-check.c | 1 +- tools/objtool/check.c | 69 ++++++++++++++++++++++++- tools/objtool/include/objtool/builtin.h | 1 +- 3 files changed, 71 insertions(+) diff --git a/tools/objtool/builtin-check.c b/tools/objtool/builtin-check.c index 95fcece..868e3e3 100644 --- a/tools/objtool/builtin-check.c +++ b/tools/objtool/builtin-check.c @@ -80,6 +80,7 @@ const struct option check_options[] = { OPT_BOOLEAN('s', "stackval", &opts.stackval, "validate frame pointer rules"), OPT_BOOLEAN('t', "static-call", &opts.static_call, "annotate static calls"), OPT_BOOLEAN('u', "uaccess", &opts.uaccess, "validate uaccess rules for SMAP"), + OPT_BOOLEAN(0 , "cfi", &opts.cfi, "annotate kernel control flow integrity (kCFI) function preambles"), OPT_CALLBACK_OPTARG(0, "dump", NULL, NULL, "orc", "dump metadata", parse_dump), OPT_GROUP("Options:"), diff --git a/tools/objtool/check.c b/tools/objtool/check.c index 27f35f5..55066c4 100644 --- a/tools/objtool/check.c +++ b/tools/objtool/check.c @@ -861,6 +861,68 @@ static int create_ibt_endbr_seal_sections(struct objtool_file *file) return 0; } +static int create_cfi_sections(struct objtool_file *file) +{ + struct section *sec, *s; + struct symbol *sym; + unsigned int *loc; + int idx; + + sec = find_section_by_name(file->elf, ".cfi_sites"); + if (sec) { + INIT_LIST_HEAD(&file->call_list); + WARN("file already has .cfi_sites section, skipping"); + return 0; + } + + idx = 0; + for_each_sec(file, s) { + if (!s->text) + continue; + + list_for_each_entry(sym, &s->symbol_list, list) { + if (sym->type != STT_FUNC) + continue; + + if (strncmp(sym->name, "__cfi_", 6)) + continue; + + idx++; + } + } + + sec = elf_create_section(file->elf, ".cfi_sites", 0, sizeof(unsigned int), idx); + if (!sec) + return -1; + + idx = 0; + for_each_sec(file, s) { + if (!s->text) + continue; + + list_for_each_entry(sym, &s->symbol_list, list) { + if (sym->type != STT_FUNC) + continue; + + if (strncmp(sym->name, "__cfi_", 6)) + continue; + + loc = (unsigned int *)sec->data->d_buf + idx; + memset(loc, 0, sizeof(unsigned int)); + + if (elf_add_reloc_to_insn(file->elf, sec, + idx * sizeof(unsigned int), + R_X86_64_PC32, + s, sym->offset)) + return -1; + + idx++; + } + } + + return 0; +} + static int create_mcount_loc_sections(struct objtool_file *file) { struct section *sec; @@ -4430,6 +4492,13 @@ int check(struct objtool_file *file) warnings += ret; } + if (opts.cfi) { + ret = create_cfi_sections(file); + if (ret < 0) + goto out; + warnings += ret; + } + if (opts.rethunk) { ret = create_return_sites_sections(file); if (ret < 0) diff --git a/tools/objtool/include/objtool/builtin.h b/tools/objtool/include/objtool/builtin.h index f341b62..c44ff39 100644 --- a/tools/objtool/include/objtool/builtin.h +++ b/tools/objtool/include/objtool/builtin.h @@ -27,6 +27,7 @@ struct opts { bool static_call; bool uaccess; int prefix; + bool cfi; /* options: */ bool backtrace; ^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH 2/4] x86/ibt: Implement FineIBT 2022-10-27 9:28 [PATCH 0/4] x86/ibt: Implement FineIBT Peter Zijlstra 2022-10-27 9:28 ` [PATCH 1/4] objtool: Add --cfi to generate the .cfi_sites section Peter Zijlstra @ 2022-10-27 9:28 ` Peter Zijlstra 2022-10-27 10:11 ` Peter Zijlstra ` (2 more replies) 2022-10-27 9:28 ` [PATCH 3/4] x86/cfi: Boot time selection of CFI scheme Peter Zijlstra ` (2 subsequent siblings) 4 siblings, 3 replies; 15+ messages in thread From: Peter Zijlstra @ 2022-10-27 9:28 UTC (permalink / raw) To: x86 Cc: linux-kernel, peterz, Kees Cook, Sami Tolvanen, Joao Moreira, Josh Poimboeuf, Mark Rutland Implement an alternative CFI scheme that merges both the fine-grained nature of kCFI but also takes full advantage of the coarse grained hardware CFI as provided by IBT. To contrast: kCFI is a pure software CFI scheme and relies on being able to read text -- specifically the instruction *before* the target symbol, and does the hash validation *before* doing the call (otherwise control flow is compromised already). FineIBT is a software and hardware hybrid scheme; by ensuring every branch target starts with a hash validation it is possible to place the hash validation after the branch. This has several advantages: o the (hash) load is avoided; no memop; no RX requirement. o IBT WAIT-FOR-ENDBR state is a speculation stop; by placing the hash validation in the immediate instruction after the branch target there is a minimal speculation window and the whole is a viable defence against SpectreBHB. o Kees feels obliged to mention it is slightly more vulnerable when the attacker can write code. Obviously this patch relies on kCFI, but additionally it also relies on the padding from the call-depth-tracking patches. It uses this padding to place the hash-validation while the call-sites are re-written to modify the indirect target to be 16 bytes in front of the original target, thus hitting this new preamble. Notably, there is no hardware that needs call-depth-tracking (Skylake) and supports IBT (Tigerlake and onwards). Suggested-by: Joao Moreira (Intel) <joao@overdrivepizza.com> Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org> --- arch/um/kernel/um_arch.c | 5 arch/x86/Kconfig | 12 + arch/x86/Makefile | 2 arch/x86/include/asm/alternative.h | 2 arch/x86/include/asm/linkage.h | 6 arch/x86/kernel/alternative.c | 253 +++++++++++++++++++++++++++++++++++-- arch/x86/kernel/cpu/common.c | 1 arch/x86/kernel/module.c | 20 ++ arch/x86/kernel/vmlinux.lds.S | 9 + include/linux/bpf.h | 2 scripts/Makefile.lib | 1 11 files changed, 293 insertions(+), 20 deletions(-) --- a/arch/um/kernel/um_arch.c +++ b/arch/um/kernel/um_arch.c @@ -444,6 +444,11 @@ void apply_returns(s32 *start, s32 *end) { } +void apply_fineibt(s32 *start_retpoline, s32 *end_retpoline, + s32 *start_cfi, s32 *end_cfi) +{ +} + void apply_alternatives(struct alt_instr *start, struct alt_instr *end) { } --- a/arch/x86/Kconfig +++ b/arch/x86/Kconfig @@ -2463,13 +2463,23 @@ config FUNCTION_PADDING_BYTES default FUNCTION_PADDING_CFI if CFI_CLANG default FUNCTION_ALIGNMENT +config CALL_PADDING + def_bool n + depends on CC_HAS_ENTRY_PADDING && OBJTOOL + select FUNCTION_ALIGNMENT_16B + +config FINEIBT + def_bool y + depends on X86_KERNEL_IBT && CFI_CLANG && RETPOLINE + select CALL_PADDING + config HAVE_CALL_THUNKS def_bool y depends on CC_HAS_ENTRY_PADDING && RETHUNK && OBJTOOL config CALL_THUNKS def_bool n - select FUNCTION_ALIGNMENT_16B + select CALL_PADDING menuconfig SPECULATION_MITIGATIONS bool "Mitigations for speculative execution vulnerabilities" --- a/arch/x86/Makefile +++ b/arch/x86/Makefile @@ -208,7 +208,7 @@ ifdef CONFIG_SLS KBUILD_CFLAGS += -mharden-sls=all endif -ifdef CONFIG_CALL_THUNKS +ifdef CONFIG_CALL_PADDING PADDING_CFLAGS := -fpatchable-function-entry=$(CONFIG_FUNCTION_PADDING_BYTES),$(CONFIG_FUNCTION_PADDING_BYTES) KBUILD_CFLAGS += $(PADDING_CFLAGS) export PADDING_CFLAGS --- a/arch/x86/include/asm/alternative.h +++ b/arch/x86/include/asm/alternative.h @@ -78,6 +78,8 @@ extern void apply_alternatives(struct al extern void apply_retpolines(s32 *start, s32 *end); extern void apply_returns(s32 *start, s32 *end); extern void apply_ibt_endbr(s32 *start, s32 *end); +extern void apply_fineibt(s32 *start_retpoline, s32 *end_retpoine, + s32 *start_cfi, s32 *end_cfi); struct module; struct paravirt_patch_site; --- a/arch/x86/include/asm/linkage.h +++ b/arch/x86/include/asm/linkage.h @@ -15,7 +15,7 @@ #define __ALIGN .balign CONFIG_FUNCTION_ALIGNMENT, 0x90; #define __ALIGN_STR __stringify(__ALIGN) -#if defined(CONFIG_CALL_THUNKS) && !defined(__DISABLE_EXPORTS) && !defined(BUILD_VDSO) +#if defined(CONFIG_CALL_PADDING) && !defined(__DISABLE_EXPORTS) && !defined(BUILD_VDSO) #define FUNCTION_PADDING .skip CONFIG_FUNCTION_ALIGNMENT, 0x90; #else #define FUNCTION_PADDING @@ -57,7 +57,7 @@ #endif /* __ASSEMBLY__ */ /* - * Depending on -fpatchable-function-entry=N,N usage (CONFIG_CALL_THUNKS) the + * Depending on -fpatchable-function-entry=N,N usage (CONFIG_CALL_PADDING) the * CFI symbol layout changes. * * Without CALL_THUNKS: @@ -81,7 +81,7 @@ * In both cases the whole thing is FUNCTION_ALIGNMENT aligned and sized. */ -#ifdef CONFIG_CALL_THUNKS +#ifdef CONFIG_CALL_PADDING #define CFI_PRE_PADDING #define CFI_POST_PADDING .skip CONFIG_FUNCTION_PADDING_BYTES, 0x90; #else --- a/arch/x86/kernel/alternative.c +++ b/arch/x86/kernel/alternative.c @@ -116,6 +116,7 @@ static void __init_or_module add_nops(vo extern s32 __retpoline_sites[], __retpoline_sites_end[]; extern s32 __return_sites[], __return_sites_end[]; +extern s32 __cfi_sites[], __cfi_sites_end[]; extern s32 __ibt_endbr_seal[], __ibt_endbr_seal_end[]; extern struct alt_instr __alt_instructions[], __alt_instructions_end[]; extern s32 __smp_locks[], __smp_locks_end[]; @@ -656,6 +657,28 @@ void __init_or_module noinline apply_ret #ifdef CONFIG_X86_KERNEL_IBT +static void poison_endbr(void *addr, bool warn) +{ + u32 endbr, poison = gen_endbr_poison(); + + if (WARN_ON_ONCE(get_kernel_nofault(endbr, addr))) + return; + + if (!is_endbr(endbr)) { + WARN_ON_ONCE(warn); + return; + } + + DPRINTK("ENDBR at: %pS (%px)", addr, addr); + + /* + * When we have IBT, the lack of ENDBR will trigger #CP + */ + DUMP_BYTES(((u8*)addr), 4, "%px: orig: ", addr); + DUMP_BYTES(((u8*)&poison), 4, "%px: repl: ", addr); + text_poke_early(addr, &poison, 4); +} + /* * Generated by: objtool --ibt */ @@ -664,31 +687,232 @@ void __init_or_module noinline apply_ibt s32 *s; for (s = start; s < end; s++) { - u32 endbr, poison = gen_endbr_poison(); void *addr = (void *)s + *s; - if (WARN_ON_ONCE(get_kernel_nofault(endbr, addr))) - continue; + poison_endbr(addr, true); + if (IS_ENABLED(CONFIG_FINEIBT)) + poison_endbr(addr - 16, false); + } +} + +#else + +void __init_or_module noinline apply_ibt_endbr(s32 *start, s32 *end) { } + +#endif /* CONFIG_X86_KERNEL_IBT */ + +#ifdef CONFIG_FINEIBT +/* + * kCFI FineIBT + * + * __cfi_\func: __cfi_\func: + * movl $0x12345678,%eax // 5 endbr64 // 4 + * nop subl $0x12345678,%r10d // 7 + * nop jz 1f // 2 + * nop ud2 // 2 + * nop 1: nop // 1 + * nop + * nop + * nop + * nop + * nop + * nop + * nop + * + * + * caller: caller: + * movl $(-0x12345678),%r10d // 6 movl $0x12345678,%r10d // 6 + * addl $-15(%r11),%r10d // 4 sub $16,%r11 // 4 + * je 1f // 2 nop4 // 4 + * ud2 // 2 + * 1: call __x86_indirect_thunk_r11 // 5 call *%r11; nop2; // 5 + * + */ + +asm( ".pushsection .rodata \n" + "fineibt_preamble_start: \n" + " endbr64 \n" + " subl $0x12345678, %r10d \n" + " je fineibt_preamble_end \n" + " ud2 \n" + " nop \n" + "fineibt_preamble_end: \n" + ".popsection\n" +); + +extern u8 fineibt_preamble_start[]; +extern u8 fineibt_preamble_end[]; + +#define fineibt_preamble_size (fineibt_preamble_end - fineibt_preamble_start) +#define fineibt_preamble_hash 7 + +asm( ".pushsection .rodata \n" + "fineibt_caller_start: \n" + " movl $0x12345678, %r10d \n" + " sub $16, %r11 \n" + ASM_NOP4 + "fineibt_caller_end: \n" + ".popsection \n" +); + +extern u8 fineibt_caller_start[]; +extern u8 fineibt_caller_end[]; + +#define fineibt_caller_size (fineibt_caller_end - fineibt_caller_start) +#define fineibt_caller_hash 2 + +#define fineibt_caller_jmp (fineibt_caller_size - 2) + +static u32 decode_preamble_hash(void *addr) +{ + u8 *p = addr; + + /* b8 78 56 34 12 mov $0x12345678,%eax */ + if (p[0] == 0xb8) + return *(u32 *)(addr + 1); + + return 0; /* invalid hash value */ +} + +static u32 decode_caller_hash(void *addr) +{ + u8 *p = addr; + + /* 41 ba 78 56 34 12 mov $0x12345678,%r10d */ + if (p[0] == 0x41 && p[1] == 0xba) + return -*(u32 *)(addr + 2); + + /* e8 0c 78 56 34 12 jmp.d8 +12 */ + if (p[0] == JMP8_INSN_OPCODE && p[1] == fineibt_caller_jmp) + return -*(u32 *)(addr + 2); + + return 0; /* invalid hash value */ +} + +/* .retpoline_sites */ +static int cfi_disable_callers(s32 *start, s32 *end) +{ + /* + * Disable kCFI by patching in a JMP.d8, this leaves the hash immediate + * in tact for later usage. Also see decode_caller_hash() and + * cfi_rewrite_callers(). + */ + const u8 jmp[] = { JMP8_INSN_OPCODE, fineibt_caller_jmp }; + s32 *s; - if (WARN_ON_ONCE(!is_endbr(endbr))) + for (s = start; s < end; s++) { + void *addr = (void *)s + *s; + u32 hash; + + addr -= fineibt_caller_size; + hash = decode_caller_hash(addr); + if (!hash) /* nocfi callers */ continue; - DPRINTK("ENDBR at: %pS (%px)", addr, addr); + text_poke_early(addr, jmp, 2); + } - /* - * When we have IBT, the lack of ENDBR will trigger #CP - */ - DUMP_BYTES(((u8*)addr), 4, "%px: orig: ", addr); - DUMP_BYTES(((u8*)&poison), 4, "%px: repl: ", addr); - text_poke_early(addr, &poison, 4); + return 0; +} + +/* .cfi_sites */ +static int cfi_rewrite_preamble(s32 *start, s32 *end) +{ + s32 *s; + + for (s = start; s < end; s++) { + void *addr = (void *)s + *s; + u32 hash; + + hash = decode_preamble_hash(addr); + if (WARN(!hash, "no CFI hash found at: %pS %px %*ph\n", + addr, addr, 5, addr)) + return -EINVAL; + + 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); } + + return 0; +} + +/* .retpoline_sites */ +static int cfi_rewrite_callers(s32 *start, s32 *end) +{ + s32 *s; + + for (s = start; s < end; s++) { + void *addr = (void *)s + *s; + u32 hash; + + addr -= fineibt_caller_size; + hash = decode_caller_hash(addr); + if (hash) { + text_poke_early(addr, fineibt_caller_start, fineibt_caller_size); + WARN_ON(*(u32 *)(addr + fineibt_caller_hash) != 0x12345678); + text_poke_early(addr + fineibt_caller_hash, &hash, 4); + } + /* rely on apply_retpolines() */ + } + + return 0; +} + +static void __apply_fineibt(s32 *start_retpoline, s32 *end_retpoline, + s32 *start_cfi, s32 *end_cfi, bool builtin) +{ + int ret; + + if (WARN_ONCE(fineibt_preamble_size != 16, + "FineIBT preamble wrong size: %ld", fineibt_preamble_size)) + return; + + if (!HAS_KERNEL_IBT || !cpu_feature_enabled(X86_FEATURE_IBT)) + return; + + /* + * Rewrite the callers to not use the __cfi_ stubs, such that we might + * rewrite them. This disables all CFI. If this succeeds but any of the + * later stages fails, we're without CFI. + */ + ret = cfi_disable_callers(start_retpoline, end_retpoline); + if (ret) + goto err; + + ret = cfi_rewrite_preamble(start_cfi, end_cfi); + if (ret) + goto err; + + ret = cfi_rewrite_callers(start_retpoline, end_retpoline); + if (ret) + goto err; + + if (builtin) + pr_info("Using FineIBT CFI\n"); + + return; + +err: + pr_err("Something went horribly wrong trying to rewrite the CFI implementation.\n"); } #else -void __init_or_module noinline apply_ibt_endbr(s32 *start, s32 *end) { } +static void __apply_fineibt(s32 *start_retpoline, s32 *end_retpoline, + s32 *start_cfi, s32 *end_cfi, bool builtin) +{ +} -#endif /* CONFIG_X86_KERNEL_IBT */ +#endif + +void apply_fineibt(s32 *start_retpoline, s32 *end_retpoline, + s32 *start_cfi, s32 *end_cfi) +{ + return __apply_fineibt(start_retpoline, end_retpoline, + start_cfi, end_cfi, + /* .builtin = */ false); +} #ifdef CONFIG_SMP static void alternatives_smp_lock(const s32 *start, const s32 *end, @@ -996,6 +1220,9 @@ void __init alternative_instructions(voi */ apply_paravirt(__parainstructions, __parainstructions_end); + __apply_fineibt(__retpoline_sites, __retpoline_sites_end, + __cfi_sites, __cfi_sites_end, true); + /* * Rewrite the retpolines, must be done before alternatives since * those can rewrite the retpoline thunks. --- a/arch/x86/kernel/cpu/common.c +++ b/arch/x86/kernel/cpu/common.c @@ -609,6 +609,7 @@ static __always_inline void setup_cet(st if (!ibt_selftest()) { pr_err("IBT selftest: Failed!\n"); + wrmsrl(MSR_IA32_S_CET, 0); setup_clear_cpu_cap(X86_FEATURE_IBT); return; } --- a/arch/x86/kernel/module.c +++ b/arch/x86/kernel/module.c @@ -255,7 +255,7 @@ int module_finalize(const Elf_Ehdr *hdr, const Elf_Shdr *s, *text = NULL, *alt = NULL, *locks = NULL, *para = NULL, *orc = NULL, *orc_ip = NULL, *retpolines = NULL, *returns = NULL, *ibt_endbr = NULL, - *calls = NULL; + *calls = NULL, *cfi = NULL; char *secstrings = (void *)hdr + sechdrs[hdr->e_shstrndx].sh_offset; for (s = sechdrs; s < sechdrs + hdr->e_shnum; s++) { @@ -277,6 +277,8 @@ int module_finalize(const Elf_Ehdr *hdr, returns = s; if (!strcmp(".call_sites", secstrings + s->sh_name)) calls = s; + if (!strcmp(".cfi_sites", secstrings + s->sh_name)) + cfi = s; if (!strcmp(".ibt_endbr_seal", secstrings + s->sh_name)) ibt_endbr = s; } @@ -289,6 +291,22 @@ int module_finalize(const Elf_Ehdr *hdr, void *pseg = (void *)para->sh_addr; apply_paravirt(pseg, pseg + para->sh_size); } + if (retpolines || cfi) { + void *rseg = NULL, *cseg = NULL; + unsigned int rsize = 0, csize = 0; + + if (retpolines) { + rseg = (void *)retpolines->sh_addr; + rsize = retpolines->sh_size; + } + + if (cfi) { + cseg = (void *)cfi->sh_addr; + csize = cfi->sh_size; + } + + apply_fineibt(rseg, rseg + rsize, cseg, cseg + csize); + } if (retpolines) { void *rseg = (void *)retpolines->sh_addr; apply_retpolines(rseg, rseg + retpolines->sh_size); --- a/arch/x86/kernel/vmlinux.lds.S +++ b/arch/x86/kernel/vmlinux.lds.S @@ -309,6 +309,15 @@ SECTIONS } #endif +#ifdef CONFIG_FINEIBT + . = ALIGN(8); + .cfi_sites : AT(ADDR(.cfi_sites) - LOAD_OFFSET) { + __cfi_sites = .; + *(.cfi_sites) + __cfi_sites_end = .; + } +#endif + /* * struct alt_inst entries. From the header (alternative.h): * "Alternative instructions for different CPU types or capabilities" --- a/include/linux/bpf.h +++ b/include/linux/bpf.h @@ -984,7 +984,7 @@ int arch_prepare_bpf_dispatcher(void *im } #ifdef CONFIG_X86_64 -#ifdef CONFIG_CALL_THUNKS +#ifdef CONFIG_CALL_PADDING #define BPF_DISPATCHER_ATTRIBUTES __attribute__((patchable_function_entry(5+CONFIG_FUNCTION_PADDING_BYTES,CONFIG_FUNCTION_PADDING_BYTES))) #else #define BPF_DISPATCHER_ATTRIBUTES __attribute__((patchable_function_entry(5))) --- a/scripts/Makefile.lib +++ b/scripts/Makefile.lib @@ -256,6 +256,7 @@ objtool-args-$(CONFIG_HAVE_JUMP_LABEL_HA objtool-args-$(CONFIG_HAVE_NOINSTR_HACK) += --hacks=noinstr objtool-args-$(CONFIG_CALL_DEPTH_TRACKING) += --hacks=skylake objtool-args-$(CONFIG_X86_KERNEL_IBT) += --ibt +objtool-args-$(CONFIG_FINEIBT) += --cfi objtool-args-$(CONFIG_FTRACE_MCOUNT_USE_OBJTOOL) += --mcount objtool-args-$(CONFIG_UNWINDER_ORC) += --orc objtool-args-$(CONFIG_RETPOLINE) += --retpoline ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 2/4] x86/ibt: Implement FineIBT 2022-10-27 9:28 ` [PATCH 2/4] x86/ibt: Implement FineIBT Peter Zijlstra @ 2022-10-27 10:11 ` Peter Zijlstra 2022-10-28 17:41 ` Kees Cook 2022-11-02 9:20 ` [tip: x86/core] " tip-bot2 for Peter Zijlstra 2 siblings, 0 replies; 15+ messages in thread From: Peter Zijlstra @ 2022-10-27 10:11 UTC (permalink / raw) To: x86 Cc: linux-kernel, Kees Cook, Sami Tolvanen, Joao Moreira, Josh Poimboeuf, Mark Rutland On Thu, Oct 27, 2022 at 11:28:14AM +0200, Peter Zijlstra wrote: > --- a/scripts/Makefile.lib > +++ b/scripts/Makefile.lib > @@ -256,6 +256,7 @@ objtool-args-$(CONFIG_HAVE_JUMP_LABEL_HA > objtool-args-$(CONFIG_HAVE_NOINSTR_HACK) += --hacks=noinstr > objtool-args-$(CONFIG_CALL_DEPTH_TRACKING) += --hacks=skylake > objtool-args-$(CONFIG_X86_KERNEL_IBT) += --ibt > +objtool-args-$(CONFIG_FINEIBT) += --cfi > objtool-args-$(CONFIG_FTRACE_MCOUNT_USE_OBJTOOL) += --mcount > objtool-args-$(CONFIG_UNWINDER_ORC) += --orc > objtool-args-$(CONFIG_RETPOLINE) += --retpoline I seem to have lost a refresh; consider that whitespace fail fixed. ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 2/4] x86/ibt: Implement FineIBT 2022-10-27 9:28 ` [PATCH 2/4] x86/ibt: Implement FineIBT Peter Zijlstra 2022-10-27 10:11 ` Peter Zijlstra @ 2022-10-28 17:41 ` Kees Cook 2022-11-02 9:20 ` [tip: x86/core] " tip-bot2 for Peter Zijlstra 2 siblings, 0 replies; 15+ messages in thread From: Kees Cook @ 2022-10-28 17:41 UTC (permalink / raw) To: Peter Zijlstra Cc: x86, linux-kernel, Sami Tolvanen, Joao Moreira, Josh Poimboeuf, Mark Rutland On Thu, Oct 27, 2022 at 11:28:14AM +0200, Peter Zijlstra wrote: > Implement an alternative CFI scheme that merges both the fine-grained > nature of kCFI but also takes full advantage of the coarse grained > hardware CFI as provided by IBT. > > To contrast: > > kCFI is a pure software CFI scheme and relies on being able to read > text -- specifically the instruction *before* the target symbol, and > does the hash validation *before* doing the call (otherwise control > flow is compromised already). > > FineIBT is a software and hardware hybrid scheme; by ensuring every > branch target starts with a hash validation it is possible to place > the hash validation after the branch. This has several advantages: > > o the (hash) load is avoided; no memop; no RX requirement. > > o IBT WAIT-FOR-ENDBR state is a speculation stop; by placing > the hash validation in the immediate instruction after > the branch target there is a minimal speculation window > and the whole is a viable defence against SpectreBHB. > > o Kees feels obliged to mention it is slightly more vulnerable > when the attacker can write code. > > Obviously this patch relies on kCFI, but additionally it also relies > on the padding from the call-depth-tracking patches. It uses this > padding to place the hash-validation while the call-sites are > re-written to modify the indirect target to be 16 bytes in front of > the original target, thus hitting this new preamble. > > Notably, there is no hardware that needs call-depth-tracking (Skylake) > and supports IBT (Tigerlake and onwards). > > Suggested-by: Joao Moreira (Intel) <joao@overdrivepizza.com> > Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org> Reviewed-by: Kees Cook <keescook@chromium.org> -- Kees Cook ^ permalink raw reply [flat|nested] 15+ messages in thread
* [tip: x86/core] x86/ibt: Implement FineIBT 2022-10-27 9:28 ` [PATCH 2/4] x86/ibt: Implement FineIBT Peter Zijlstra 2022-10-27 10:11 ` Peter Zijlstra 2022-10-28 17:41 ` Kees Cook @ 2022-11-02 9:20 ` tip-bot2 for Peter Zijlstra 2 siblings, 0 replies; 15+ messages in thread From: tip-bot2 for Peter Zijlstra @ 2022-11-02 9:20 UTC (permalink / raw) To: linux-tip-commits Cc: Joao Moreira (Intel), Peter Zijlstra (Intel), Kees Cook, x86, linux-kernel The following commit has been merged into the x86/core branch of tip: Commit-ID: 931ab63664f02b17d2213ef36b83e1e50190a0aa Gitweb: https://git.kernel.org/tip/931ab63664f02b17d2213ef36b83e1e50190a0aa Author: Peter Zijlstra <peterz@infradead.org> AuthorDate: Thu, 27 Oct 2022 11:28:14 +02:00 Committer: Peter Zijlstra <peterz@infradead.org> CommitterDate: Tue, 01 Nov 2022 13:44:10 +01:00 x86/ibt: Implement FineIBT Implement an alternative CFI scheme that merges both the fine-grained nature of kCFI but also takes full advantage of the coarse grained hardware CFI as provided by IBT. To contrast: kCFI is a pure software CFI scheme and relies on being able to read text -- specifically the instruction *before* the target symbol, and does the hash validation *before* doing the call (otherwise control flow is compromised already). FineIBT is a software and hardware hybrid scheme; by ensuring every branch target starts with a hash validation it is possible to place the hash validation after the branch. This has several advantages: o the (hash) load is avoided; no memop; no RX requirement. o IBT WAIT-FOR-ENDBR state is a speculation stop; by placing the hash validation in the immediate instruction after the branch target there is a minimal speculation window and the whole is a viable defence against SpectreBHB. o Kees feels obliged to mention it is slightly more vulnerable when the attacker can write code. Obviously this patch relies on kCFI, but additionally it also relies on the padding from the call-depth-tracking patches. It uses this padding to place the hash-validation while the call-sites are re-written to modify the indirect target to be 16 bytes in front of the original target, thus hitting this new preamble. Notably, there is no hardware that needs call-depth-tracking (Skylake) and supports IBT (Tigerlake and onwards). Suggested-by: Joao Moreira (Intel) <joao@overdrivepizza.com> Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org> Reviewed-by: Kees Cook <keescook@chromium.org> Link: https://lore.kernel.org/r/20221027092842.634714496@infradead.org --- arch/um/kernel/um_arch.c | 5 +- arch/x86/Kconfig | 14 +- arch/x86/Makefile | 2 +- arch/x86/include/asm/alternative.h | 2 +- arch/x86/include/asm/linkage.h | 6 +- arch/x86/kernel/alternative.c | 253 ++++++++++++++++++++++++++-- arch/x86/kernel/cpu/common.c | 1 +- arch/x86/kernel/module.c | 20 +- arch/x86/kernel/vmlinux.lds.S | 9 +- include/linux/bpf.h | 2 +- scripts/Makefile.lib | 1 +- 11 files changed, 294 insertions(+), 21 deletions(-) diff --git a/arch/um/kernel/um_arch.c b/arch/um/kernel/um_arch.c index 8adf8e8..786b44d 100644 --- a/arch/um/kernel/um_arch.c +++ b/arch/um/kernel/um_arch.c @@ -444,6 +444,11 @@ void apply_returns(s32 *start, s32 *end) { } +void apply_fineibt(s32 *start_retpoline, s32 *end_retpoline, + s32 *start_cfi, s32 *end_cfi) +{ +} + void apply_alternatives(struct alt_instr *start, struct alt_instr *end) { } diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig index 32818aa..479ee63 100644 --- a/arch/x86/Kconfig +++ b/arch/x86/Kconfig @@ -2463,17 +2463,27 @@ config FUNCTION_PADDING_BYTES default FUNCTION_PADDING_CFI if CFI_CLANG default FUNCTION_ALIGNMENT +config CALL_PADDING + def_bool n + depends on CC_HAS_ENTRY_PADDING && OBJTOOL + select FUNCTION_ALIGNMENT_16B + +config FINEIBT + def_bool y + depends on X86_KERNEL_IBT && CFI_CLANG && RETPOLINE + select CALL_PADDING + config HAVE_CALL_THUNKS def_bool y depends on CC_HAS_ENTRY_PADDING && RETHUNK && OBJTOOL config CALL_THUNKS def_bool n - select FUNCTION_ALIGNMENT_16B + select CALL_PADDING config PREFIX_SYMBOLS def_bool y - depends on CALL_THUNKS && !CFI_CLANG + depends on CALL_PADDING && !CFI_CLANG menuconfig SPECULATION_MITIGATIONS bool "Mitigations for speculative execution vulnerabilities" diff --git a/arch/x86/Makefile b/arch/x86/Makefile index 1640e00..a3a07df 100644 --- a/arch/x86/Makefile +++ b/arch/x86/Makefile @@ -208,7 +208,7 @@ ifdef CONFIG_SLS KBUILD_CFLAGS += -mharden-sls=all endif -ifdef CONFIG_CALL_THUNKS +ifdef CONFIG_CALL_PADDING PADDING_CFLAGS := -fpatchable-function-entry=$(CONFIG_FUNCTION_PADDING_BYTES),$(CONFIG_FUNCTION_PADDING_BYTES) KBUILD_CFLAGS += $(PADDING_CFLAGS) export PADDING_CFLAGS diff --git a/arch/x86/include/asm/alternative.h b/arch/x86/include/asm/alternative.h index 664c077..7659217 100644 --- a/arch/x86/include/asm/alternative.h +++ b/arch/x86/include/asm/alternative.h @@ -78,6 +78,8 @@ extern void apply_alternatives(struct alt_instr *start, struct alt_instr *end); extern void apply_retpolines(s32 *start, s32 *end); extern void apply_returns(s32 *start, s32 *end); extern void apply_ibt_endbr(s32 *start, s32 *end); +extern void apply_fineibt(s32 *start_retpoline, s32 *end_retpoine, + s32 *start_cfi, s32 *end_cfi); struct module; struct paravirt_patch_site; diff --git a/arch/x86/include/asm/linkage.h b/arch/x86/include/asm/linkage.h index 45e0df8..dd9b811 100644 --- a/arch/x86/include/asm/linkage.h +++ b/arch/x86/include/asm/linkage.h @@ -15,7 +15,7 @@ #define __ALIGN .balign CONFIG_FUNCTION_ALIGNMENT, 0x90; #define __ALIGN_STR __stringify(__ALIGN) -#if defined(CONFIG_CALL_THUNKS) && !defined(__DISABLE_EXPORTS) && !defined(BUILD_VDSO) +#if defined(CONFIG_CALL_PADDING) && !defined(__DISABLE_EXPORTS) && !defined(BUILD_VDSO) #define FUNCTION_PADDING .skip CONFIG_FUNCTION_ALIGNMENT, 0x90; #else #define FUNCTION_PADDING @@ -57,7 +57,7 @@ #endif /* __ASSEMBLY__ */ /* - * Depending on -fpatchable-function-entry=N,N usage (CONFIG_CALL_THUNKS) the + * Depending on -fpatchable-function-entry=N,N usage (CONFIG_CALL_PADDING) the * CFI symbol layout changes. * * Without CALL_THUNKS: @@ -81,7 +81,7 @@ * In both cases the whole thing is FUNCTION_ALIGNMENT aligned and sized. */ -#ifdef CONFIG_CALL_THUNKS +#ifdef CONFIG_CALL_PADDING #define CFI_PRE_PADDING #define CFI_POST_PADDING .skip CONFIG_FUNCTION_PADDING_BYTES, 0x90; #else diff --git a/arch/x86/kernel/alternative.c b/arch/x86/kernel/alternative.c index b4ac4e5..91b0e63 100644 --- a/arch/x86/kernel/alternative.c +++ b/arch/x86/kernel/alternative.c @@ -116,6 +116,7 @@ static void __init_or_module add_nops(void *insns, unsigned int len) extern s32 __retpoline_sites[], __retpoline_sites_end[]; extern s32 __return_sites[], __return_sites_end[]; +extern s32 __cfi_sites[], __cfi_sites_end[]; extern s32 __ibt_endbr_seal[], __ibt_endbr_seal_end[]; extern struct alt_instr __alt_instructions[], __alt_instructions_end[]; extern s32 __smp_locks[], __smp_locks_end[]; @@ -656,6 +657,28 @@ void __init_or_module noinline apply_returns(s32 *start, s32 *end) { } #ifdef CONFIG_X86_KERNEL_IBT +static void poison_endbr(void *addr, bool warn) +{ + u32 endbr, poison = gen_endbr_poison(); + + if (WARN_ON_ONCE(get_kernel_nofault(endbr, addr))) + return; + + if (!is_endbr(endbr)) { + WARN_ON_ONCE(warn); + return; + } + + DPRINTK("ENDBR at: %pS (%px)", addr, addr); + + /* + * When we have IBT, the lack of ENDBR will trigger #CP + */ + DUMP_BYTES(((u8*)addr), 4, "%px: orig: ", addr); + DUMP_BYTES(((u8*)&poison), 4, "%px: repl: ", addr); + text_poke_early(addr, &poison, 4); +} + /* * Generated by: objtool --ibt */ @@ -664,31 +687,232 @@ void __init_or_module noinline apply_ibt_endbr(s32 *start, s32 *end) s32 *s; for (s = start; s < end; s++) { - u32 endbr, poison = gen_endbr_poison(); void *addr = (void *)s + *s; - if (WARN_ON_ONCE(get_kernel_nofault(endbr, addr))) - continue; + poison_endbr(addr, true); + if (IS_ENABLED(CONFIG_FINEIBT)) + poison_endbr(addr - 16, false); + } +} + +#else + +void __init_or_module noinline apply_ibt_endbr(s32 *start, s32 *end) { } + +#endif /* CONFIG_X86_KERNEL_IBT */ + +#ifdef CONFIG_FINEIBT +/* + * kCFI FineIBT + * + * __cfi_\func: __cfi_\func: + * movl $0x12345678,%eax // 5 endbr64 // 4 + * nop subl $0x12345678,%r10d // 7 + * nop jz 1f // 2 + * nop ud2 // 2 + * nop 1: nop // 1 + * nop + * nop + * nop + * nop + * nop + * nop + * nop + * + * + * caller: caller: + * movl $(-0x12345678),%r10d // 6 movl $0x12345678,%r10d // 6 + * addl $-15(%r11),%r10d // 4 sub $16,%r11 // 4 + * je 1f // 2 nop4 // 4 + * ud2 // 2 + * 1: call __x86_indirect_thunk_r11 // 5 call *%r11; nop2; // 5 + * + */ + +asm( ".pushsection .rodata \n" + "fineibt_preamble_start: \n" + " endbr64 \n" + " subl $0x12345678, %r10d \n" + " je fineibt_preamble_end \n" + " ud2 \n" + " nop \n" + "fineibt_preamble_end: \n" + ".popsection\n" +); + +extern u8 fineibt_preamble_start[]; +extern u8 fineibt_preamble_end[]; + +#define fineibt_preamble_size (fineibt_preamble_end - fineibt_preamble_start) +#define fineibt_preamble_hash 7 + +asm( ".pushsection .rodata \n" + "fineibt_caller_start: \n" + " movl $0x12345678, %r10d \n" + " sub $16, %r11 \n" + ASM_NOP4 + "fineibt_caller_end: \n" + ".popsection \n" +); + +extern u8 fineibt_caller_start[]; +extern u8 fineibt_caller_end[]; + +#define fineibt_caller_size (fineibt_caller_end - fineibt_caller_start) +#define fineibt_caller_hash 2 + +#define fineibt_caller_jmp (fineibt_caller_size - 2) + +static u32 decode_preamble_hash(void *addr) +{ + u8 *p = addr; + + /* b8 78 56 34 12 mov $0x12345678,%eax */ + if (p[0] == 0xb8) + return *(u32 *)(addr + 1); + + return 0; /* invalid hash value */ +} + +static u32 decode_caller_hash(void *addr) +{ + u8 *p = addr; + + /* 41 ba 78 56 34 12 mov $0x12345678,%r10d */ + if (p[0] == 0x41 && p[1] == 0xba) + return -*(u32 *)(addr + 2); + + /* e8 0c 78 56 34 12 jmp.d8 +12 */ + if (p[0] == JMP8_INSN_OPCODE && p[1] == fineibt_caller_jmp) + return -*(u32 *)(addr + 2); + + return 0; /* invalid hash value */ +} + +/* .retpoline_sites */ +static int cfi_disable_callers(s32 *start, s32 *end) +{ + /* + * Disable kCFI by patching in a JMP.d8, this leaves the hash immediate + * in tact for later usage. Also see decode_caller_hash() and + * cfi_rewrite_callers(). + */ + const u8 jmp[] = { JMP8_INSN_OPCODE, fineibt_caller_jmp }; + s32 *s; - if (WARN_ON_ONCE(!is_endbr(endbr))) + for (s = start; s < end; s++) { + void *addr = (void *)s + *s; + u32 hash; + + addr -= fineibt_caller_size; + hash = decode_caller_hash(addr); + if (!hash) /* nocfi callers */ continue; - DPRINTK("ENDBR at: %pS (%px)", addr, addr); + text_poke_early(addr, jmp, 2); + } - /* - * When we have IBT, the lack of ENDBR will trigger #CP - */ - DUMP_BYTES(((u8*)addr), 4, "%px: orig: ", addr); - DUMP_BYTES(((u8*)&poison), 4, "%px: repl: ", addr); - text_poke_early(addr, &poison, 4); + return 0; +} + +/* .cfi_sites */ +static int cfi_rewrite_preamble(s32 *start, s32 *end) +{ + s32 *s; + + for (s = start; s < end; s++) { + void *addr = (void *)s + *s; + u32 hash; + + hash = decode_preamble_hash(addr); + if (WARN(!hash, "no CFI hash found at: %pS %px %*ph\n", + addr, addr, 5, addr)) + return -EINVAL; + + 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); } + + return 0; +} + +/* .retpoline_sites */ +static int cfi_rewrite_callers(s32 *start, s32 *end) +{ + s32 *s; + + for (s = start; s < end; s++) { + void *addr = (void *)s + *s; + u32 hash; + + addr -= fineibt_caller_size; + hash = decode_caller_hash(addr); + if (hash) { + text_poke_early(addr, fineibt_caller_start, fineibt_caller_size); + WARN_ON(*(u32 *)(addr + fineibt_caller_hash) != 0x12345678); + text_poke_early(addr + fineibt_caller_hash, &hash, 4); + } + /* rely on apply_retpolines() */ + } + + return 0; +} + +static void __apply_fineibt(s32 *start_retpoline, s32 *end_retpoline, + s32 *start_cfi, s32 *end_cfi, bool builtin) +{ + int ret; + + if (WARN_ONCE(fineibt_preamble_size != 16, + "FineIBT preamble wrong size: %ld", fineibt_preamble_size)) + return; + + if (!HAS_KERNEL_IBT || !cpu_feature_enabled(X86_FEATURE_IBT)) + return; + + /* + * Rewrite the callers to not use the __cfi_ stubs, such that we might + * rewrite them. This disables all CFI. If this succeeds but any of the + * later stages fails, we're without CFI. + */ + ret = cfi_disable_callers(start_retpoline, end_retpoline); + if (ret) + goto err; + + ret = cfi_rewrite_preamble(start_cfi, end_cfi); + if (ret) + goto err; + + ret = cfi_rewrite_callers(start_retpoline, end_retpoline); + if (ret) + goto err; + + if (builtin) + pr_info("Using FineIBT CFI\n"); + + return; + +err: + pr_err("Something went horribly wrong trying to rewrite the CFI implementation.\n"); } #else -void __init_or_module noinline apply_ibt_endbr(s32 *start, s32 *end) { } +static void __apply_fineibt(s32 *start_retpoline, s32 *end_retpoline, + s32 *start_cfi, s32 *end_cfi, bool builtin) +{ +} -#endif /* CONFIG_X86_KERNEL_IBT */ +#endif + +void apply_fineibt(s32 *start_retpoline, s32 *end_retpoline, + s32 *start_cfi, s32 *end_cfi) +{ + return __apply_fineibt(start_retpoline, end_retpoline, + start_cfi, end_cfi, + /* .builtin = */ false); +} #ifdef CONFIG_SMP static void alternatives_smp_lock(const s32 *start, const s32 *end, @@ -996,6 +1220,9 @@ void __init alternative_instructions(void) */ apply_paravirt(__parainstructions, __parainstructions_end); + __apply_fineibt(__retpoline_sites, __retpoline_sites_end, + __cfi_sites, __cfi_sites_end, true); + /* * Rewrite the retpolines, must be done before alternatives since * those can rewrite the retpoline thunks. diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c index 2bec4b4..423a760 100644 --- a/arch/x86/kernel/cpu/common.c +++ b/arch/x86/kernel/cpu/common.c @@ -609,6 +609,7 @@ static __always_inline void setup_cet(struct cpuinfo_x86 *c) if (!ibt_selftest()) { pr_err("IBT selftest: Failed!\n"); + wrmsrl(MSR_IA32_S_CET, 0); setup_clear_cpu_cap(X86_FEATURE_IBT); return; } diff --git a/arch/x86/kernel/module.c b/arch/x86/kernel/module.c index 2fb9de2..0142982 100644 --- a/arch/x86/kernel/module.c +++ b/arch/x86/kernel/module.c @@ -255,7 +255,7 @@ int module_finalize(const Elf_Ehdr *hdr, const Elf_Shdr *s, *text = NULL, *alt = NULL, *locks = NULL, *para = NULL, *orc = NULL, *orc_ip = NULL, *retpolines = NULL, *returns = NULL, *ibt_endbr = NULL, - *calls = NULL; + *calls = NULL, *cfi = NULL; char *secstrings = (void *)hdr + sechdrs[hdr->e_shstrndx].sh_offset; for (s = sechdrs; s < sechdrs + hdr->e_shnum; s++) { @@ -277,6 +277,8 @@ int module_finalize(const Elf_Ehdr *hdr, returns = s; if (!strcmp(".call_sites", secstrings + s->sh_name)) calls = s; + if (!strcmp(".cfi_sites", secstrings + s->sh_name)) + cfi = s; if (!strcmp(".ibt_endbr_seal", secstrings + s->sh_name)) ibt_endbr = s; } @@ -289,6 +291,22 @@ int module_finalize(const Elf_Ehdr *hdr, void *pseg = (void *)para->sh_addr; apply_paravirt(pseg, pseg + para->sh_size); } + if (retpolines || cfi) { + void *rseg = NULL, *cseg = NULL; + unsigned int rsize = 0, csize = 0; + + if (retpolines) { + rseg = (void *)retpolines->sh_addr; + rsize = retpolines->sh_size; + } + + if (cfi) { + cseg = (void *)cfi->sh_addr; + csize = cfi->sh_size; + } + + apply_fineibt(rseg, rseg + rsize, cseg, cseg + csize); + } if (retpolines) { void *rseg = (void *)retpolines->sh_addr; apply_retpolines(rseg, rseg + retpolines->sh_size); diff --git a/arch/x86/kernel/vmlinux.lds.S b/arch/x86/kernel/vmlinux.lds.S index 49f3f86..2e0ee14 100644 --- a/arch/x86/kernel/vmlinux.lds.S +++ b/arch/x86/kernel/vmlinux.lds.S @@ -309,6 +309,15 @@ SECTIONS } #endif +#ifdef CONFIG_FINEIBT + . = ALIGN(8); + .cfi_sites : AT(ADDR(.cfi_sites) - LOAD_OFFSET) { + __cfi_sites = .; + *(.cfi_sites) + __cfi_sites_end = .; + } +#endif + /* * struct alt_inst entries. From the header (alternative.h): * "Alternative instructions for different CPU types or capabilities" diff --git a/include/linux/bpf.h b/include/linux/bpf.h index 5296aea..923a3d5 100644 --- a/include/linux/bpf.h +++ b/include/linux/bpf.h @@ -984,7 +984,7 @@ int arch_prepare_bpf_dispatcher(void *image, void *buf, s64 *funcs, int num_func } #ifdef CONFIG_X86_64 -#ifdef CONFIG_CALL_THUNKS +#ifdef CONFIG_CALL_PADDING #define BPF_DISPATCHER_ATTRIBUTES __attribute__((patchable_function_entry(5+CONFIG_FUNCTION_PADDING_BYTES,CONFIG_FUNCTION_PADDING_BYTES))) #else #define BPF_DISPATCHER_ATTRIBUTES __attribute__((patchable_function_entry(5))) diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib index 2e03bcb..2b2fab7 100644 --- a/scripts/Makefile.lib +++ b/scripts/Makefile.lib @@ -256,6 +256,7 @@ objtool-args-$(CONFIG_HAVE_JUMP_LABEL_HACK) += --hacks=jump_label objtool-args-$(CONFIG_HAVE_NOINSTR_HACK) += --hacks=noinstr objtool-args-$(CONFIG_CALL_DEPTH_TRACKING) += --hacks=skylake objtool-args-$(CONFIG_X86_KERNEL_IBT) += --ibt +objtool-args-$(CONFIG_FINEIBT) += --cfi objtool-args-$(CONFIG_FTRACE_MCOUNT_USE_OBJTOOL) += --mcount objtool-args-$(CONFIG_UNWINDER_ORC) += --orc objtool-args-$(CONFIG_RETPOLINE) += --retpoline ^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH 3/4] x86/cfi: Boot time selection of CFI scheme 2022-10-27 9:28 [PATCH 0/4] x86/ibt: Implement FineIBT Peter Zijlstra 2022-10-27 9:28 ` [PATCH 1/4] objtool: Add --cfi to generate the .cfi_sites section Peter Zijlstra 2022-10-27 9:28 ` [PATCH 2/4] x86/ibt: Implement FineIBT Peter Zijlstra @ 2022-10-27 9:28 ` Peter Zijlstra 2022-10-28 17:41 ` Kees Cook 2022-11-02 9:19 ` [tip: x86/core] " tip-bot2 for Peter Zijlstra 2022-10-27 9:28 ` [PATCH 4/4] x86/cfi: Add boot time hash randomization Peter Zijlstra 2022-10-28 11:01 ` [PATCH 0/4] x86/ibt: Implement FineIBT David Laight 4 siblings, 2 replies; 15+ messages in thread From: Peter Zijlstra @ 2022-10-27 9:28 UTC (permalink / raw) To: x86 Cc: linux-kernel, peterz, Kees Cook, Sami Tolvanen, Joao Moreira, Josh Poimboeuf, Mark Rutland Add the "cfi=" boot parameter to allow people to select a CFI scheme at boot time. Mostly useful for development / debugging. Requested-by: Kees Cook <keescook@chromium.org> Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org> --- arch/x86/kernel/alternative.c | 103 +++++++++++++++++++++++++++++++++--------- 1 file changed, 83 insertions(+), 20 deletions(-) --- a/arch/x86/kernel/alternative.c +++ b/arch/x86/kernel/alternative.c @@ -702,6 +702,47 @@ void __init_or_module noinline apply_ibt #endif /* CONFIG_X86_KERNEL_IBT */ #ifdef CONFIG_FINEIBT + +enum cfi_mode { + CFI_DEFAULT, + CFI_OFF, + CFI_KCFI, + CFI_FINEIBT, +}; + +static enum cfi_mode cfi_mode __ro_after_init = CFI_DEFAULT; + +static __init int cfi_parse_cmdline(char *str) +{ + if (!str) + return -EINVAL; + + while (str) { + char *next = strchr(str, ','); + if (next) { + *next = 0; + next++; + } + + if (!strcmp(str, "auto")) { + cfi_mode = CFI_DEFAULT; + } else if (!strcmp(str, "off")) { + cfi_mode = CFI_OFF; + } else if (!strcmp(str, "kcfi")) { + cfi_mode = CFI_KCFI; + } else if (!strcmp(str, "fineibt")) { + cfi_mode = CFI_FINEIBT; + } else { + pr_err("Ignoring unknown cfi option (%s).", str); + } + + str = next; + } + + return 0; +} +early_param("cfi", cfi_parse_cmdline); + /* * kCFI FineIBT * @@ -868,30 +909,52 @@ static void __apply_fineibt(s32 *start_r "FineIBT preamble wrong size: %ld", fineibt_preamble_size)) return; - if (!HAS_KERNEL_IBT || !cpu_feature_enabled(X86_FEATURE_IBT)) + if (cfi_mode == CFI_DEFAULT) { + cfi_mode = CFI_KCFI; + if (HAS_KERNEL_IBT && cpu_feature_enabled(X86_FEATURE_IBT)) + cfi_mode = CFI_FINEIBT; + } + + switch (cfi_mode) { + case CFI_OFF: + ret = cfi_disable_callers(start_retpoline, end_retpoline); + if (ret) + goto err; + + if (builtin) + pr_info("Disabling CFI\n"); return; - /* - * Rewrite the callers to not use the __cfi_ stubs, such that we might - * rewrite them. This disables all CFI. If this succeeds but any of the - * later stages fails, we're without CFI. - */ - ret = cfi_disable_callers(start_retpoline, end_retpoline); - if (ret) - goto err; - - ret = cfi_rewrite_preamble(start_cfi, end_cfi); - if (ret) - goto err; - - ret = cfi_rewrite_callers(start_retpoline, end_retpoline); - if (ret) - goto err; + case CFI_KCFI: + if (builtin) + pr_info("Using kCFI\n"); + return; - if (builtin) - pr_info("Using FineIBT CFI\n"); + case CFI_FINEIBT: + /* + * Rewrite the callers to not use the __cfi_ stubs, such that we might + * rewrite them. This disables all CFI. If this succeeds but any of the + * later stages fails, we're without CFI. + */ + ret = cfi_disable_callers(start_retpoline, end_retpoline); + if (ret) + goto err; + + ret = cfi_rewrite_preamble(start_cfi, end_cfi); + if (ret) + goto err; + + ret = cfi_rewrite_callers(start_retpoline, end_retpoline); + if (ret) + goto err; - return; + if (builtin) + pr_info("Using FineIBT CFI\n"); + return; + + default: + break; + } err: pr_err("Something went horribly wrong trying to rewrite the CFI implementation.\n"); ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 3/4] x86/cfi: Boot time selection of CFI scheme 2022-10-27 9:28 ` [PATCH 3/4] x86/cfi: Boot time selection of CFI scheme Peter Zijlstra @ 2022-10-28 17:41 ` Kees Cook 2022-11-02 9:19 ` [tip: x86/core] " tip-bot2 for Peter Zijlstra 1 sibling, 0 replies; 15+ messages in thread From: Kees Cook @ 2022-10-28 17:41 UTC (permalink / raw) To: Peter Zijlstra Cc: x86, linux-kernel, Sami Tolvanen, Joao Moreira, Josh Poimboeuf, Mark Rutland On Thu, Oct 27, 2022 at 11:28:15AM +0200, Peter Zijlstra wrote: > Add the "cfi=" boot parameter to allow people to select a CFI scheme > at boot time. Mostly useful for development / debugging. > > Requested-by: Kees Cook <keescook@chromium.org> > Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org> Reviewed-by: Kees Cook <keescook@chromium.org> -- Kees Cook ^ permalink raw reply [flat|nested] 15+ messages in thread
* [tip: x86/core] x86/cfi: Boot time selection of CFI scheme 2022-10-27 9:28 ` [PATCH 3/4] x86/cfi: Boot time selection of CFI scheme Peter Zijlstra 2022-10-28 17:41 ` Kees Cook @ 2022-11-02 9:19 ` tip-bot2 for Peter Zijlstra 1 sibling, 0 replies; 15+ messages in thread From: tip-bot2 for Peter Zijlstra @ 2022-11-02 9:19 UTC (permalink / raw) To: linux-tip-commits; +Cc: Peter Zijlstra (Intel), Kees Cook, x86, linux-kernel The following commit has been merged into the x86/core branch of tip: Commit-ID: 082c4c815252ea333b0f3a51e336df60c2314fe2 Gitweb: https://git.kernel.org/tip/082c4c815252ea333b0f3a51e336df60c2314fe2 Author: Peter Zijlstra <peterz@infradead.org> AuthorDate: Thu, 27 Oct 2022 11:28:15 +02:00 Committer: Peter Zijlstra <peterz@infradead.org> CommitterDate: Tue, 01 Nov 2022 13:44:11 +01:00 x86/cfi: Boot time selection of CFI scheme Add the "cfi=" boot parameter to allow people to select a CFI scheme at boot time. Mostly useful for development / debugging. Requested-by: Kees Cook <keescook@chromium.org> Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org> Reviewed-by: Kees Cook <keescook@chromium.org> Link: https://lore.kernel.org/r/20221027092842.699804264@infradead.org --- arch/x86/kernel/alternative.c | 99 +++++++++++++++++++++++++++------ 1 file changed, 81 insertions(+), 18 deletions(-) diff --git a/arch/x86/kernel/alternative.c b/arch/x86/kernel/alternative.c index 91b0e63..9d3b587 100644 --- a/arch/x86/kernel/alternative.c +++ b/arch/x86/kernel/alternative.c @@ -702,6 +702,47 @@ void __init_or_module noinline apply_ibt_endbr(s32 *start, s32 *end) { } #endif /* CONFIG_X86_KERNEL_IBT */ #ifdef CONFIG_FINEIBT + +enum cfi_mode { + CFI_DEFAULT, + CFI_OFF, + CFI_KCFI, + CFI_FINEIBT, +}; + +static enum cfi_mode cfi_mode __ro_after_init = CFI_DEFAULT; + +static __init int cfi_parse_cmdline(char *str) +{ + if (!str) + return -EINVAL; + + while (str) { + char *next = strchr(str, ','); + if (next) { + *next = 0; + next++; + } + + if (!strcmp(str, "auto")) { + cfi_mode = CFI_DEFAULT; + } else if (!strcmp(str, "off")) { + cfi_mode = CFI_OFF; + } else if (!strcmp(str, "kcfi")) { + cfi_mode = CFI_KCFI; + } else if (!strcmp(str, "fineibt")) { + cfi_mode = CFI_FINEIBT; + } else { + pr_err("Ignoring unknown cfi option (%s).", str); + } + + str = next; + } + + return 0; +} +early_param("cfi", cfi_parse_cmdline); + /* * kCFI FineIBT * @@ -868,30 +909,52 @@ static void __apply_fineibt(s32 *start_retpoline, s32 *end_retpoline, "FineIBT preamble wrong size: %ld", fineibt_preamble_size)) return; - if (!HAS_KERNEL_IBT || !cpu_feature_enabled(X86_FEATURE_IBT)) + if (cfi_mode == CFI_DEFAULT) { + cfi_mode = CFI_KCFI; + if (HAS_KERNEL_IBT && cpu_feature_enabled(X86_FEATURE_IBT)) + cfi_mode = CFI_FINEIBT; + } + + switch (cfi_mode) { + case CFI_OFF: + ret = cfi_disable_callers(start_retpoline, end_retpoline); + if (ret) + goto err; + + if (builtin) + pr_info("Disabling CFI\n"); return; - /* - * Rewrite the callers to not use the __cfi_ stubs, such that we might - * rewrite them. This disables all CFI. If this succeeds but any of the - * later stages fails, we're without CFI. - */ - ret = cfi_disable_callers(start_retpoline, end_retpoline); - if (ret) - goto err; + case CFI_KCFI: + if (builtin) + pr_info("Using kCFI\n"); + return; - ret = cfi_rewrite_preamble(start_cfi, end_cfi); - if (ret) - goto err; + case CFI_FINEIBT: + /* + * Rewrite the callers to not use the __cfi_ stubs, such that we might + * rewrite them. This disables all CFI. If this succeeds but any of the + * later stages fails, we're without CFI. + */ + ret = cfi_disable_callers(start_retpoline, end_retpoline); + if (ret) + goto err; + + ret = cfi_rewrite_preamble(start_cfi, end_cfi); + if (ret) + goto err; - ret = cfi_rewrite_callers(start_retpoline, end_retpoline); - if (ret) - goto err; + ret = cfi_rewrite_callers(start_retpoline, end_retpoline); + if (ret) + goto err; - if (builtin) - pr_info("Using FineIBT CFI\n"); + if (builtin) + pr_info("Using FineIBT CFI\n"); + return; - return; + default: + break; + } err: pr_err("Something went horribly wrong trying to rewrite the CFI implementation.\n"); ^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH 4/4] x86/cfi: Add boot time hash randomization 2022-10-27 9:28 [PATCH 0/4] x86/ibt: Implement FineIBT Peter Zijlstra ` (2 preceding siblings ...) 2022-10-27 9:28 ` [PATCH 3/4] x86/cfi: Boot time selection of CFI scheme Peter Zijlstra @ 2022-10-27 9:28 ` Peter Zijlstra 2022-10-28 17:42 ` Kees Cook 2022-11-02 9:19 ` [tip: x86/core] " tip-bot2 for Peter Zijlstra 2022-10-28 11:01 ` [PATCH 0/4] x86/ibt: Implement FineIBT David Laight 4 siblings, 2 replies; 15+ messages in thread From: Peter Zijlstra @ 2022-10-27 9:28 UTC (permalink / raw) To: x86 Cc: linux-kernel, peterz, Kees Cook, Sami Tolvanen, Joao Moreira, Josh Poimboeuf, Mark Rutland In order to avoid known hashes (from knowing the boot image), randomize the CFI hashes with a per-boot random seed. Suggested-by: Kees Cook <keescook@chromium.org> Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org> --- arch/x86/kernel/alternative.c | 120 +++++++++++++++++++++++++++++++++++++----- 1 file changed, 108 insertions(+), 12 deletions(-) --- a/arch/x86/kernel/alternative.c +++ b/arch/x86/kernel/alternative.c @@ -711,6 +711,24 @@ enum cfi_mode { }; static enum cfi_mode cfi_mode __ro_after_init = CFI_DEFAULT; +static bool cfi_rand __ro_after_init = true; +static u32 cfi_seed __ro_after_init; + +/* + * Re-hash the CFI hash with a boot-time seed while making sure the result is + * not a valid ENDBR instruction. + */ +static u32 cfi_rehash(u32 hash) +{ + hash ^= cfi_seed; + while (unlikely(is_endbr(hash) || is_endbr(-hash))) { + bool lsb = hash & 1; + hash >>= 1; + if (lsb) + hash ^= 0x80200003; + } + return hash; +} static __init int cfi_parse_cmdline(char *str) { @@ -728,10 +746,13 @@ static __init int cfi_parse_cmdline(char cfi_mode = CFI_DEFAULT; } else if (!strcmp(str, "off")) { cfi_mode = CFI_OFF; + cfi_rand = false; } else if (!strcmp(str, "kcfi")) { cfi_mode = CFI_KCFI; } else if (!strcmp(str, "fineibt")) { cfi_mode = CFI_FINEIBT; + } else if (!strcmp(str, "norand")) { + cfi_rand = false; } else { pr_err("Ignoring unknown cfi option (%s).", str); } @@ -856,7 +877,50 @@ static int cfi_disable_callers(s32 *star return 0; } +static int cfi_enable_callers(s32 *start, s32 *end) +{ + /* + * Re-enable kCFI, undo what cfi_disable_callers() did. + */ + const u8 mov[] = { 0x41, 0xba }; + s32 *s; + + for (s = start; s < end; s++) { + void *addr = (void *)s + *s; + u32 hash; + + addr -= fineibt_caller_size; + hash = decode_caller_hash(addr); + if (!hash) /* nocfi callers */ + continue; + + text_poke_early(addr, mov, 2); + } + + return 0; +} + /* .cfi_sites */ +static int cfi_rand_preamble(s32 *start, s32 *end) +{ + s32 *s; + + for (s = start; s < end; s++) { + void *addr = (void *)s + *s; + u32 hash; + + hash = decode_preamble_hash(addr); + if (WARN(!hash, "no CFI hash found at: %pS %px %*ph\n", + addr, addr, 5, addr)) + return -EINVAL; + + hash = cfi_rehash(hash); + text_poke_early(addr + 1, &hash, 4); + } + + return 0; +} + static int cfi_rewrite_preamble(s32 *start, s32 *end) { s32 *s; @@ -879,6 +943,25 @@ static int cfi_rewrite_preamble(s32 *sta } /* .retpoline_sites */ +static int cfi_rand_callers(s32 *start, s32 *end) +{ + s32 *s; + + for (s = start; s < end; s++) { + void *addr = (void *)s + *s; + u32 hash; + + addr -= fineibt_caller_size; + hash = decode_caller_hash(addr); + if (hash) { + hash = -cfi_rehash(hash); + text_poke_early(addr + 2, &hash, 4); + } + } + + return 0; +} + static int cfi_rewrite_callers(s32 *start, s32 *end) { s32 *s; @@ -915,31 +998,44 @@ static void __apply_fineibt(s32 *start_r cfi_mode = CFI_FINEIBT; } - switch (cfi_mode) { - case CFI_OFF: - ret = cfi_disable_callers(start_retpoline, end_retpoline); + /* + * Rewrite the callers to not use the __cfi_ stubs, such that we might + * rewrite them. This disables all CFI. If this succeeds but any of the + * later stages fails, we're without CFI. + */ + ret = cfi_disable_callers(start_retpoline, end_retpoline); + if (ret) + goto err; + + if (cfi_rand) { + if (builtin) + cfi_seed = get_random_u32(); + + ret = cfi_rand_preamble(start_cfi, end_cfi); if (ret) goto err; + ret = cfi_rand_callers(start_retpoline, end_retpoline); + if (ret) + goto err; + } + + switch (cfi_mode) { + case CFI_OFF: if (builtin) pr_info("Disabling CFI\n"); return; case CFI_KCFI: + ret = cfi_enable_callers(start_retpoline, end_retpoline); + if (ret) + goto err; + if (builtin) pr_info("Using kCFI\n"); return; case CFI_FINEIBT: - /* - * Rewrite the callers to not use the __cfi_ stubs, such that we might - * rewrite them. This disables all CFI. If this succeeds but any of the - * later stages fails, we're without CFI. - */ - ret = cfi_disable_callers(start_retpoline, end_retpoline); - if (ret) - goto err; - ret = cfi_rewrite_preamble(start_cfi, end_cfi); if (ret) goto err; ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 4/4] x86/cfi: Add boot time hash randomization 2022-10-27 9:28 ` [PATCH 4/4] x86/cfi: Add boot time hash randomization Peter Zijlstra @ 2022-10-28 17:42 ` Kees Cook 2022-11-02 9:19 ` [tip: x86/core] " tip-bot2 for Peter Zijlstra 1 sibling, 0 replies; 15+ messages in thread From: Kees Cook @ 2022-10-28 17:42 UTC (permalink / raw) To: Peter Zijlstra Cc: x86, linux-kernel, Sami Tolvanen, Joao Moreira, Josh Poimboeuf, Mark Rutland On Thu, Oct 27, 2022 at 11:28:16AM +0200, Peter Zijlstra wrote: > In order to avoid known hashes (from knowing the boot image), > randomize the CFI hashes with a per-boot random seed. > > Suggested-by: Kees Cook <keescook@chromium.org> > Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org> > --- > arch/x86/kernel/alternative.c | 120 +++++++++++++++++++++++++++++++++++++----- > 1 file changed, 108 insertions(+), 12 deletions(-) > > --- a/arch/x86/kernel/alternative.c > +++ b/arch/x86/kernel/alternative.c > @@ -711,6 +711,24 @@ enum cfi_mode { > }; > > static enum cfi_mode cfi_mode __ro_after_init = CFI_DEFAULT; > +static bool cfi_rand __ro_after_init = true; > +static u32 cfi_seed __ro_after_init; > + > +/* > + * Re-hash the CFI hash with a boot-time seed while making sure the result is > + * not a valid ENDBR instruction. > + */ > +static u32 cfi_rehash(u32 hash) > +{ > + hash ^= cfi_seed; > + while (unlikely(is_endbr(hash) || is_endbr(-hash))) { > + bool lsb = hash & 1; > + hash >>= 1; > + if (lsb) > + hash ^= 0x80200003; > + } > + return hash; > +} I guess this risks hash collision with existing hashes, but meeeh. I'm glad to have the randomization. :) Reviewed-by: Kees Cook <keescook@chromium.org> -- Kees Cook ^ permalink raw reply [flat|nested] 15+ messages in thread
* [tip: x86/core] x86/cfi: Add boot time hash randomization 2022-10-27 9:28 ` [PATCH 4/4] x86/cfi: Add boot time hash randomization Peter Zijlstra 2022-10-28 17:42 ` Kees Cook @ 2022-11-02 9:19 ` tip-bot2 for Peter Zijlstra 1 sibling, 0 replies; 15+ messages in thread From: tip-bot2 for Peter Zijlstra @ 2022-11-02 9:19 UTC (permalink / raw) To: linux-tip-commits; +Cc: Kees Cook, Peter Zijlstra (Intel), x86, linux-kernel The following commit has been merged into the x86/core branch of tip: Commit-ID: 0c3e806ec0f9771fa1f34c60499097d9260a8bb7 Gitweb: https://git.kernel.org/tip/0c3e806ec0f9771fa1f34c60499097d9260a8bb7 Author: Peter Zijlstra <peterz@infradead.org> AuthorDate: Thu, 27 Oct 2022 11:28:16 +02:00 Committer: Peter Zijlstra <peterz@infradead.org> CommitterDate: Tue, 01 Nov 2022 13:44:11 +01:00 x86/cfi: Add boot time hash randomization In order to avoid known hashes (from knowing the boot image), randomize the CFI hashes with a per-boot random seed. Suggested-by: Kees Cook <keescook@chromium.org> Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org> Reviewed-by: Kees Cook <keescook@chromium.org> Link: https://lore.kernel.org/r/20221027092842.765195516@infradead.org --- arch/x86/kernel/alternative.c | 120 +++++++++++++++++++++++++++++---- 1 file changed, 108 insertions(+), 12 deletions(-) diff --git a/arch/x86/kernel/alternative.c b/arch/x86/kernel/alternative.c index 9d3b587..aa7f791 100644 --- a/arch/x86/kernel/alternative.c +++ b/arch/x86/kernel/alternative.c @@ -711,6 +711,24 @@ enum cfi_mode { }; static enum cfi_mode cfi_mode __ro_after_init = CFI_DEFAULT; +static bool cfi_rand __ro_after_init = true; +static u32 cfi_seed __ro_after_init; + +/* + * Re-hash the CFI hash with a boot-time seed while making sure the result is + * not a valid ENDBR instruction. + */ +static u32 cfi_rehash(u32 hash) +{ + hash ^= cfi_seed; + while (unlikely(is_endbr(hash) || is_endbr(-hash))) { + bool lsb = hash & 1; + hash >>= 1; + if (lsb) + hash ^= 0x80200003; + } + return hash; +} static __init int cfi_parse_cmdline(char *str) { @@ -728,10 +746,13 @@ static __init int cfi_parse_cmdline(char *str) cfi_mode = CFI_DEFAULT; } else if (!strcmp(str, "off")) { cfi_mode = CFI_OFF; + cfi_rand = false; } else if (!strcmp(str, "kcfi")) { cfi_mode = CFI_KCFI; } else if (!strcmp(str, "fineibt")) { cfi_mode = CFI_FINEIBT; + } else if (!strcmp(str, "norand")) { + cfi_rand = false; } else { pr_err("Ignoring unknown cfi option (%s).", str); } @@ -856,7 +877,50 @@ static int cfi_disable_callers(s32 *start, s32 *end) return 0; } +static int cfi_enable_callers(s32 *start, s32 *end) +{ + /* + * Re-enable kCFI, undo what cfi_disable_callers() did. + */ + const u8 mov[] = { 0x41, 0xba }; + s32 *s; + + for (s = start; s < end; s++) { + void *addr = (void *)s + *s; + u32 hash; + + addr -= fineibt_caller_size; + hash = decode_caller_hash(addr); + if (!hash) /* nocfi callers */ + continue; + + text_poke_early(addr, mov, 2); + } + + return 0; +} + /* .cfi_sites */ +static int cfi_rand_preamble(s32 *start, s32 *end) +{ + s32 *s; + + for (s = start; s < end; s++) { + void *addr = (void *)s + *s; + u32 hash; + + hash = decode_preamble_hash(addr); + if (WARN(!hash, "no CFI hash found at: %pS %px %*ph\n", + addr, addr, 5, addr)) + return -EINVAL; + + hash = cfi_rehash(hash); + text_poke_early(addr + 1, &hash, 4); + } + + return 0; +} + static int cfi_rewrite_preamble(s32 *start, s32 *end) { s32 *s; @@ -879,6 +943,25 @@ static int cfi_rewrite_preamble(s32 *start, s32 *end) } /* .retpoline_sites */ +static int cfi_rand_callers(s32 *start, s32 *end) +{ + s32 *s; + + for (s = start; s < end; s++) { + void *addr = (void *)s + *s; + u32 hash; + + addr -= fineibt_caller_size; + hash = decode_caller_hash(addr); + if (hash) { + hash = -cfi_rehash(hash); + text_poke_early(addr + 2, &hash, 4); + } + } + + return 0; +} + static int cfi_rewrite_callers(s32 *start, s32 *end) { s32 *s; @@ -915,31 +998,44 @@ static void __apply_fineibt(s32 *start_retpoline, s32 *end_retpoline, cfi_mode = CFI_FINEIBT; } - switch (cfi_mode) { - case CFI_OFF: - ret = cfi_disable_callers(start_retpoline, end_retpoline); + /* + * Rewrite the callers to not use the __cfi_ stubs, such that we might + * rewrite them. This disables all CFI. If this succeeds but any of the + * later stages fails, we're without CFI. + */ + ret = cfi_disable_callers(start_retpoline, end_retpoline); + if (ret) + goto err; + + if (cfi_rand) { + if (builtin) + cfi_seed = get_random_u32(); + + ret = cfi_rand_preamble(start_cfi, end_cfi); if (ret) goto err; + ret = cfi_rand_callers(start_retpoline, end_retpoline); + if (ret) + goto err; + } + + switch (cfi_mode) { + case CFI_OFF: if (builtin) pr_info("Disabling CFI\n"); return; case CFI_KCFI: + ret = cfi_enable_callers(start_retpoline, end_retpoline); + if (ret) + goto err; + if (builtin) pr_info("Using kCFI\n"); return; case CFI_FINEIBT: - /* - * Rewrite the callers to not use the __cfi_ stubs, such that we might - * rewrite them. This disables all CFI. If this succeeds but any of the - * later stages fails, we're without CFI. - */ - ret = cfi_disable_callers(start_retpoline, end_retpoline); - if (ret) - goto err; - ret = cfi_rewrite_preamble(start_cfi, end_cfi); if (ret) goto err; ^ permalink raw reply related [flat|nested] 15+ messages in thread
* RE: [PATCH 0/4] x86/ibt: Implement FineIBT 2022-10-27 9:28 [PATCH 0/4] x86/ibt: Implement FineIBT Peter Zijlstra ` (3 preceding siblings ...) 2022-10-27 9:28 ` [PATCH 4/4] x86/cfi: Add boot time hash randomization Peter Zijlstra @ 2022-10-28 11:01 ` David Laight 2022-10-28 12:03 ` Peter Zijlstra 4 siblings, 1 reply; 15+ messages in thread From: David Laight @ 2022-10-28 11:01 UTC (permalink / raw) To: 'Peter Zijlstra', x86 Cc: linux-kernel, Kees Cook, Sami Tolvanen, Joao Moreira, Josh Poimboeuf, Mark Rutland From: Peter Zijlstra > Sent: 27 October 2022 10:28 > > Hi all, > > Updated FineIBT series; I've (hopefully) incorporated all feedback from last > time with the notable exception of the Kconfig CFI default -- I'm not sure we > want to add to the Kconfig space for this, also what would a distro do with it. > > Anyway; please have a look, I'm hoping to merge this soonish so we can make the > next cycle. Is there a test to ensure that modules are actually compiled with the required endbra, function prologue gap (etc). Having the module load fail is somewhat better than a crash. It is almost certainly quite easy to generate an out of tree module that is missing all of those (even if compiled at the same time as the kernel). (Never mind issues with modules that contain binary blobs.) David - Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK Registration No: 1397386 (Wales) ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 0/4] x86/ibt: Implement FineIBT 2022-10-28 11:01 ` [PATCH 0/4] x86/ibt: Implement FineIBT David Laight @ 2022-10-28 12:03 ` Peter Zijlstra 0 siblings, 0 replies; 15+ messages in thread From: Peter Zijlstra @ 2022-10-28 12:03 UTC (permalink / raw) To: David Laight Cc: x86, linux-kernel, Kees Cook, Sami Tolvanen, Joao Moreira, Josh Poimboeuf, Mark Rutland On Fri, Oct 28, 2022 at 11:01:08AM +0000, David Laight wrote: > From: Peter Zijlstra > > Sent: 27 October 2022 10:28 > > > > Hi all, > > > > Updated FineIBT series; I've (hopefully) incorporated all feedback from last > > time with the notable exception of the Kconfig CFI default -- I'm not sure we > > want to add to the Kconfig space for this, also what would a distro do with it. > > > > Anyway; please have a look, I'm hoping to merge this soonish so we can make the > > next cycle. > > Is there a test to ensure that modules are actually compiled > with the required endbra, function prologue gap (etc). > Having the module load fail is somewhat better than a crash. > > It is almost certainly quite easy to generate an out of tree module that > is missing all of those (even if compiled at the same time as the kernel). > (Never mind issues with modules that contain binary blobs.) There is not; it is always possible to load a 'malformed' module. We have no sanity checking on modules. It is no different from any other binary compatilibity issue; if you build a dud module, you get to keep the pieces. ^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2022-11-02 9:22 UTC | newest] Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-10-27 9:28 [PATCH 0/4] x86/ibt: Implement FineIBT Peter Zijlstra 2022-10-27 9:28 ` [PATCH 1/4] objtool: Add --cfi to generate the .cfi_sites section Peter Zijlstra 2022-11-02 9:20 ` [tip: x86/core] " tip-bot2 for Peter Zijlstra 2022-10-27 9:28 ` [PATCH 2/4] x86/ibt: Implement FineIBT Peter Zijlstra 2022-10-27 10:11 ` Peter Zijlstra 2022-10-28 17:41 ` Kees Cook 2022-11-02 9:20 ` [tip: x86/core] " tip-bot2 for Peter Zijlstra 2022-10-27 9:28 ` [PATCH 3/4] x86/cfi: Boot time selection of CFI scheme Peter Zijlstra 2022-10-28 17:41 ` Kees Cook 2022-11-02 9:19 ` [tip: x86/core] " tip-bot2 for Peter Zijlstra 2022-10-27 9:28 ` [PATCH 4/4] x86/cfi: Add boot time hash randomization Peter Zijlstra 2022-10-28 17:42 ` Kees Cook 2022-11-02 9:19 ` [tip: x86/core] " tip-bot2 for Peter Zijlstra 2022-10-28 11:01 ` [PATCH 0/4] x86/ibt: Implement FineIBT David Laight 2022-10-28 12:03 ` Peter Zijlstra
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.