linux-kernel - Re: [PATCH v2 00/13] objtool: Detect and warn about indirect calls in _

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <eegs5wq4eoqpu5yqlzug7icptiwzusracrp3nlmjkxwfywzvez@jngbkb3xqj6o>
Date: Tue, 3 Jun 2025 09:29:43 -0700
From: Josh Poimboeuf <jpoimboe@...nel.org>
To: Peter Zijlstra <peterz@...radead.org>
Cc: Sean Christopherson <seanjc@...gle.com>, 
	"H. Peter Anvin" <hpa@...or.com>, x86@...nel.org, kys@...rosoft.com, haiyangz@...rosoft.com, 
	wei.liu@...nel.org, decui@...rosoft.com, tglx@...utronix.de, mingo@...hat.com, 
	bp@...en8.de, dave.hansen@...ux.intel.com, pbonzini@...hat.com, 
	ardb@...nel.org, kees@...nel.org, Arnd Bergmann <arnd@...db.de>, 
	gregkh@...uxfoundation.org, linux-hyperv@...r.kernel.org, linux-kernel@...r.kernel.org, 
	kvm@...r.kernel.org, linux-efi@...r.kernel.org, samitolvanen@...gle.com, 
	ojeda@...nel.org, xin@...or.com
Subject: Re: [PATCH v2 00/13] objtool: Detect and warn about indirect calls
 in __nocfi functions

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