On Tue, Oct 24, 2023 at 10:19:13PM +0200, Borislav Petkov wrote: > On Thu, Oct 19, 2023 at 08:20:51AM -0700, Josh Poimboeuf wrote: > > GCC doesn't read asm. Even if it did that wouldn't fix things for > > callers of custom-ABI return-thunk-using functions. > > > > The below seems to work. > > Right, I guess we can do something like that. Linker is not happy here > about that symbol, tho: > > ld: arch/x86/lib/retpoline.o:(.altinstr_replacement+0x95): undefined reference to `warn_thunk_thunk' > make[2]: *** [scripts/Makefile.vmlinux:37: vmlinux] Error 1 > make[1]: *** [/mnt/kernel/kernel/5th/linux/Makefile:1165: vmlinux] Error 2 > make: *** [Makefile:234: __sub-make] Error 2 Ok, back to playing with this. A fix below along with a beefed up warning message. If only I can remember now how we did trigger the warning in the first place in order to test it... --- diff --git a/arch/x86/entry/calling.h b/arch/x86/entry/calling.h index e59d3073e7cf..a4679e8f30ad 100644 --- a/arch/x86/entry/calling.h +++ b/arch/x86/entry/calling.h @@ -426,3 +426,36 @@ For 32-bit we have the following conventions - kernel is built with .endm #endif /* CONFIG_SMP */ + +/* rdi: arg1 ... normal C conventions. rax is saved/restored. */ +.macro THUNK name, func +SYM_FUNC_START(\name) + pushq %rbp + movq %rsp, %rbp + + pushq %rdi + pushq %rsi + pushq %rdx + pushq %rcx + pushq %rax + pushq %r8 + pushq %r9 + pushq %r10 + pushq %r11 + + call \func + + popq %r11 + popq %r10 + popq %r9 + popq %r8 + popq %rax + popq %rcx + popq %rdx + popq %rsi + popq %rdi + popq %rbp + RET +SYM_FUNC_END(\name) + _ASM_NOKPROBE(\name) +.endm diff --git a/arch/x86/entry/entry.S b/arch/x86/entry/entry.S index 8c8d38f0cb1d..582731f74dc8 100644 --- a/arch/x86/entry/entry.S +++ b/arch/x86/entry/entry.S @@ -7,6 +7,8 @@ #include <linux/linkage.h> #include <asm/msr-index.h> +#include "calling.h" + .pushsection .noinstr.text, "ax" SYM_FUNC_START(entry_ibpb) @@ -20,3 +22,5 @@ SYM_FUNC_END(entry_ibpb) EXPORT_SYMBOL_GPL(entry_ibpb); .popsection + +THUNK warn_thunk_thunk, __warn_thunk diff --git a/arch/x86/entry/thunk_64.S b/arch/x86/entry/thunk_64.S index 416b400f39db..119ebdc3d362 100644 --- a/arch/x86/entry/thunk_64.S +++ b/arch/x86/entry/thunk_64.S @@ -9,39 +9,6 @@ #include "calling.h" #include <asm/asm.h> - /* rdi: arg1 ... normal C conventions. rax is saved/restored. */ - .macro THUNK name, func -SYM_FUNC_START(\name) - pushq %rbp - movq %rsp, %rbp - - pushq %rdi - pushq %rsi - pushq %rdx - pushq %rcx - pushq %rax - pushq %r8 - pushq %r9 - pushq %r10 - pushq %r11 - - call \func - - popq %r11 - popq %r10 - popq %r9 - popq %r8 - popq %rax - popq %rcx - popq %rdx - popq %rsi - popq %rdi - popq %rbp - RET -SYM_FUNC_END(\name) - _ASM_NOKPROBE(\name) - .endm - THUNK preempt_schedule_thunk, preempt_schedule THUNK preempt_schedule_notrace_thunk, preempt_schedule_notrace EXPORT_SYMBOL(preempt_schedule_thunk) diff --git a/arch/x86/include/asm/nospec-branch.h b/arch/x86/include/asm/nospec-branch.h index 691ff1ef701b..64b175f03cdb 100644 --- a/arch/x86/include/asm/nospec-branch.h +++ b/arch/x86/include/asm/nospec-branch.h @@ -348,6 +348,8 @@ extern void entry_ibpb(void); extern void (*x86_return_thunk)(void); +extern void __warn_thunk(void); + #ifdef CONFIG_CALL_DEPTH_TRACKING extern void call_depth_return_thunk(void); diff --git a/arch/x86/kernel/cpu/bugs.c b/arch/x86/kernel/cpu/bugs.c index bb0ab8466b91..00a6d024ddf7 100644 --- a/arch/x86/kernel/cpu/bugs.c +++ b/arch/x86/kernel/cpu/bugs.c @@ -2849,3 +2849,19 @@ ssize_t cpu_show_gds(struct device *dev, struct device_attribute *attr, char *bu return cpu_show_common(dev, attr, buf, X86_BUG_GDS); } #endif + +void __warn_thunk(void) +{ + pr_warn_once("\n"); + pr_warn_once("**********************************************************\n"); + pr_warn_once("** NOTICE NOTICE NOTICE NOTICE NOTICE NOTICE NOTICE **\n"); + pr_warn_once("** **\n"); + pr_warn_once("** unpatched return thunk in use. This should not **\n"); + pr_warn_once("** on a production kernel. Please report this to **\n"); + pr_warn_once("** x86@xxxxxxxxxx. **\n"); + pr_warn_once("** **\n"); + pr_warn_once("** NOTICE NOTICE NOTICE NOTICE NOTICE NOTICE NOTICE **\n"); + pr_warn_once("**********************************************************\n"); + + dump_stack(); +} diff --git a/arch/x86/lib/retpoline.S b/arch/x86/lib/retpoline.S index 7b2589877d06..dfcb7d64d05a 100644 --- a/arch/x86/lib/retpoline.S +++ b/arch/x86/lib/retpoline.S @@ -369,19 +369,14 @@ SYM_FUNC_END(call_depth_return_thunk) * 'JMP __x86_return_thunk' sites are changed to something else by * apply_returns(). * - * This should be converted eventually to call a warning function which - * should scream loudly when the default return thunk is called after - * alternatives have been applied. - * - * That warning function cannot BUG() because the bug splat cannot be - * displayed in all possible configurations, leading to users not really - * knowing why the machine froze. + * The RET is replaced with a WARN_ONCE() to ensure it is never used at + * runtime. Alternative instructions are applied after apply_returns(). */ SYM_CODE_START(__x86_return_thunk) UNWIND_HINT_FUNC ANNOTATE_NOENDBR - ANNOTATE_UNRET_SAFE - ret + ALTERNATIVE __stringify(ANNOTATE_UNRET_SAFE; ret), \ + "jmp warn_thunk_thunk", X86_FEATURE_ALWAYS int3 SYM_CODE_END(__x86_return_thunk) EXPORT_SYMBOL(__x86_return_thunk) -- Regards/Gruss, Boris. https://people.kernel.org/tglx/notes-about-netiquette