linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Lai Jiangshan <jiangshanlai+lkml@gmail.com>
To: Sean Christopherson <seanjc@google.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>,
	kvm@vger.kernel.org, LKML <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v3 01/37] KVM: VMX: Flush all EPTP/VPID contexts on remote TLB flush
Date: Tue, 3 Aug 2021 09:45:34 +0800	[thread overview]
Message-ID: <CAJhGHyCPyu6BVZwqvySeT2LSr81Xospdv2O=ssvTQv0Rvky0UA@mail.gmail.com> (raw)
In-Reply-To: <20200320212833.3507-2-sean.j.christopherson@intel.com>

(I'm replying to a very old email, so many CCs are dropped.)

On Sat, Mar 21, 2020 at 5:33 AM Sean Christopherson
<sean.j.christopherson@intel.com> wrote:
>
> Flush all EPTP/VPID contexts if a TLB flush _may_ have been triggered by
> a remote or deferred TLB flush, i.e. by KVM_REQ_TLB_FLUSH.  Remote TLB
> flushes require all contexts to be invalidated, not just the active
> contexts, e.g. all mappings in all contexts for a given HVA need to be
> invalidated on a mmu_notifier invalidation.  Similarly, the instigator
> of the deferred TLB flush may be expecting all contexts to be flushed,
> e.g. vmx_vcpu_load_vmcs().
>
> Without nested VMX, flushing only the current EPTP/VPID context isn't
> problematic because KVM uses a constant VPID for each vCPU, and

Hello, Sean

Is the patch optimized for cases where nested VMX is active?
I think the non-nested cases are normal cases.

Although the related code has been changed, the logic of the patch
is still working now, would it be better if we restore the optimization
for the normal cases (non-nested)?

Thanks
Lai

> mmu_alloc_direct_roots() all but guarantees KVM will use a single EPTP
> for L1.  In the rare case where a different EPTP is created or reused,
> KVM (currently) unconditionally flushes the new EPTP context prior to
> entering the guest.
>
> With nested VMX, KVM conditionally uses a different VPID for L2, and
> unconditionally uses a different EPTP for L2.  Because KVM doesn't
> _intentionally_ guarantee L2's EPTP/VPID context is flushed on nested
> VM-Enter, it'd be possible for a malicious L1 to attack the host and/or
> different VMs by exploiting the lack of flushing for L2.
>
>   1) Launch nested guest from malicious L1.
>
>   2) Nested VM-Enter to L2.
>
>   3) Access target GPA 'g'.  CPU inserts TLB entry tagged with L2's ASID
>      mapping 'g' to host PFN 'x'.
>
>   2) Nested VM-Exit to L1.
>
>   3) L1 triggers kernel same-page merging (ksm) by duplicating/zeroing
>      the page for PFN 'x'.
>
>   4) Host kernel merges PFN 'x' with PFN 'y', i.e. unmaps PFN 'x' and
>      remaps the page to PFN 'y'.  mmu_notifier sends invalidate command,
>      KVM flushes TLB only for L1's ASID.
>
>   4) Host kernel reallocates PFN 'x' to some other task/guest.
>
>   5) Nested VM-Enter to L2.  KVM does not invalidate L2's EPTP or VPID.
>
>   6) L2 accesses GPA 'g' and gains read/write access to PFN 'x' via its
>      stale TLB entry.
>
> However, current KVM unconditionally flushes L1's EPTP/VPID context on
> nested VM-Exit.  But, that behavior is mostly unintentional, KVM doesn't
> go out of its way to flush EPTP/VPID on nested VM-Enter/VM-Exit, rather
> a TLB flush is guaranteed to occur prior to re-entering L1 due to
> __kvm_mmu_new_cr3() always being called with skip_tlb_flush=false.  On
> nested VM-Enter, this happens via kvm_init_shadow_ept_mmu() (nested EPT
> enabled) or in nested_vmx_load_cr3() (nested EPT disabled).  On nested
> VM-Exit it occurs via nested_vmx_load_cr3().
>
> This also fixes a bug where a deferred TLB flush in the context of L2,
> with EPT disabled, would flush L1's VPID instead of L2's VPID, as
> vmx_flush_tlb() flushes L1's VPID regardless of is_guest_mode().
>
> Cc: Vitaly Kuznetsov <vkuznets@redhat.com>
> Cc: Ben Gardon <bgardon@google.com>
> Cc: Jim Mattson <jmattson@google.com>
> Cc: Junaid Shahid <junaids@google.com>
> Cc: Liran Alon <liran.alon@oracle.com>
> Cc: Boris Ostrovsky <boris.ostrovsky@oracle.com>
> Cc: John Haxby <john.haxby@oracle.com>
> Reviewed-by: Liran Alon <liran.alon@oracle.com>
> Fixes: efebf0aaec3d ("KVM: nVMX: Do not flush TLB on L1<->L2 transitions if L1 uses VPID and EPT")
> Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com>
> ---
>  arch/x86/kvm/vmx/vmx.h | 28 +++++++++++++++++++++++++++-
>  1 file changed, 27 insertions(+), 1 deletion(-)
>
> diff --git a/arch/x86/kvm/vmx/vmx.h b/arch/x86/kvm/vmx/vmx.h
> index be93d597306c..d6d67b816ebe 100644
> --- a/arch/x86/kvm/vmx/vmx.h
> +++ b/arch/x86/kvm/vmx/vmx.h
> @@ -518,7 +518,33 @@ static inline void __vmx_flush_tlb(struct kvm_vcpu *vcpu, int vpid,
>
>  static inline void vmx_flush_tlb(struct kvm_vcpu *vcpu, bool invalidate_gpa)
>  {
> -       __vmx_flush_tlb(vcpu, to_vmx(vcpu)->vpid, invalidate_gpa);
> +       struct vcpu_vmx *vmx = to_vmx(vcpu);
> +
> +       /*
> +        * Flush all EPTP/VPID contexts if the TLB flush _may_ have been
> +        * invoked via kvm_flush_remote_tlbs(), which always passes %true for
> +        * @invalidate_gpa.  Flushing remote TLBs requires all contexts to be
> +        * flushed, not just the active context.
> +        *
> +        * Note, this also ensures a deferred TLB flush with VPID enabled and
> +        * EPT disabled invalidates the "correct" VPID, by nuking both L1 and
> +        * L2's VPIDs.
> +        */
> +       if (invalidate_gpa) {
> +               if (enable_ept) {
> +                       ept_sync_global();
> +               } else if (enable_vpid) {
> +                       if (cpu_has_vmx_invvpid_global()) {
> +                               vpid_sync_vcpu_global();
> +                       } else {
> +                               WARN_ON_ONCE(!cpu_has_vmx_invvpid_single());
> +                               vpid_sync_vcpu_single(vmx->vpid);
> +                               vpid_sync_vcpu_single(vmx->nested.vpid02);
> +                       }
> +               }
> +       } else {
> +               __vmx_flush_tlb(vcpu, vmx->vpid, false);
> +       }
>  }
>
>  static inline void decache_tsc_multiplier(struct vcpu_vmx *vmx)
> --
> 2.24.1
>

  reply	other threads:[~2021-08-03  1:45 UTC|newest]

Thread overview: 83+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-03-20 21:27 [PATCH v3 00/37] KVM: x86: TLB flushing fixes and enhancements Sean Christopherson
2020-03-20 21:27 ` [PATCH v3 01/37] KVM: VMX: Flush all EPTP/VPID contexts on remote TLB flush Sean Christopherson
2021-08-03  1:45   ` Lai Jiangshan [this message]
2021-08-03 15:39     ` Sean Christopherson
2021-08-04  3:11       ` Lai Jiangshan
2021-08-04 15:33         ` Sean Christopherson
2020-03-20 21:27 ` [PATCH v3 02/37] KVM: nVMX: Validate the EPTP when emulating INVEPT(EXTENT_CONTEXT) Sean Christopherson
2020-03-23 14:51   ` Vitaly Kuznetsov
2020-03-23 15:45     ` Sean Christopherson
2020-03-23 23:46       ` Paolo Bonzini
2020-03-20 21:27 ` [PATCH v3 03/37] KVM: nVMX: Invalidate all EPTP contexts when emulating INVEPT for L1 Sean Christopherson
2020-03-23 15:24   ` Vitaly Kuznetsov
2020-03-23 15:53     ` Sean Christopherson
2020-03-23 16:24   ` Jim Mattson
2020-03-23 16:28     ` Sean Christopherson
2020-03-23 16:36       ` Jim Mattson
2020-03-23 16:44         ` Sean Christopherson
2020-03-23 23:50           ` Paolo Bonzini
2020-03-24  0:12             ` Jim Mattson
2020-03-30 18:38               ` Sean Christopherson
2020-03-20 21:28 ` [PATCH v3 04/37] KVM: nVMX: Invalidate all roots when emulating INVVPID without EPT Sean Christopherson
2020-03-23 15:34   ` Vitaly Kuznetsov
2020-03-23 16:04     ` Sean Christopherson
2020-03-23 16:33       ` Vitaly Kuznetsov
2020-03-23 16:50         ` Sean Christopherson
2020-03-23 16:57           ` Vitaly Kuznetsov
2020-03-20 21:28 ` [PATCH v3 05/37] KVM: x86: Export kvm_propagate_fault() (as kvm_inject_emulated_page_fault) Sean Christopherson
2020-03-23 15:47   ` Vitaly Kuznetsov
2020-03-23 16:24     ` Sean Christopherson
2020-03-23 23:56       ` Paolo Bonzini
2020-03-20 21:28 ` [PATCH v3 06/37] KVM: x86: Consolidate logic for injecting page faults to L1 Sean Christopherson
2020-03-24  0:47   ` Paolo Bonzini
2020-03-20 21:28 ` [PATCH v3 07/37] KVM: x86: Sync SPTEs when injecting page/EPT fault into L1 Sean Christopherson
2020-03-20 21:28 ` [PATCH v3 08/37] KVM: VMX: Skip global INVVPID fallback if vpid==0 in vpid_sync_context() Sean Christopherson
2020-03-25  9:33   ` Vitaly Kuznetsov
2020-03-20 21:28 ` [PATCH v3 09/37] KVM: VMX: Use vpid_sync_context() directly when possible Sean Christopherson
2020-03-20 21:28 ` [PATCH v3 10/37] KVM: VMX: Move vpid_sync_vcpu_addr() down a few lines Sean Christopherson
2020-03-20 21:28 ` [PATCH v3 11/37] KVM: VMX: Handle INVVPID fallback logic in vpid_sync_vcpu_addr() Sean Christopherson
2020-03-20 21:28 ` [PATCH v3 12/37] KVM: VMX: Drop redundant capability checks in low level INVVPID helpers Sean Christopherson
2020-03-20 21:28 ` [PATCH v3 13/37] KVM: nVMX: Use vpid_sync_vcpu_addr() to emulate INVVPID with address Sean Christopherson
2020-03-20 21:28 ` [PATCH v3 14/37] KVM: x86: Move "flush guest's TLB" logic to separate kvm_x86_ops hook Sean Christopherson
2020-03-25 10:23   ` Vitaly Kuznetsov
2020-03-25 15:41     ` Paolo Bonzini
2020-03-25 16:08       ` Vitaly Kuznetsov
2020-03-25 15:48     ` Sean Christopherson
2020-03-25 16:11       ` Vitaly Kuznetsov
2020-03-20 21:28 ` [PATCH v3 15/37] KVM: VMX: Clean up vmx_flush_tlb_gva() Sean Christopherson
2020-03-20 21:28 ` [PATCH v3 16/37] KVM: x86: Drop @invalidate_gpa param from kvm_x86_ops' tlb_flush() Sean Christopherson
2020-03-25 11:23   ` Vitaly Kuznetsov
2020-03-20 21:28 ` [PATCH v3 17/37] KVM: SVM: Wire up ->tlb_flush_guest() directly to svm_flush_tlb() Sean Christopherson
2020-03-25 11:23   ` Vitaly Kuznetsov
2020-03-20 21:28 ` [PATCH v3 18/37] KVM: VMX: Move vmx_flush_tlb() to vmx.c Sean Christopherson
2020-03-25 11:25   ` Vitaly Kuznetsov
2020-03-20 21:28 ` [PATCH v3 19/37] KVM: nVMX: Move nested_get_vpid02() to vmx/nested.h Sean Christopherson
2020-03-25 11:25   ` Vitaly Kuznetsov
2020-03-20 21:28 ` [PATCH v3 20/37] KVM: VMX: Introduce vmx_flush_tlb_current() Sean Christopherson
2020-03-20 21:28 ` [PATCH v3 21/37] KVM: SVM: Document the ASID logic in svm_flush_tlb() Sean Christopherson
2020-03-20 21:28 ` [PATCH v3 22/37] KVM: x86: Rename ->tlb_flush() to ->tlb_flush_all() Sean Christopherson
2020-03-20 21:28 ` [PATCH v3 23/37] KVM: nVMX: Add helper to handle TLB flushes on nested VM-Enter/VM-Exit Sean Christopherson
2021-10-28 13:11   ` Lai Jiangshan
2021-10-28 15:22     ` Sean Christopherson
2021-10-29  0:44       ` Lai Jiangshan
2021-10-29 17:10         ` Sean Christopherson
2021-10-30  1:34           ` Lai Jiangshan
2021-11-04 17:47             ` Sean Christopherson
2020-03-20 21:28 ` [PATCH v3 24/37] KVM: x86: Introduce KVM_REQ_TLB_FLUSH_CURRENT to flush current ASID Sean Christopherson
2020-03-20 21:28 ` [PATCH v3 25/37] KVM: x86/mmu: Use KVM_REQ_TLB_FLUSH_CURRENT for MMU specific flushes Sean Christopherson
2020-03-20 21:28 ` [PATCH v3 26/37] KVM: nVMX: Selectively use TLB_FLUSH_CURRENT for nested VM-Enter/VM-Exit Sean Christopherson
2020-03-20 21:28 ` [PATCH v3 27/37] KVM: nVMX: Reload APIC access page on nested VM-Exit only if necessary Sean Christopherson
2020-03-20 21:28 ` [PATCH v3 28/37] KVM: VMX: Retrieve APIC access page HPA only when necessary Sean Christopherson
2020-03-20 21:28 ` [PATCH v3 29/37] KVM: VMX: Don't reload APIC access page if its control is disabled Sean Christopherson
2020-03-20 21:28 ` [PATCH v3 30/37] KVM: x86/mmu: Move fast_cr3_switch() side effects to __kvm_mmu_new_cr3() Sean Christopherson
2020-03-20 21:28 ` [PATCH v3 31/37] KVM: x86/mmu: Add separate override for MMU sync during fast CR3 switch Sean Christopherson
2020-03-24 11:07   ` Paolo Bonzini
2020-03-20 21:28 ` [PATCH v3 32/37] KVM: x86/mmu: Add module param to force TLB flush on root reuse Sean Christopherson
2020-03-20 21:28 ` [PATCH v3 33/37] KVM: nVMX: Skip MMU sync on nested VMX transition when possible Sean Christopherson
2020-03-24 11:19   ` Paolo Bonzini
2020-03-20 21:28 ` [PATCH v3 34/37] KVM: nVMX: Don't flush TLB on nested VMX transition Sean Christopherson
2020-03-24 11:20   ` Paolo Bonzini
2020-03-24 18:10     ` Sean Christopherson
2020-03-20 21:28 ` [PATCH v3 35/37] KVM: nVMX: Free only the affected contexts when emulating INVEPT Sean Christopherson
2020-03-20 21:28 ` [PATCH v3 36/37] KVM: x86: Replace "cr3" with "pgd" in "new cr3/pgd" related code Sean Christopherson
2020-03-20 21:28 ` [PATCH v3 37/37] KVM: VMX: Clean cr3/pgd handling in vmx_load_mmu_pgd() Sean Christopherson

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CAJhGHyCPyu6BVZwqvySeT2LSr81Xospdv2O=ssvTQv0Rvky0UA@mail.gmail.com' \
    --to=jiangshanlai+lkml@gmail.com \
    --cc=kvm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=pbonzini@redhat.com \
    --cc=seanjc@google.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).