This is a note to let you know that I've just added the patch titled objtool: Add entry UNRET validation to the 5.18-stable tree which can be found at: http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary The filename of the patch is: objtool-add-entry-unret-validation.patch and it can be found in the queue-5.18 subdirectory. If you, or anyone else, feels it should not be added to the stable tree, please let <stable@xxxxxxxxxxxxxxx> know about it. >From foo@baz Tue Jul 12 05:03:58 PM CEST 2022 From: Thadeu Lima de Souza Cascardo <cascardo@xxxxxxxxxxxxx> Date: Mon, 11 Jul 2022 10:51:17 -0300 Subject: objtool: Add entry UNRET validation From: Thadeu Lima de Souza Cascardo <cascardo@xxxxxxxxxxxxx> commit a09a6e2399ba0595c3042b3164f3ca68a3cff33e upstream. Since entry asm is tricky, add a validation pass that ensures the retbleed mitigation has been done before the first actual RET instruction. Entry points are those that either have UNWIND_HINT_ENTRY, which acts as UNWIND_HINT_EMPTY but marks the instruction as an entry point, or those that have UWIND_HINT_IRET_REGS at +0. This is basically a variant of validate_branch() that is intra-function and it will simply follow all branches from marked entry points and ensures that all paths lead to ANNOTATE_UNRET_END. If a path hits RET or an indirection the path is a fail and will be reported. There are 3 ANNOTATE_UNRET_END instances: - UNTRAIN_RET itself - exception from-kernel; this path doesn't need UNTRAIN_RET - all early exceptions; these also don't need UNTRAIN_RET Signed-off-by: Peter Zijlstra (Intel) <peterz@xxxxxxxxxxxxx> Signed-off-by: Borislav Petkov <bp@xxxxxxx> Reviewed-by: Josh Poimboeuf <jpoimboe@xxxxxxxxxx> Signed-off-by: Borislav Petkov <bp@xxxxxxx> [cascardo: tools/objtool/builtin-check.c no link option validation] [cascardo: tools/objtool/check.c opts.ibt is ibt] [cascardo: tools/objtool/include/objtool/builtin.h leave unret option as bool, no struct opts] [cascardo: objtool is still called from scripts/link-vmlinux.sh] Signed-off-by: Thadeu Lima de Souza Cascardo <cascardo@xxxxxxxxxxxxx> Signed-off-by: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx> --- arch/x86/entry/entry_64.S | 3 arch/x86/entry/entry_64_compat.S | 6 - arch/x86/include/asm/nospec-branch.h | 12 ++ arch/x86/include/asm/unwind_hints.h | 4 arch/x86/kernel/head_64.S | 5 arch/x86/xen/xen-asm.S | 10 - include/linux/objtool.h | 3 scripts/link-vmlinux.sh | 3 tools/include/linux/objtool.h | 3 tools/objtool/builtin-check.c | 3 tools/objtool/check.c | 177 ++++++++++++++++++++++++++++++-- tools/objtool/include/objtool/builtin.h | 2 tools/objtool/include/objtool/check.h | 11 + 13 files changed, 220 insertions(+), 22 deletions(-) --- a/arch/x86/entry/entry_64.S +++ b/arch/x86/entry/entry_64.S @@ -85,7 +85,7 @@ */ SYM_CODE_START(entry_SYSCALL_64) - UNWIND_HINT_EMPTY + UNWIND_HINT_ENTRY ENDBR swapgs @@ -1088,6 +1088,7 @@ SYM_CODE_START_LOCAL(error_entry) .Lerror_entry_done_lfence: FENCE_SWAPGS_KERNEL_ENTRY leaq 8(%rsp), %rax /* return pt_regs pointer */ + ANNOTATE_UNRET_END RET .Lbstep_iret: --- a/arch/x86/entry/entry_64_compat.S +++ b/arch/x86/entry/entry_64_compat.S @@ -49,7 +49,7 @@ * 0(%ebp) arg6 */ SYM_CODE_START(entry_SYSENTER_compat) - UNWIND_HINT_EMPTY + UNWIND_HINT_ENTRY ENDBR /* Interrupts are off on entry. */ SWAPGS @@ -204,7 +204,7 @@ SYM_CODE_END(entry_SYSENTER_compat) * 0(%esp) arg6 */ SYM_CODE_START(entry_SYSCALL_compat) - UNWIND_HINT_EMPTY + UNWIND_HINT_ENTRY ENDBR /* Interrupts are off on entry. */ swapgs @@ -353,7 +353,7 @@ SYM_CODE_END(entry_SYSCALL_compat) * ebp arg6 */ SYM_CODE_START(entry_INT80_compat) - UNWIND_HINT_EMPTY + UNWIND_HINT_ENTRY ENDBR /* * Interrupts are off on entry. --- a/arch/x86/include/asm/nospec-branch.h +++ b/arch/x86/include/asm/nospec-branch.h @@ -82,6 +82,17 @@ #define ANNOTATE_UNRET_SAFE ANNOTATE_RETPOLINE_SAFE /* + * Abuse ANNOTATE_RETPOLINE_SAFE on a NOP to indicate UNRET_END, should + * eventually turn into it's own annotation. + */ +.macro ANNOTATE_UNRET_END +#ifdef CONFIG_DEBUG_ENTRY + ANNOTATE_RETPOLINE_SAFE + nop +#endif +.endm + +/* * JMP_NOSPEC and CALL_NOSPEC macros can be used instead of a simple * indirect jmp/call which may be susceptible to the Spectre variant 2 * attack. @@ -131,6 +142,7 @@ */ .macro UNTRAIN_RET #ifdef CONFIG_RETPOLINE + ANNOTATE_UNRET_END ALTERNATIVE_2 "", \ "call zen_untrain_ret", X86_FEATURE_UNRET, \ "call entry_ibpb", X86_FEATURE_ENTRY_IBPB --- a/arch/x86/include/asm/unwind_hints.h +++ b/arch/x86/include/asm/unwind_hints.h @@ -11,6 +11,10 @@ UNWIND_HINT sp_reg=ORC_REG_UNDEFINED type=UNWIND_HINT_TYPE_CALL end=1 .endm +.macro UNWIND_HINT_ENTRY + UNWIND_HINT sp_reg=ORC_REG_UNDEFINED type=UNWIND_HINT_TYPE_ENTRY end=1 +.endm + .macro UNWIND_HINT_REGS base=%rsp offset=0 indirect=0 extra=1 partial=0 .if \base == %rsp .if \indirect --- a/arch/x86/kernel/head_64.S +++ b/arch/x86/kernel/head_64.S @@ -334,6 +334,8 @@ SYM_CODE_START_NOALIGN(vc_boot_ghcb) UNWIND_HINT_IRET_REGS offset=8 ENDBR + ANNOTATE_UNRET_END + /* Build pt_regs */ PUSH_AND_CLEAR_REGS @@ -393,6 +395,7 @@ SYM_CODE_END(early_idt_handler_array) SYM_CODE_START_LOCAL(early_idt_handler_common) UNWIND_HINT_IRET_REGS offset=16 + ANNOTATE_UNRET_END /* * The stack is the hardware frame, an error code or zero, and the * vector number. @@ -442,6 +445,8 @@ SYM_CODE_START_NOALIGN(vc_no_ghcb) UNWIND_HINT_IRET_REGS offset=8 ENDBR + ANNOTATE_UNRET_END + /* Build pt_regs */ PUSH_AND_CLEAR_REGS --- a/arch/x86/xen/xen-asm.S +++ b/arch/x86/xen/xen-asm.S @@ -121,7 +121,7 @@ SYM_FUNC_END(xen_read_cr2_direct); .macro xen_pv_trap name SYM_CODE_START(xen_\name) - UNWIND_HINT_EMPTY + UNWIND_HINT_ENTRY ENDBR pop %rcx pop %r11 @@ -235,7 +235,7 @@ SYM_CODE_END(xenpv_restore_regs_and_retu /* Normal 64-bit system call target */ SYM_CODE_START(xen_entry_SYSCALL_64) - UNWIND_HINT_EMPTY + UNWIND_HINT_ENTRY ENDBR popq %rcx popq %r11 @@ -255,7 +255,7 @@ SYM_CODE_END(xen_entry_SYSCALL_64) /* 32-bit compat syscall target */ SYM_CODE_START(xen_entry_SYSCALL_compat) - UNWIND_HINT_EMPTY + UNWIND_HINT_ENTRY ENDBR popq %rcx popq %r11 @@ -273,7 +273,7 @@ SYM_CODE_END(xen_entry_SYSCALL_compat) /* 32-bit compat sysenter target */ SYM_CODE_START(xen_entry_SYSENTER_compat) - UNWIND_HINT_EMPTY + UNWIND_HINT_ENTRY ENDBR /* * NB: Xen is polite and clears TF from EFLAGS for us. This means @@ -297,7 +297,7 @@ SYM_CODE_END(xen_entry_SYSENTER_compat) SYM_CODE_START(xen_entry_SYSCALL_compat) SYM_CODE_START(xen_entry_SYSENTER_compat) - UNWIND_HINT_EMPTY + UNWIND_HINT_ENTRY ENDBR lea 16(%rsp), %rsp /* strip %rcx, %r11 */ mov $-ENOSYS, %rax --- a/include/linux/objtool.h +++ b/include/linux/objtool.h @@ -32,11 +32,14 @@ struct unwind_hint { * * UNWIND_HINT_FUNC: Generate the unwind metadata of a callable function. * Useful for code which doesn't have an ELF function annotation. + * + * UNWIND_HINT_ENTRY: machine entry without stack, SYSCALL/SYSENTER etc. */ #define UNWIND_HINT_TYPE_CALL 0 #define UNWIND_HINT_TYPE_REGS 1 #define UNWIND_HINT_TYPE_REGS_PARTIAL 2 #define UNWIND_HINT_TYPE_FUNC 3 +#define UNWIND_HINT_TYPE_ENTRY 4 #ifdef CONFIG_STACK_VALIDATION --- a/scripts/link-vmlinux.sh +++ b/scripts/link-vmlinux.sh @@ -130,6 +130,9 @@ objtool_link() if is_enabled CONFIG_VMLINUX_VALIDATION; then objtoolopt="${objtoolopt} --noinstr" + if is_enabled CONFIG_RETPOLINE; then + objtoolopt="${objtoolopt} --unret" + fi fi if [ -n "${objtoolopt}" ]; then --- a/tools/include/linux/objtool.h +++ b/tools/include/linux/objtool.h @@ -32,11 +32,14 @@ struct unwind_hint { * * UNWIND_HINT_FUNC: Generate the unwind metadata of a callable function. * Useful for code which doesn't have an ELF function annotation. + * + * UNWIND_HINT_ENTRY: machine entry without stack, SYSCALL/SYSENTER etc. */ #define UNWIND_HINT_TYPE_CALL 0 #define UNWIND_HINT_TYPE_REGS 1 #define UNWIND_HINT_TYPE_REGS_PARTIAL 2 #define UNWIND_HINT_TYPE_FUNC 3 +#define UNWIND_HINT_TYPE_ENTRY 4 #ifdef CONFIG_STACK_VALIDATION --- a/tools/objtool/builtin-check.c +++ b/tools/objtool/builtin-check.c @@ -21,7 +21,7 @@ bool no_fp, no_unreachable, retpoline, module, backtrace, uaccess, stats, lto, vmlinux, mcount, noinstr, backup, sls, dryrun, - ibt; + ibt, unret; static const char * const check_usage[] = { "objtool check [<options>] file.o", @@ -37,6 +37,7 @@ const struct option check_options[] = { OPT_BOOLEAN('f', "no-fp", &no_fp, "Skip frame pointer validation"), OPT_BOOLEAN('u', "no-unreachable", &no_unreachable, "Skip 'unreachable instruction' warnings"), OPT_BOOLEAN('r', "retpoline", &retpoline, "Validate retpoline assumptions"), + OPT_BOOLEAN(0, "unret", &unret, "validate entry unret placement"), OPT_BOOLEAN('m', "module", &module, "Indicates the object will be part of a kernel module"), OPT_BOOLEAN('b', "backtrace", &backtrace, "unwind on error"), OPT_BOOLEAN('a', "uaccess", &uaccess, "enable uaccess checking"), --- a/tools/objtool/check.c +++ b/tools/objtool/check.c @@ -2031,16 +2031,24 @@ static int read_unwind_hints(struct objt insn->hint = true; - if (ibt && hint->type == UNWIND_HINT_TYPE_REGS_PARTIAL) { + if (hint->type == UNWIND_HINT_TYPE_REGS_PARTIAL) { struct symbol *sym = find_symbol_by_offset(insn->sec, insn->offset); - if (sym && sym->bind == STB_GLOBAL && - insn->type != INSN_ENDBR && !insn->noendbr) { - WARN_FUNC("UNWIND_HINT_IRET_REGS without ENDBR", - insn->sec, insn->offset); + if (sym && sym->bind == STB_GLOBAL) { + if (ibt && insn->type != INSN_ENDBR && !insn->noendbr) { + WARN_FUNC("UNWIND_HINT_IRET_REGS without ENDBR", + insn->sec, insn->offset); + } + + insn->entry = 1; } } + if (hint->type == UNWIND_HINT_TYPE_ENTRY) { + hint->type = UNWIND_HINT_TYPE_CALL; + insn->entry = 1; + } + if (hint->type == UNWIND_HINT_TYPE_FUNC) { insn->cfi = &func_cfi; continue; @@ -2115,8 +2123,9 @@ static int read_retpoline_hints(struct o if (insn->type != INSN_JUMP_DYNAMIC && insn->type != INSN_CALL_DYNAMIC && - insn->type != INSN_RETURN) { - WARN_FUNC("retpoline_safe hint not an indirect jump/call/ret", + insn->type != INSN_RETURN && + insn->type != INSN_NOP) { + WARN_FUNC("retpoline_safe hint not an indirect jump/call/ret/nop", insn->sec, insn->offset); return -1; } @@ -3412,8 +3421,8 @@ static int validate_branch(struct objtoo return 1; } - visited = 1 << state.uaccess; - if (insn->visited) { + visited = VISITED_BRANCH << state.uaccess; + if (insn->visited & VISITED_BRANCH_MASK) { if (!insn->hint && !insn_cfi_match(insn, &state.cfi)) return 1; @@ -3642,6 +3651,145 @@ static int validate_unwind_hints(struct return warnings; } +/* + * Validate rethunk entry constraint: must untrain RET before the first RET. + * + * Follow every branch (intra-function) and ensure ANNOTATE_UNRET_END comes + * before an actual RET instruction. + */ +static int validate_entry(struct objtool_file *file, struct instruction *insn) +{ + struct instruction *next, *dest; + int ret, warnings = 0; + + for (;;) { + next = next_insn_to_validate(file, insn); + + if (insn->visited & VISITED_ENTRY) + return 0; + + insn->visited |= VISITED_ENTRY; + + if (!insn->ignore_alts && !list_empty(&insn->alts)) { + struct alternative *alt; + bool skip_orig = false; + + list_for_each_entry(alt, &insn->alts, list) { + if (alt->skip_orig) + skip_orig = true; + + ret = validate_entry(file, alt->insn); + if (ret) { + if (backtrace) + BT_FUNC("(alt)", insn); + return ret; + } + } + + if (skip_orig) + return 0; + } + + switch (insn->type) { + + case INSN_CALL_DYNAMIC: + case INSN_JUMP_DYNAMIC: + case INSN_JUMP_DYNAMIC_CONDITIONAL: + WARN_FUNC("early indirect call", insn->sec, insn->offset); + return 1; + + case INSN_JUMP_UNCONDITIONAL: + case INSN_JUMP_CONDITIONAL: + if (!is_sibling_call(insn)) { + if (!insn->jump_dest) { + WARN_FUNC("unresolved jump target after linking?!?", + insn->sec, insn->offset); + return -1; + } + ret = validate_entry(file, insn->jump_dest); + if (ret) { + if (backtrace) { + BT_FUNC("(branch%s)", insn, + insn->type == INSN_JUMP_CONDITIONAL ? "-cond" : ""); + } + return ret; + } + + if (insn->type == INSN_JUMP_UNCONDITIONAL) + return 0; + + break; + } + + /* fallthrough */ + case INSN_CALL: + dest = find_insn(file, insn->call_dest->sec, + insn->call_dest->offset); + if (!dest) { + WARN("Unresolved function after linking!?: %s", + insn->call_dest->name); + return -1; + } + + ret = validate_entry(file, dest); + if (ret) { + if (backtrace) + BT_FUNC("(call)", insn); + return ret; + } + /* + * If a call returns without error, it must have seen UNTRAIN_RET. + * Therefore any non-error return is a success. + */ + return 0; + + case INSN_RETURN: + WARN_FUNC("RET before UNTRAIN", insn->sec, insn->offset); + return 1; + + case INSN_NOP: + if (insn->retpoline_safe) + return 0; + break; + + default: + break; + } + + if (!next) { + WARN_FUNC("teh end!", insn->sec, insn->offset); + return -1; + } + insn = next; + } + + return warnings; +} + +/* + * Validate that all branches starting at 'insn->entry' encounter UNRET_END + * before RET. + */ +static int validate_unret(struct objtool_file *file) +{ + struct instruction *insn; + int ret, warnings = 0; + + for_each_insn(file, insn) { + if (!insn->entry) + continue; + + ret = validate_entry(file, insn); + if (ret < 0) { + WARN_FUNC("Failed UNRET validation", insn->sec, insn->offset); + return ret; + } + warnings += ret; + } + + return warnings; +} + static int validate_retpoline(struct objtool_file *file) { struct instruction *insn; @@ -4005,6 +4153,17 @@ int check(struct objtool_file *file) goto out; warnings += ret; + if (unret) { + /* + * Must be after validate_branch() and friends, it plays + * further games with insn->visited. + */ + ret = validate_unret(file); + if (ret < 0) + return ret; + warnings += ret; + } + if (ibt) { ret = validate_ibt(file); if (ret < 0) --- a/tools/objtool/include/objtool/builtin.h +++ b/tools/objtool/include/objtool/builtin.h @@ -10,7 +10,7 @@ extern const struct option check_options[]; extern bool no_fp, no_unreachable, retpoline, module, backtrace, uaccess, stats, lto, vmlinux, mcount, noinstr, backup, sls, dryrun, - ibt; + ibt, unret; extern int cmd_parse_options(int argc, const char **argv, const char * const usage[]); --- a/tools/objtool/include/objtool/check.h +++ b/tools/objtool/include/objtool/check.h @@ -51,8 +51,10 @@ struct instruction { ignore_alts : 1, hint : 1, retpoline_safe : 1, - noendbr : 1; - /* 2 bit hole */ + noendbr : 1, + entry : 1; + /* 1 bit hole */ + s8 instr; u8 visited; /* u8 hole */ @@ -69,6 +71,11 @@ struct instruction { struct cfi_state *cfi; }; +#define VISITED_BRANCH 0x01 +#define VISITED_BRANCH_UACCESS 0x02 +#define VISITED_BRANCH_MASK 0x03 +#define VISITED_ENTRY 0x04 + static inline bool is_static_jump(struct instruction *insn) { return insn->type == INSN_JUMP_CONDITIONAL || Patches currently in stable-queue which might be from cascardo@xxxxxxxxxxxxx are queue-5.18/x86-sev-avoid-using-__x86_return_thunk.patch queue-5.18/kvm-vmx-prevent-rsb-underflow-before-vmenter.patch queue-5.18/x86-ftrace-use-alternative-ret-encoding.patch queue-5.18/objtool-re-add-unwind_hint_-save_restore.patch queue-5.18/x86-bugs-add-retbleed-ibpb.patch queue-5.18/x86-kexec-disable-ret-on-kexec.patch queue-5.18/x86-bugs-enable-stibp-for-jmp2ret.patch queue-5.18/x86-retpoline-cleanup-some-ifdefery.patch queue-5.18/x86-speculation-disable-rrsba-behavior.patch queue-5.18/kvm-vmx-flatten-__vmx_vcpu_run.patch queue-5.18/x86-kvm-vmx-make-noinstr-clean.patch queue-5.18/x86-retbleed-add-fine-grained-kconfig-knobs.patch queue-5.18/x86-cpu-amd-add-spectral-chicken.patch queue-5.18/kvm-vmx-fix-ibrs-handling-after-vmexit.patch queue-5.18/kvm-vmx-prevent-guest-rsb-poisoning-attacks-with-eibrs.patch queue-5.18/x86-vsyscall_emu-64-don-t-use-ret-in-vsyscall-emulation.patch queue-5.18/objtool-skip-non-text-sections-when-adding-return-thunk-sites.patch queue-5.18/x86-bugs-do-ibpb-fallback-check-only-once.patch queue-5.18/x86-add-magic-amd-return-thunk.patch queue-5.18/x86-bugs-keep-a-per-cpu-ia32_spec_ctrl-value.patch queue-5.18/x86-objtool-create-.return_sites.patch queue-5.18/x86-kvm-fix-setcc-emulation-for-return-thunks.patch queue-5.18/x86-cpu-amd-enumerate-btc_no.patch queue-5.18/x86-entry-move-push_and_clear_regs-out-of-error_entry.patch queue-5.18/x86-retpoline-swizzle-retpoline-thunk.patch queue-5.18/x86-entry-switch-the-stack-after-error_entry-returns.patch queue-5.18/x86-speculation-fix-firmware-entry-spec_ctrl-handling.patch queue-5.18/x86-speculation-add-spectre_v2-ibrs-option-to-support-kernel-ibrs.patch queue-5.18/x86-xen-add-untrain_ret.patch queue-5.18/x86-undo-return-thunk-damage.patch queue-5.18/x86-speculation-remove-x86_spec_ctrl_mask.patch queue-5.18/x86-entry-avoid-very-early-ret.patch queue-5.18/x86-bugs-add-cannon-lake-to-retbleed-affected-cpu-list.patch queue-5.18/x86-entry-move-push_and_clear_regs-back-into-error_entry.patch queue-5.18/x86-speculation-fill-rsb-on-vmexit-for-ibrs.patch queue-5.18/objtool-add-entry-unret-validation.patch queue-5.18/kvm-vmx-convert-launched-argument-to-flags.patch queue-5.18/x86-bpf-use-alternative-ret-encoding.patch queue-5.18/x86-common-stamp-out-the-stepping-madness.patch queue-5.18/x86-bugs-split-spectre_v2_select_mitigation-and-spectre_v2_user_select_mitigation.patch queue-5.18/x86-entry-don-t-call-error_entry-for-xenpv.patch queue-5.18/x86-bugs-report-intel-retbleed-vulnerability.patch queue-5.18/x86-cpufeatures-move-retpoline-flags-to-word-11.patch queue-5.18/x86-speculation-fix-spec_ctrl-write-on-smt-state-change.patch queue-5.18/x86-retpoline-use-mfunction-return.patch queue-5.18/x86-xen-rename-sys-entry-points.patch queue-5.18/x86-bugs-optimize-spec_ctrl-msr-writes.patch queue-5.18/x86-traps-use-pt_regs-directly-in-fixup_bad_iret.patch queue-5.18/x86-bugs-do-not-enable-ibpb-on-entry-when-ibpb-is-not-supported.patch queue-5.18/x86-bugs-report-amd-retbleed-vulnerability.patch queue-5.18/x86-static_call-use-alternative-ret-encoding.patch queue-5.18/x86-speculation-fix-rsb-filling-with-config_retpoline-n.patch queue-5.18/x86-use-return-thunk-in-asm-code.patch queue-5.18/intel_idle-disable-ibrs-during-long-idle.patch queue-5.18/x86-entry-remove-skip_r11rcx.patch queue-5.18/x86-speculation-use-cached-host-spec_ctrl-value-for-guest-entry-exit.patch queue-5.18/x86-bugs-add-amd-retbleed-boot-parameter.patch queue-5.18/x86-entry-add-kernel-ibrs-implementation.patch queue-5.18/objtool-treat-.text.__x86.-as-noinstr.patch queue-5.18/objtool-update-retpoline-validation.patch