| Message ID | 20250414113754.540779611@infradead.org |
|---|---|
| State | New |
| Headers | show |
| Series | objtool: Detect and warn about indirect calls in __nocfi functions | expand |
On Mon, Apr 14, 2025 at 01:11:46PM +0200, Peter Zijlstra wrote: > SYM_FUNC_START(__efi_call) > + /* > + * The EFI code doesn't have any CFI :-( > + */ > + ANNOTATE_NOCFI > pushq %rbp > movq %rsp, %rbp > and $~0xf, %rsp This looks like an insn annotation but is actually a func annotation. ANNOTATE_NOCFI_SYM() would be a lot clearer, for all the asm code. Though maybe it's better for ANNOTATE_NOCFI to annotate the call site itself (for asm) while ANNOTATE_NOCFI_SYM annotates the entire function (in C). So either there would be two separate annotypes or the annotation would get interpreted based on whether it's at the beginning of the function. > +++ b/include/linux/objtool.h > @@ -185,6 +185,8 @@ > */ > #define ANNOTATE_REACHABLE(label) __ASM_ANNOTATE(label, ANNOTYPE_REACHABLE) > > +#define ANNOTATE_NOCFI_SYM(sym) asm(__ASM_ANNOTATE(sym, ANNOTYPE_NOCFI)) This needs a comment like the others. > @@ -2436,6 +2438,15 @@ static int __annotate_late(struct objtoo > insn->_jump_table = (void *)1; > break; > > + case ANNOTYPE_NOCFI: > + sym = insn->sym; > + if (!sym) { > + WARN_INSN(insn, "dodgy NOCFI annotation"); > + break; Return an error? > @@ -4006,6 +4017,36 @@ static int validate_retpoline(struct obj > + /* > + * kCFI call sites look like: > + * > + * movl $(-0x12345678), %r10d > + * addl -4(%r11), %r10d > + * jz 1f > + * ud2 > + * 1: cs call __x86_indirect_thunk_r11 > + * > + * Verify all indirect calls are kCFI adorned by checking for the > + * UD2. Notably, doing __nocfi calls to regular (cfi) functions is > + * broken. This "__nocfi calls" is confusing me. IIUC, there are two completely different meanings for "nocfi": - __nocfi: disable the kcfi function entry stuff - ANNOTATE_NOCFI_SYM: Regardless of whether the function is __nocfi, allow it to have non-CFI indirect call sites. Can we call this ANNOTATE_NOCFI_SAFE or something?
On Mon, Apr 14, 2025 at 04:43:26PM -0700, Josh Poimboeuf wrote: > > + * Verify all indirect calls are kCFI adorned by checking for the > > + * UD2. Notably, doing __nocfi calls to regular (cfi) functions is > > + * broken. > > This "__nocfi calls" is confusing me. IIUC, there are two completely > different meanings for "nocfi": > > - __nocfi: disable the kcfi function entry stuff Ah, no. __nocfi is a bit of a mess, this is both the function entry thing, but also very much the caller verification stuff for indirect calls done inside this function. This leads to lovely stuff like: void (*foo)(void); static __always_inline __nocfi void nocfi_caller(void) { foo(); } void bar(void) { nocfi_caller(); foo(); } This actually compiles and has bar() have two distinctly different indirect calls to foo, while bar itself has a __cfi preamble. Anyway, let me have a poke at the annotation.
On Mon, Apr 14, 2025 at 04:43:26PM -0700, Josh Poimboeuf wrote:
> Can we call this ANNOTATE_NOCFI_SAFE or something?
I'm hesitant to do so, because some of these sites really are not safe.
EFI and VMX interrupt crud really are a security issue.
EFI really is unfixable but not less brokene, because the EFI code is
out of our control, the VMX thing might be fixable, not sure I
understand KVM well enough.
--- a/arch/x86/kernel/machine_kexec_64.c +++ b/arch/x86/kernel/machine_kexec_64.c @@ -442,6 +442,7 @@ void __nocfi machine_kexec(struct kimage __ftrace_enabled_restore(save_ftrace_enabled); } +ANNOTATE_NOCFI_SYM(machine_kexec); /* arch-dependent functionality related to kexec file-based syscall */ --- a/arch/x86/kvm/emulate.c +++ b/arch/x86/kvm/emulate.c @@ -5071,6 +5071,10 @@ static noinline int fastop(struct x86_em return emulate_de(ctxt); return X86EMUL_CONTINUE; } +/* + * The ASM stubs don't have CFI on. + */ +ANNOTATE_NOCFI_SYM(fastop); void init_decode_cache(struct x86_emulate_ctxt *ctxt) { --- a/arch/x86/kvm/vmx/vmenter.S +++ b/arch/x86/kvm/vmx/vmenter.S @@ -363,5 +363,9 @@ SYM_FUNC_END(vmread_error_trampoline) .section .text, "ax" SYM_FUNC_START(vmx_do_interrupt_irqoff) + /* + * Calling an IDT gate directly. + */ + ANNOTATE_NOCFI VMX_DO_EVENT_IRQOFF CALL_NOSPEC _ASM_ARG1 SYM_FUNC_END(vmx_do_interrupt_irqoff) --- a/arch/x86/platform/efi/efi_stub_64.S +++ b/arch/x86/platform/efi/efi_stub_64.S @@ -11,6 +11,10 @@ #include <asm/nospec-branch.h> SYM_FUNC_START(__efi_call) + /* + * The EFI code doesn't have any CFI :-( + */ + ANNOTATE_NOCFI pushq %rbp movq %rsp, %rbp and $~0xf, %rsp --- a/drivers/misc/lkdtm/perms.c +++ b/drivers/misc/lkdtm/perms.c @@ -9,6 +9,7 @@ #include <linux/vmalloc.h> #include <linux/mman.h> #include <linux/uaccess.h> +#include <linux/objtool.h> #include <asm/cacheflush.h> #include <asm/sections.h> @@ -86,6 +87,7 @@ static noinline __nocfi void execute_loc func(); pr_err("FAIL: func returned\n"); } +ANNOTATE_NOCFI_SYM(execute_location); static void execute_user_location(void *dst) { --- a/include/linux/objtool.h +++ b/include/linux/objtool.h @@ -185,6 +185,8 @@ */ #define ANNOTATE_REACHABLE(label) __ASM_ANNOTATE(label, ANNOTYPE_REACHABLE) +#define ANNOTATE_NOCFI_SYM(sym) asm(__ASM_ANNOTATE(sym, ANNOTYPE_NOCFI)) + #else #define ANNOTATE_NOENDBR ANNOTATE type=ANNOTYPE_NOENDBR #define ANNOTATE_RETPOLINE_SAFE ANNOTATE type=ANNOTYPE_RETPOLINE_SAFE @@ -194,6 +196,7 @@ #define ANNOTATE_INTRA_FUNCTION_CALL ANNOTATE type=ANNOTYPE_INTRA_FUNCTION_CALL #define ANNOTATE_UNRET_BEGIN ANNOTATE type=ANNOTYPE_UNRET_BEGIN #define ANNOTATE_REACHABLE ANNOTATE type=ANNOTYPE_REACHABLE +#define ANNOTATE_NOCFI ANNOTATE type=ANNOTYPE_NOCFI #endif #if defined(CONFIG_NOINSTR_VALIDATION) && \ --- a/include/linux/objtool_types.h +++ b/include/linux/objtool_types.h @@ -66,5 +66,6 @@ struct unwind_hint { #define ANNOTYPE_INTRA_FUNCTION_CALL 7 #define ANNOTYPE_REACHABLE 8 #define ANNOTYPE_JUMP_TABLE 9 +#define ANNOTYPE_NOCFI 10 #endif /* _LINUX_OBJTOOL_TYPES_H */ --- a/tools/include/linux/objtool_types.h +++ b/tools/include/linux/objtool_types.h @@ -66,5 +66,6 @@ struct unwind_hint { #define ANNOTYPE_INTRA_FUNCTION_CALL 7 #define ANNOTYPE_REACHABLE 8 #define ANNOTYPE_JUMP_TABLE 9 +#define ANNOTYPE_NOCFI 10 #endif /* _LINUX_OBJTOOL_TYPES_H */ --- a/tools/objtool/check.c +++ b/tools/objtool/check.c @@ -2387,6 +2387,8 @@ static int __annotate_ifc(struct objtool static int __annotate_late(struct objtool_file *file, int type, struct instruction *insn) { + struct symbol *sym; + switch (type) { case ANNOTYPE_NOENDBR: /* early */ @@ -2436,6 +2438,15 @@ static int __annotate_late(struct objtoo insn->_jump_table = (void *)1; break; + case ANNOTYPE_NOCFI: + sym = insn->sym; + if (!sym) { + WARN_INSN(insn, "dodgy NOCFI annotation"); + break; + } + insn->sym->nocfi = 1; + break; + default: ERROR_INSN(insn, "Unknown annotation type: %d", type); return -1; @@ -4006,6 +4017,36 @@ static int validate_retpoline(struct obj warnings++; } + if (!opts.cfi) + return warnings; + + /* + * kCFI call sites look like: + * + * movl $(-0x12345678), %r10d + * addl -4(%r11), %r10d + * jz 1f + * ud2 + * 1: cs call __x86_indirect_thunk_r11 + * + * Verify all indirect calls are kCFI adorned by checking for the + * UD2. Notably, doing __nocfi calls to regular (cfi) functions is + * broken. + */ + list_for_each_entry(insn, &file->retpoline_call_list, call_node) { + struct symbol *sym = insn->sym; + + if (sym && sym->type == STT_FUNC && !sym->nocfi) { + struct instruction *prev = + prev_insn_same_sym(file, insn); + + if (!prev || prev->type != INSN_BUG) { + WARN_INSN(insn, "no-cfi indirect call!"); + warnings++; + } + } + } + return warnings; } --- a/tools/objtool/include/objtool/elf.h +++ b/tools/objtool/include/objtool/elf.h @@ -70,6 +70,7 @@ struct symbol { u8 local_label : 1; u8 frame_pointer : 1; u8 ignore : 1; + u8 nocfi : 1; struct list_head pv_target; struct reloc *relocs; };
Validate that all indirect calls adhere to kCFI rules. Notably doing nocfi indirect call to a cfi function is broken. Apparently some Rust 'core' code violates this and explodes when ran with FineIBT. All the ANNOTATE_NOCFI sites are prime targets for attackers. - runtime EFI is especially henous because it also needs to disable IBT. Basically calling unknown code without CFI protection at runtime is a massice security issue. - Kexec image handover; if you can exploit this, you get to keep it :-) - KVM, once for the interrupt injection calling IDT gates directly. - KVM, once for the FASTOP emulation stuff. Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org> --- arch/x86/kernel/machine_kexec_64.c | 1 arch/x86/kvm/emulate.c | 4 +++ arch/x86/kvm/vmx/vmenter.S | 4 +++ arch/x86/platform/efi/efi_stub_64.S | 4 +++ drivers/misc/lkdtm/perms.c | 2 + include/linux/objtool.h | 3 ++ include/linux/objtool_types.h | 1 tools/include/linux/objtool_types.h | 1 tools/objtool/check.c | 41 ++++++++++++++++++++++++++++++++++++ tools/objtool/include/objtool/elf.h | 1 10 files changed, 62 insertions(+)