On Mon, Jun 02, 2025 at 10:43:42PM -0700, Josh Poimboeuf wrote:
> On Thu, May 29, 2025 at 11:30:17AM +0200, Peter Zijlstra wrote:
> > > > So the sequence of fail is:
> > > >
> > > > push %rbp
> > > > mov %rsp, %rbp # cfa.base = BP
> > > >
> > > > SAVE
> >
> > sub $0x40,%rsp
> > and $0xffffffffffffffc0,%rsp
> >
> > This hits the 'older GCC, drap with frame pointer' case in OP_SRC_AND.
> > Which means we then hard rely on the frame pointer to get things right.
> >
> > However, per all the PUSH/POP_REGS nonsense, BP can get clobbered.
> > Specifically the code between the CALL and POP %rbp below are up in the
> > air. I don't think it can currently unwind properly there.
>
> RBP is callee saved, so there's no need to pop it or any of the other
> callee-saved regs. If they were to change, that would break C ABI
> pretty badly. Maybe add a skip_callee=1 arg to POP_REGS?
This compiles for me:
diff --git a/arch/x86/entry/calling.h b/arch/x86/entry/calling.h
index d83236b96f22..414f8bcf07ec 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,29 +113,31 @@ 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
+.macro POP_REGS pop_rdi=1 pop_callee=1
+.if \pop_callee
popq %r15
popq %r14
popq %r13
popq %r12
popq %rbp
popq %rbx
+.endif
popq %r11
popq %r10
popq %r9
diff --git a/arch/x86/entry/entry_64_fred.S b/arch/x86/entry/entry_64_fred.S
index 29c5c32c16c3..277f980c46fd 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
+ POP_REGS pop_callee=0
+
+ ALTERNATIVE "mov %rbp, %rsp", __stringify(ERETS), X86_FEATURE_FRED
1:
/*
* Objtool doesn't understand what ERETS does, this hint tells it that
