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: 45eb29140e68ffe8e93a5471006858a018480a45Ok 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.