Message-Id: <20211103140527.752797-1-eesposit@redhat.com>
Date:   Wed,  3 Nov 2021 10:05:20 -0400
From:   Emanuele Giuseppe Esposito <eesposit@...hat.com>
To:     kvm@...r.kernel.org
Cc:     Paolo Bonzini <pbonzini@...hat.com>,
        Maxim Levitsky <mlevitsk@...hat.com>,
        Sean Christopherson <seanjc@...gle.com>,
        Vitaly Kuznetsov <vkuznets@...hat.com>,
        Wanpeng Li <wanpengli@...cent.com>,
        Jim Mattson <jmattson@...gle.com>,
        Joerg Roedel <joro@...tes.org>,
        Thomas Gleixner <tglx@...utronix.de>,
        Ingo Molnar <mingo@...hat.com>, Borislav Petkov <bp@...en8.de>,
        x86@...nel.org, "H. Peter Anvin" <hpa@...or.com>,
        linux-kernel@...r.kernel.org,
        Emanuele Giuseppe Esposito <eesposit@...hat.com>
Subject: [PATCH v5 0/7] KVM: nSVM: avoid TOC/TOU race when checking vmcb12

Currently there is a TOC/TOU race between the check of vmcb12's
efer, cr0 and cr4 registers and the later save of their values in
svm_set_*, because the guest could modify the values in the meanwhile.

To solve this issue, this series introduces and uses svm->nested.save
structure in enter_svm_guest_mode to save the current value of efer,
cr0 and cr4 and later use these to set the vcpu->arch.* state.

Similarly, svm->nested.ctl contains fields that are not used, so having
a full vmcb_control_area means passing uninitialized fields.

Patches 1,3 and 8 take care of renaming and refactoring code.
Patches 2 and 6 introduce respectively vmcb_ctrl_area_cached and
vmcb_save_area_cached.
Patches 4 and 5 use vmcb_save_area_cached to avoid TOC/TOU, and patch
7 uses vmcb_ctrl_area_cached.

Signed-off-by: Emanuele Giuseppe Esposito <eesposit@...hat.com>

---
v5:
* rebased on kvm/queue branch

v4:
* introduce _* helpers (_nested_vmcb_check_save,
  _nested_copy_vmcb_control_to_cache, _nested_copy_vmcb_save_to_cache)
  that take care of additional parameters.
* svm_set_nested_state: introduce {save, ctl}_cached variables
  to not pollute svm->nested.{save,ctl} state, especially if the
  check fails. remove also unnecessary memset added in previous versions.
* svm_get_nested_state: change stack variable ctl introduced in this series
 into a pointer that will be zeroed and freed after it has been copied to user

v3:
* merge this series with "KVM: nSVM: use vmcb_ctrl_area_cached instead
  of vmcb_control_area in nested state"
* rename "nested_load_save_from_vmcb12" in
  "nested_copy_vmcb_save_to_cache"
* rename "nested_load_control_from_vmcb12" in
  "nested_copy_vmcb_control_to_cache"
* change check functions (nested_vmcb_valid_sregs and nested_vmcb_valid_sregs)
  to accept only the vcpu parameter, since we only check
  nested state now
* rename "vmcb_is_intercept_cached" in "vmcb12_is_intercept"
  and duplicate the implementation instead of calling vmcb_is_intercept

v2:
* svm->nested.save is a separate struct vmcb_save_area_cached,
  and not vmcb_save_area.
* update also vmcb02->cr3 with svm->nested.save.cr3

RFC:
* use svm->nested.save instead of local variables.
* not dependent anymore from "KVM: nSVM: remove useless kvm_clear_*_queue"
* simplified patches, we just use the struct and not move the check
  nearer to the TOU.

Emanuele Giuseppe Esposito (7):
  KVM: nSVM: move nested_vmcb_check_cr3_cr4 logic in
    nested_vmcb_valid_sregs
  nSVM: introduce smv->nested.save to cache save area fields
  nSVM: rename nested_load_control_from_vmcb12 in
    nested_copy_vmcb_control_to_cache
  nSVM: use vmcb_save_area_cached in nested_vmcb_valid_sregs()
  nSVM: use svm->nested.save to load vmcb12 registers and avoid TOC/TOU
    races
  nSVM: introduce struct vmcb_ctrl_area_cached
  nSVM: use vmcb_ctrl_area_cached instead of vmcb_control_area in struct
    svm_nested_state

 arch/x86/kvm/svm/nested.c | 250 ++++++++++++++++++++++++--------------
 arch/x86/kvm/svm/svm.c    |   7 +-
 arch/x86/kvm/svm/svm.h    |  59 ++++++++-
 3 files changed, 213 insertions(+), 103 deletions(-)

-- 
2.27.0

Powered by blists - more mailing lists