FAILED: patch "[PATCH] KVM: x86: Partially allow KVM_SET_CPUID{,2} after KVM_RUN" failed to apply to 5.16-stable tree
Subsystems (3)
[email protected]
maintainerThe patch below does not apply to the 5.16-stable tree.
If someone wants it applied there, or to any other stable or longterm tree, then please email the backport, including the original git commit id to <[email protected]>.
thanks,
greg k-h
------------------ original commit in Linus's tree ------------------
From c6617c61e8fe44b9e9fdfede921f61cac6b5149d Mon Sep 17 00:00:00 2001
From: Vitaly Kuznetsov <[email protected]>
Date: Mon, 17 Jan 2022 16:05:40 +0100
Subject: [PATCH] KVM: x86: Partially allow KVM_SET_CPUID{,2} after KVM_RUN
Commit feb627e8d6f6 ("KVM: x86: Forbid KVM_SET_CPUID{,2} after KVM_RUN") forbade changing CPUID altogether but unfortunately this is not fully compatible with existing VMMs. In particular, QEMU reuses vCPU fds for CPU hotplug after unplug and it calls KVM_SET_CPUID2. Instead of full ban, check whether the supplied CPUID data is equal to what was previously set.
Message-Id: <[email protected]>
arch/x86/kvm/cpuid.c +36 -0
@@ -119,6 +119,28 @@ static int kvm_check_cpuid(struct kvm_vcpu *vcpu,return fpu_enable_guest_xfd_features (&vcpu->arch.guest_fpu, xfeatures); } +/* Check whether the supplied CPUID data is equal to what is already set for the vCPU. */ +static int kvm_cpuid_check_equal (struct kvm_vcpu *vcpu,struct kvm_cpuid_entry2 *e2, +int nent) +{ +struct kvm_cpuid_entry2 *orig; +int i; + +if (nent != vcpu->arch.cpuid_nent) +return -EINVAL ; + +for (i =0 ; i < nent; i++) { + orig = &vcpu->arch.cpuid_entries[i]; +if (e2[i].function != orig->function || + e2[i].index != orig->index || + e2[i].eax != orig->eax || e2[i].ebx != orig->ebx || + e2[i].ecx != orig->ecx || e2[i].edx != orig->edx) +return -EINVAL ; + } + +return 0 ; +} +static void kvm_update_kvm_cpuid_base (struct kvm_vcpu *vcpu) {u32 function;
@@ -313,6 +335,20 @@ static int kvm_set_cpuid(struct kvm_vcpu *vcpu, struct kvm_cpuid_entry2 *e2,__kvm_update_cpuid_runtime (vcpu, e2, nent); +/* +* 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. It would've been better to forbid any +* KVM_SET_CPUID{,2} calls after KVM_RUN altogether but unfortunately +* some VMMs (e.g. QEMU) reuse vCPU fds for CPU hotplug/unplug and do +* KVM_SET_CPUID{,2} again. To support this legacy behavior, check +* whether the supplied CPUID data is equal to what's already set. +*/ +if (vcpu->arch.last_vmentry_cpu !=-1 ) +return kvm_cpuid_check_equal (vcpu, e2, nent); + r =kvm_check_cpuid (vcpu, e2, nent);if (r)return r;
arch/x86/kvm/x86.c +0 -19
@@ -5230,17 +5230,6 @@ 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;
@@ -5251,14 +5240,6 @@ 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;
Quoted 4 lines · <[email protected]> writes:
The patch below does not apply to the 5.16-stable tree.
If someone wants it applied there, or to any other stable or longterm tree, then please email the backport, including the original git commit id to <[email protected]>.
I'll do the backporting.
Side note: there's (at least) one more fix needed in the area for SGX enabled CPUs:
https://lore.kernel.org/kvm/[email protected]/
("No functional change intended" doesn't apply anymore as the order has changed). Hope it'll make upstream (Cc: stable) in the next rc.
Quoted 118 lines
thanks,
greg k-h
------------------ original commit in Linus's tree ------------------
From
c6617c61e8fe44b9e9fdfede921f61cac6b5149dMon Sep 17 00:00:00 2001
From: Vitaly Kuznetsov <[email protected]>
Date: Mon, 17 Jan 2022 16:05:40 +0100
Subject: [PATCH] KVM: x86: Partially allow KVM_SET_CPUID{,2} afterKVM_RUNCommit
feb627e8d6f6("KVM: x86: Forbid KVM_SET_CPUID{,2} afterKVM_RUN") forbade changing CPUID altogether but unfortunately this is not fully compatible with existing VMMs. In particular, QEMU reuses vCPU fds for CPU hotplug after unplug and it callsKVM_SET_CPUID2. Instead of full ban, check whether the supplied CPUID data is equal to what was previously set.Reported-by Igor Mammedov <[email protected]>Fixes feb627e8d6f6 ("KVM: x86: Forbid KVM_SET_CPUID{,2} after KVM_RUN")Signed-off-by Vitaly Kuznetsov <[email protected]>Message-Id: <[email protected]>
[Do not call kvm_find_cpuid_entry repeatedly. - Paolo]Signed-off-by Paolo Bonzini <[email protected]>arch/x86/kvm/cpuid.c +36
-0@@ -119,6 +119,28 @@ static int kvm_check_cpuid(struct kvm_vcpu *vcpu,return fpu_enable_guest_xfd_features (&vcpu->arch.guest_fpu, xfeatures); } +/* Check whether the supplied CPUID data is equal to what is already set for the vCPU. */ +static int kvm_cpuid_check_equal (struct kvm_vcpu *vcpu,struct kvm_cpuid_entry2 *e2, +int nent) +{ +struct kvm_cpuid_entry2 *orig; +int i; + +if (nent != vcpu->arch.cpuid_nent) +return -EINVAL ; + +for (i =0 ; i < nent; i++) { + orig = &vcpu->arch.cpuid_entries[i]; +if (e2[i].function != orig->function || + e2[i].index != orig->index || + e2[i].eax != orig->eax || e2[i].ebx != orig->ebx || + e2[i].ecx != orig->ecx || e2[i].edx != orig->edx) +return -EINVAL ; + } + +return 0 ; +} +static void kvm_update_kvm_cpuid_base (struct kvm_vcpu *vcpu) {u32 function;@@ -313,6 +335,20 @@ static int kvm_set_cpuid(struct kvm_vcpu *vcpu, struct kvm_cpuid_entry2 *e2,__kvm_update_cpuid_runtime (vcpu, e2, nent); +/* +* 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. It would've been better to forbid any +* KVM_SET_CPUID{,2} calls after KVM_RUN altogether but unfortunately +* some VMMs (e.g. QEMU) reuse vCPU fds for CPU hotplug/unplug and do +* KVM_SET_CPUID{,2} again. To support this legacy behavior, check +* whether the supplied CPUID data is equal to what's already set. +*/ +if (vcpu->arch.last_vmentry_cpu !=-1 ) +return kvm_cpuid_check_equal (vcpu, e2, nent); + r =kvm_check_cpuid (vcpu, e2, nent);if (r)return r;arch/x86/kvm/x86.c +0
-19@@ -5230,17 +5230,6 @@ 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;@@ -5251,14 +5240,6 @@ 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;
Signature · 1 line
Vitaly