Re: [PATCH 1/6] KVM: Explicitly verify target vCPU is online in kvm_get_vcpu()

[Sean Christopherson <seanjc@xxxxxxxxxx>]

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;
}