linux-kernel - Re: [PATCH v2 1/2] KVM: SVM: Set RFLAGS.IF=1 in C code, to get VMRUN out of the STI shadow

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: <CALMp9eRfU7b_4080ku-z2w+pQT0dZyenBb=9rM6m2kH-9-5WLA@mail.gmail.com>
Date: Mon, 24 Feb 2025 09:24:25 -0800
From: Jim Mattson <jmattson@...gle.com>
To: Sean Christopherson <seanjc@...gle.com>
Cc: Paolo Bonzini <pbonzini@...hat.com>, kvm@...r.kernel.org, linux-kernel@...r.kernel.org, 
	Doug Covelli <doug.covelli@...adcom.com>
Subject: Re: [PATCH v2 1/2] KVM: SVM: Set RFLAGS.IF=1 in C code, to get VMRUN
 out of the STI shadow

On Mon, Feb 24, 2025 at 8:55 AM Sean Christopherson <seanjc@...gle.com> wrote:
>
> Enable/disable local IRQs, i.e. set/clear RFLAGS.IF, in the common
> svm_vcpu_enter_exit() just after/before guest_state_{enter,exit}_irqoff()
> so that VMRUN is not executed in an STI shadow.  AMD CPUs have a quirk
> (some would say "bug"), where the STI shadow bleeds into the guest's
> intr_state field if a #VMEXIT occurs during injection of an event, i.e. if
> the VMRUN doesn't complete before the subsequent #VMEXIT.
>
> The spurious "interrupts masked" state is relatively benign, as it only
> occurs during event injection and is transient.  Because KVM is already
> injecting an event, the guest can't be in HLT, and if KVM is querying IRQ
> blocking for injection, then KVM would need to force an immediate exit
> anyways since injecting multiple events is impossible.
>
> However, because KVM copies int_state verbatim from vmcb02 to vmcb12, the
> spurious STI shadow is visible to L1 when running a nested VM, which can
> trip sanity checks, e.g. in VMware's VMM.
>
> Hoist the STI+CLI all the way to C code, as the aforementioned calls to
> guest_state_{enter,exit}_irqoff() already inform lockdep that IRQs are
> enabled/disabled, and taking a fault on VMRUN with RFLAGS.IF=1 is already
> possible.  I.e. if there's kernel code that is confused by running with
> RFLAGS.IF=1, then it's already a problem.  In practice, since GIF=0 also
> blocks NMIs, the only change in exposure to non-KVM code (relative to
> surrounding VMRUN with STI+CLI) is exception handling code, and except for
> the kvm_rebooting=1 case, all exception in the core VM-Enter/VM-Exit path
> are fatal.
>
> Use the "raw" variants to enable/disable IRQs to avoid tracing in the
> "no instrumentation" code; the guest state helpers also take care of
> tracing IRQ state.
>
> Oppurtunstically document why KVM needs to do STI in the first place.
>
> Reported-by: Doug Covelli <doug.covelli@...adcom.com>
> Closes: https://lore.kernel.org/all/CADH9ctBs1YPmE4aCfGPNBwA10cA8RuAk2gO7542DjMZgs4uzJQ@mail.gmail.com
> Fixes: f14eec0a3203 ("KVM: SVM: move more vmentry code to assembly")
> Cc: stable@...r.kernel.org
> Signed-off-by: Sean Christopherson <seanjc@...gle.com>

Reviewed-by: Jim Mattson <jmattson@...gle.com>