Thread (52 messages) 52 messages, 7 authors, 2025-06-06

Re: [PATCH v2 00/13] objtool: Detect and warn about indirect calls in __nocfi functions

From: Sean Christopherson <seanjc@google.com>
Date: 2025-05-01 19:00:01
Also in: kvm, linux-efi, lkml

On Thu, May 01, 2025, H. Peter Anvin wrote:
On May 1, 2025 11:30:18 AM PDT, Sean Christopherson [off-list ref] wrote:
quoted
On Thu, May 01, 2025, Peter Zijlstra wrote:
quoted
On Thu, May 01, 2025 at 12:30:38PM +0200, Peter Zijlstra wrote:
quoted
On Wed, Apr 30, 2025 at 09:06:00PM +0200, Peter Zijlstra wrote:
quoted
On Wed, Apr 30, 2025 at 07:24:15AM -0700, H. Peter Anvin wrote:
quoted
quoted
KVM has another; the VMX interrupt injection stuff calls the IDT handler
directly.  Is there an alternative? Can we keep a table of Linux functions
slighly higher up the call stack (asm_\cfunc ?) and add CFI to those?
quoted
We do have a table of handlers higher up in the stack in the form of
the dispatch tables for FRED. They don't in general even need the
assembly entry stubs, either.
Oh, right. I'll go have a look at those.
Right, so perhaps the easiest way around this is to setup the FRED entry
tables unconditionally, have VMX mandate CONFIG_FRED and then have it
always use the FRED entry points.

Let me see how ugly that gets.
Something like so... except this is broken. Its reporting spurious
interrupts on vector 0x00, so something is buggered passing that vector
along.
Uh, aren't you making this way more complex than it needs to be?  IIUC, KVM never
uses the FRED hardware entry points, i.e. the FRED entry tables don't need to be
in place because they'll never be used.  The only bits of code KVM needs is the
__fred_entry_from_kvm() glue.

Lightly tested, but this combo works for IRQs and NMIs on non-FRED hardware.
Hrm, and now I see that fred_extint() relies on fred_install_sysvec(), which makes
me quite curious as to why IRQs didn't go sideways.  Oh, because sysvec_table[]
is statically defined at compile time except for PV crud.

So yeah, I think my the patches are correct, they just the need a small bit of
prep work to support dynamic setup of sysvec_table.
quoted
--
From 664468143109ab7c525c0babeba62195fa4c657e Mon Sep 17 00:00:00 2001
From: Sean Christopherson <seanjc@google.com>
Date: Thu, 1 May 2025 11:20:29 -0700
Subject: [PATCH 1/2] x86/fred: Play nice with invoking
asm_fred_entry_from_kvm() on non-FRED hardware

Modify asm_fred_entry_from_kvm() to allow it to be invoked by KVM even
when FRED isn't fully enabled, e.g. when running with CONFIG_X86_FRED=y
on non-FRED hardware.  This will allow forcing KVM to always use the FRED
entry points for 64-bit kernels, which in turn will eliminate a rather
gross non-CFI indirect call that KVM uses to trampoline IRQs by doing IDT
lookups.

When FRED isn't enabled, simply skip ERETS and restore RBP and RSP from
the stack frame prior to doing a "regular" RET back to KVM (in quotes
because of all the RET mitigation horrors).

Signed-off-by: Sean Christopherson <seanjc@google.com>
---
arch/x86/entry/entry_64_fred.S | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/arch/x86/entry/entry_64_fred.S b/arch/x86/entry/entry_64_fred.S
index 29c5c32c16c3..7aff2f0a285f 100644
--- a/arch/x86/entry/entry_64_fred.S
+++ b/arch/x86/entry/entry_64_fred.S
@@ -116,7 +116,8 @@ SYM_FUNC_START(asm_fred_entry_from_kvm)
	movq %rsp, %rdi				/* %rdi -> pt_regs */
	call __fred_entry_from_kvm		/* Call the C entry point */
	POP_REGS
-	ERETS
+
+	ALTERNATIVE "", __stringify(ERETS), X86_FEATURE_FRED
1:
	/*
	 * Objtool doesn't understand what ERETS does, this hint tells it that
@@ -124,7 +125,7 @@ SYM_FUNC_START(asm_fred_entry_from_kvm)
	 * isn't strictly needed, but it's the simplest form.
	 */
	UNWIND_HINT_RESTORE
-	pop %rbp
+	leave
	RET

SYM_FUNC_END(asm_fred_entry_from_kvm)

base-commit: 45eb29140e68ffe8e93a5471006858a018480a45
Ok maybe I'm being dense, but what is left other than simply calling
__fred_entry_from_kvm() as a normal C function? 

I'm on the go so there might be something in the code I'm missing, but on the
surface...?
I'm sure it's doable, though I'd be more than a little nervous about diverging
from what FRED=y does, e.g. in case code somewhere expects the stack to look
exactly like a real FRED event.

And since we'd still need the assembly to support FRED=y, I don't see any point
in adding more code when it's trivially easy to have asm_fred_entry_from_kvm()
skip ERETS.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help