Messages in this thread Patch in this message | | | From | Vitaly Kuznetsov <> | Subject | Re: [PATCH v2 02/13] KVM: x86: Protect kvm_hv_msr_[get|set]_crash_data() from Spectre-v1/L1TF attacks | Date | Thu, 12 Dec 2019 10:43:47 +0100 |
| |
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?
> > 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
| |