On Thu, Jun 05, 2025 at 10:19:41AM -0700, Josh Poimboeuf wrote:
> diff --git a/arch/x86/entry/calling.h b/arch/x86/entry/calling.h
> index d83236b96f22..e680afbf65b6 100644
> --- a/arch/x86/entry/calling.h
> +++ b/arch/x86/entry/calling.h
> @@ -99,7 +99,7 @@ For 32-bit we have the following conventions - kernel is built with
> .endif
> .endm
>
> -.macro CLEAR_REGS clear_bp=1
> +.macro CLEAR_REGS clear_callee=1
> /*
> * Sanitize registers of values that a speculation attack might
> * otherwise want to exploit. The lower registers are likely clobbered
> @@ -113,20 +113,19 @@ For 32-bit we have the following conventions - kernel is built with
> xorl %r9d, %r9d /* nospec r9 */
> xorl %r10d, %r10d /* nospec r10 */
> xorl %r11d, %r11d /* nospec r11 */
> + .if \clear_callee
> xorl %ebx, %ebx /* nospec rbx */
> - .if \clear_bp
> xorl %ebp, %ebp /* nospec rbp */
> - .endif
> xorl %r12d, %r12d /* nospec r12 */
> xorl %r13d, %r13d /* nospec r13 */
> xorl %r14d, %r14d /* nospec r14 */
> xorl %r15d, %r15d /* nospec r15 */
> -
> + .endif
> .endm
>
> -.macro PUSH_AND_CLEAR_REGS rdx=%rdx rcx=%rcx rax=%rax save_ret=0 clear_bp=1 unwind_hint=1
> +.macro PUSH_AND_CLEAR_REGS rdx=%rdx rcx=%rcx rax=%rax save_ret=0 clear_callee=1 unwind_hint=1
> PUSH_REGS rdx=\rdx, rcx=\rcx, rax=\rax, save_ret=\save_ret unwind_hint=\unwind_hint
> - CLEAR_REGS clear_bp=\clear_bp
> + CLEAR_REGS clear_callee=\clear_callee
> .endm
>
> .macro POP_REGS pop_rdi=1
Nice. So that leaves the callee-clobbered, extra caller-saved and return
registers cleared, and preserves the callee-saved regs.
> diff --git a/arch/x86/entry/entry_64_fred.S b/arch/x86/entry/entry_64_fred.S
> index 29c5c32c16c3..5d1eef193b79 100644
> --- a/arch/x86/entry/entry_64_fred.S
> +++ b/arch/x86/entry/entry_64_fred.S
> @@ -112,11 +112,12 @@ SYM_FUNC_START(asm_fred_entry_from_kvm)
> push %rax /* Return RIP */
> push $0 /* Error code, 0 for IRQ/NMI */
>
> - PUSH_AND_CLEAR_REGS clear_bp=0 unwind_hint=0
> + PUSH_AND_CLEAR_REGS clear_callee=0 unwind_hint=0
> movq %rsp, %rdi /* %rdi -> pt_regs */
> call __fred_entry_from_kvm /* Call the C entry point */
> - POP_REGS
> - ERETS
> + addq $C_PTREGS_SIZE, %rsp
> +
> + ALTERNATIVE "mov %rbp, %rsp", __stringify(ERETS), X86_FEATURE_FRED
So... I was wondering.. do we actually ever need the ERETS? AFAICT this
will only ever 'inject' external interrupts, and those are not supposed
to change the exception frame, like ever. Only exceptions get to change
the exception frame, but those are explicitly excluded in fred_extint().
As such, it should always be correct to just do:
leave;
RET;
at this point, and call it a day, no? Just completely forget about all
this sillyness with alternatives and funky stack state.
Only problem seems to be that if we do this, then
has_modified_stack_frame() has a fit, because of the register state.
The first to complain is bx, the push %rbx modifies the CFI state to
track where on the stack its saved, and that's not what initial_func_cfi
has.
We can stomp on that with UNWIND_HINT_FUNC right before RET. It's all a
bit magical, but should work, right?
So keeping your CLEAR_REGS changes, I've ended up with the below:
---
diff --git a/arch/x86/entry/entry_64_fred.S b/arch/x86/entry/entry_64_fred.S
index 29c5c32c16c3..8c03d04ea69d 100644
--- a/arch/x86/entry/entry_64_fred.S
+++ b/arch/x86/entry/entry_64_fred.S
@@ -62,8 +62,6 @@ SYM_FUNC_START(asm_fred_entry_from_kvm)
push %rbp
mov %rsp, %rbp
- UNWIND_HINT_SAVE
-
/*
* Both IRQ and NMI from VMX can be handled on current task stack
* because there is no need to protect from reentrancy and the call
@@ -112,19 +110,35 @@ SYM_FUNC_START(asm_fred_entry_from_kvm)
push %rax /* Return RIP */
push $0 /* Error code, 0 for IRQ/NMI */
- PUSH_AND_CLEAR_REGS clear_bp=0 unwind_hint=0
+ PUSH_AND_CLEAR_REGS clear_callee=0 unwind_hint=0
movq %rsp, %rdi /* %rdi -> pt_regs */
+
+ /*
+ * At this point: {rdi, rsi, rdx, rcx, r8, r9}, {r10, r11}, {rax, rdx}
+ * are clobbered, which corresponds to: arguments, extra caller-saved
+ * and return. All registers a C function is allowed to clobber.
+ *
+ * Notably, the callee-saved registers: {rbx, r12, r13, r14, r15}
+ * are untouched, with the exception of rbp, which carries the stack
+ * frame and will be restored before exit.
+ *
+ * Further calling another C function will not alter this state.
+ */
call __fred_entry_from_kvm /* Call the C entry point */
- POP_REGS
- ERETS
-1:
+
+1: /*
+ * Therefore, all that remains to be done at this point is restore the
+ * stack and frame pointer register.
+ */
+ leave
/*
- * Objtool doesn't understand what ERETS does, this hint tells it that
- * yes, we'll reach here and with what stack state. A save/restore pair
- * isn't strictly needed, but it's the simplest form.
+ * Objtool gets confused by the cfi register state; this doesn't match
+ * initial_func_cfi because of PUSH_REGS, where it tracks where those
+ * registers are on the stack.
+ *
+ * Forcefully make it forget this before returning.
*/
- UNWIND_HINT_RESTORE
- pop %rbp
+ UNWIND_HINT_FUNC
RET
SYM_FUNC_END(asm_fred_entry_from_kvm)
