lkml.org 
[lkml]   [2021]   [Nov]   [11]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    SubjectRe: [PATCH v5 0/7] KVM: nSVM: avoid TOC/TOU race when checking vmcb12
    From
    On 11/3/21 15:05, Emanuele Giuseppe Esposito wrote:
    > 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@redhat.com>

    Queued (for 5.17). I changed the helper functions to have a "__" prefix
    since that's more common in Linux.

    Paolo

    >
    > ---
    > 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(-)
    >

    \
     
     \ /
      Last update: 2021-11-11 14:56    [W:4.191 / U:0.060 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site