On Thu, Dec 12, 2019 at 1:43 AM Vitaly Kuznetsov vkuznets@redhat.com wrote:
Marios Pomonis pomonis@google.com writes:
This fixes Spectre-v1/L1TF vulnerabilities in kvm_hv_msr_get_crash_data() and kvm_hv_msr_set_crash_data(). These functions contain index computations that use the (attacker-controlled) MSR number.
Just to educate myself,
in both cases 'index' is equal to 'msr - HV_X64_MSR_CRASH_P0' where 'msr' is constrained: case HV_X64_MSR_CRASH_P0 ... HV_X64_MSR_CRASH_P4: ....
and moreover, kvm_hv_{get,set}_msr_common() is only being called for a narrow set of MSRs. How can an atacker overcome these limitations?
This attack scenario relies on speculative execution. Practically, one could train the branch predictors involved to speculatively execute this path even if the adversary-supplied MSR number does not fall into the legitimate range. The adversary-supplied MSR number however is going to be used when -speculatively- computing the index of the array thus allowing an attacker to load normally illegitimate memory values in the L1 cache.
Fixes: commit e7d9513b60e8 ("kvm/x86: added hyper-v crash msrs into kvm hyperv context")
Signed-off-by: Nick Finco nifi@google.com Signed-off-by: Marios Pomonis pomonis@google.com Reviewed-by: Andrew Honig ahonig@google.com Cc: stable@vger.kernel.org
arch/x86/kvm/hyperv.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/arch/x86/kvm/hyperv.c b/arch/x86/kvm/hyperv.c index 23ff65504d7e..26408434b9bc 100644 --- a/arch/x86/kvm/hyperv.c +++ b/arch/x86/kvm/hyperv.c @@ -809,11 +809,12 @@ static int kvm_hv_msr_get_crash_data(struct kvm_vcpu *vcpu, u32 index, u64 *pdata) { struct kvm_hv *hv = &vcpu->kvm->arch.hyperv;
size_t size = ARRAY_SIZE(hv->hv_crash_param);
if (WARN_ON_ONCE(index >= ARRAY_SIZE(hv->hv_crash_param)))
if (WARN_ON_ONCE(index >= size)) return -EINVAL;
*pdata = hv->hv_crash_param[index];
*pdata = hv->hv_crash_param[array_index_nospec(index, size)]; return 0;
}
@@ -852,11 +853,12 @@ static int kvm_hv_msr_set_crash_data(struct kvm_vcpu *vcpu, u32 index, u64 data) { struct kvm_hv *hv = &vcpu->kvm->arch.hyperv;
size_t size = ARRAY_SIZE(hv->hv_crash_param);
if (WARN_ON_ONCE(index >= ARRAY_SIZE(hv->hv_crash_param)))
if (WARN_ON_ONCE(index >= size)) return -EINVAL;
hv->hv_crash_param[index] = data;
hv->hv_crash_param[array_index_nospec(index, size)] = data; return 0;
}
-- Vitaly