Commit feb627e8 authored by Vitaly Kuznetsov's avatar Vitaly Kuznetsov Committed by Paolo Bonzini
Browse files

KVM: x86: Forbid KVM_SET_CPUID{,2} after KVM_RUN 



Commit 63f5a190 ("KVM: x86: Alert userspace that KVM_SET_CPUID{,2}
after KVM_RUN is broken") officially deprecated KVM_SET_CPUID{,2} ioctls
after first successful KVM_RUN and promissed to make this sequence forbiden
in 5.16. It's time to fulfil the promise.

Signed-off-by: default avatarVitaly Kuznetsov <vkuznets@redhat.com>
Message-Id: <20211122175818.608220-3-vkuznets@redhat.com>
Signed-off-by: default avatarPaolo Bonzini <pbonzini@redhat.com>
parent 6c118643

arch/x86/kvm/mmu/mmu.c

+11 −17
Original line number Diff line number Diff line
@@ -5025,6 +5025,14 @@ void kvm_mmu_after_set_cpuid(struct kvm_vcpu *vcpu) 
	/*

	 * Invalidate all MMU roles to force them to reinitialize as CPUID

	 * information is factored into reserved bit calculations.

	 *

	 * Correctly handling multiple vCPU models with respect to paging and

	 * physical address properties) in a single VM would require tracking

	 * all relevant CPUID information in kvm_mmu_page_role. That is very

	 * undesirable as it would increase the memory requirements for

	 * gfn_track (see struct kvm_mmu_page_role comments).  For now that

	 * problem is swept under the rug; KVM's CPUID API is horrific and

	 * it's all but impossible to solve it without introducing a new API.

	 */

	vcpu->arch.root_mmu.mmu_role.ext.valid = 0;

	vcpu->arch.guest_mmu.mmu_role.ext.valid = 0;
@@ -5032,24 +5040,10 @@ void kvm_mmu_after_set_cpuid(struct kvm_vcpu *vcpu) 
	kvm_mmu_reset_context(vcpu);



	/*

	 * KVM does not correctly handle changing guest CPUID after KVM_RUN, as

	 * MAXPHYADDR, GBPAGES support, AMD reserved bit behavior, etc.. aren't

	 * tracked in kvm_mmu_page_role.  As a result, KVM may miss guest page

	 * faults due to reusing SPs/SPTEs.  Alert userspace, but otherwise

	 * sweep the problem under the rug.

	 *

	 * KVM's horrific CPUID ABI makes the problem all but impossible to

	 * solve, as correctly handling multiple vCPU models (with respect to

	 * paging and physical address properties) in a single VM would require

	 * tracking all relevant CPUID information in kvm_mmu_page_role.  That

	 * is very undesirable as it would double the memory requirements for

	 * gfn_track (see struct kvm_mmu_page_role comments), and in practice

	 * no sane VMM mucks with the core vCPU model on the fly.

	 */

	if (vcpu->arch.last_vmentry_cpu != -1) {

		pr_warn_ratelimited("KVM: KVM_SET_CPUID{,2} after KVM_RUN may cause guest instability\n");

		pr_warn_ratelimited("KVM: KVM_SET_CPUID{,2} will fail after KVM_RUN starting with Linux 5.16\n");

	}

	 * Changing guest CPUID after KVM_RUN is forbidden, see the comment in

	 * kvm_arch_vcpu_ioctl().

	 */

	KVM_BUG_ON(vcpu->arch.last_vmentry_cpu != -1, vcpu->kvm);

}



void kvm_mmu_reset_context(struct kvm_vcpu *vcpu)

arch/x86/kvm/x86.c

+19 −0
Original line number Diff line number Diff line
@@ -5148,6 +5148,17 @@ long kvm_arch_vcpu_ioctl(struct file *filp, 
		struct kvm_cpuid __user *cpuid_arg = argp;

		struct kvm_cpuid cpuid;



		/*

		 * KVM does not correctly handle changing guest CPUID after KVM_RUN, as

		 * MAXPHYADDR, GBPAGES support, AMD reserved bit behavior, etc.. aren't

		 * tracked in kvm_mmu_page_role.  As a result, KVM may miss guest page

		 * faults due to reusing SPs/SPTEs.  In practice no sane VMM mucks with

		 * the core vCPU model on the fly, so fail.

		 */

		r = -EINVAL;

		if (vcpu->arch.last_vmentry_cpu != -1)

			goto out;



		r = -EFAULT;

		if (copy_from_user(&cpuid, cpuid_arg, sizeof(cpuid)))

			goto out;
@@ -5158,6 +5169,14 @@ long kvm_arch_vcpu_ioctl(struct file *filp, 
		struct kvm_cpuid2 __user *cpuid_arg = argp;

		struct kvm_cpuid2 cpuid;



		/*

		 * KVM_SET_CPUID{,2} after KVM_RUN is forbidded, see the comment in

		 * KVM_SET_CPUID case above.

		 */

		r = -EINVAL;

		if (vcpu->arch.last_vmentry_cpu != -1)

			goto out;



		r = -EFAULT;

		if (copy_from_user(&cpuid, cpuid_arg, sizeof(cpuid)))

			goto out;