[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <a653b3e3-a75b-cdd4-bf58-1bfc9691299c@amd.com>
Date: Thu, 10 Oct 2024 18:33:24 +0200
From: "Gupta, Pankaj" <pankaj.gupta@....com>
To: Sean Christopherson <seanjc@...gle.com>
Cc: Paolo Bonzini <pbonzini@...hat.com>, kvm@...r.kernel.org,
linux-kernel@...r.kernel.org, Will Deacon <will@...nel.org>,
Michal Luczaj <mhal@...x.co>, Alexander Potapenko <glider@...gle.com>,
Marc Zyngier <maz@...nel.org>, Oliver Upton <oliver.upton@...ux.dev>
Subject: Re: [PATCH 1/6] KVM: Explicitly verify target vCPU is online in
kvm_get_vcpu()
On 10/10/2024 5:54 PM, Sean Christopherson wrote:
> On Thu, Oct 10, 2024, Pankaj Gupta wrote:
>> On 10/9/2024 5:04 PM, Sean Christopherson wrote:
>>> diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
>>> index db567d26f7b9..450dd0444a92 100644
>>> --- a/include/linux/kvm_host.h
>>> +++ b/include/linux/kvm_host.h
>>> @@ -969,6 +969,15 @@ static inline struct kvm_io_bus *kvm_get_bus(struct kvm *kvm, enum kvm_bus idx)
>>> static inline struct kvm_vcpu *kvm_get_vcpu(struct kvm *kvm, int i)
>>> {
>>> int num_vcpus = atomic_read(&kvm->online_vcpus);
>>> +
>>> + /*
>>> + * Explicitly verify the target vCPU is online, as the anti-speculation
>>> + * logic only limits the CPU's ability to speculate, e.g. given a "bad"
>>> + * index, clamping the index to 0 would return vCPU0, not NULL.
>>> + */
>>> + if (i >= num_vcpus)
>>> + return NULL;
>>
>> Would sev.c needs a NULL check for?
>>
>> sev_migrate_from()
>> ...
>> src_vcpu = kvm_get_vcpu(src_kvm, i);
>> src_svm = to_svm(src_vcpu);
>> ...
>
> Nope, sev_check_source_vcpus() verifies the source and destination have the same
> number of online vCPUs before calling sev_migrate_from(), and it's all done with
> both VMs locked.
>
> static int sev_check_source_vcpus(struct kvm *dst, struct kvm *src)
> {
> struct kvm_vcpu *src_vcpu;
> unsigned long i;
>
> if (!sev_es_guest(src))
> return 0;
>
> if (atomic_read(&src->online_vcpus) != atomic_read(&dst->online_vcpus))
> return -EINVAL;
>
> kvm_for_each_vcpu(i, src_vcpu, src) {
> if (!src_vcpu->arch.guest_state_protected)
> return -EINVAL;
> }
>
> return 0;
> }
Yes, right!
Thanks,
Pankaj
Powered by blists - more mailing lists