diff mbox

[backport,hints,for,<3.10] KVM: x86: Convert vapic synchronization to _cached functions (CVE-2013-6368)

Message ID 1387193897-24658-1-git-send-email-pbonzini@redhat.com (mailing list archive)
State New, archived
Headers show

Commit Message

Paolo Bonzini Dec. 16, 2013, 11:38 a.m. UTC 
The KVM patch "fix vapic memory corruption" applies to most kernels that
have KVM, but the fix does not apply on many older branches.  The APIs it
uses are available in 3.1, but until 3.9 kvm_gfn_to_hva_cache_init had
one fewer parameter.

The comments in this patch should help fixing kvm_lapic_set_vapic_addr
in older kernels.  I will review the backports as they are posted to
LKML (if I am CCed...).

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 arch/x86/kvm/lapic.c | 38 +++++++++++++++++++++++++++++---------
 arch/x86/kvm/lapic.h |  4 ++--
 arch/x86/kvm/x86.c   | 33 +--------------------------------
 3 files changed, 32 insertions(+), 43 deletions(-)

Comments

Greg Kroah-Hartman Dec. 16, 2013, 3:24 p.m. UTC | #1 
On Mon, Dec 16, 2013 at 12:38:17PM +0100, Paolo Bonzini wrote:
> The KVM patch "fix vapic memory corruption" applies to most kernels that
> have KVM, but the fix does not apply on many older branches.  The APIs it
> uses are available in 3.1, but until 3.9 kvm_gfn_to_hva_cache_init had
> one fewer parameter.
> 
> The comments in this patch should help fixing kvm_lapic_set_vapic_addr
> in older kernels.  I will review the backports as they are posted to
> LKML (if I am CCed...).
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  arch/x86/kvm/lapic.c | 38 +++++++++++++++++++++++++++++---------
>  arch/x86/kvm/lapic.h |  4 ++--
>  arch/x86/kvm/x86.c   | 33 +--------------------------------
>  3 files changed, 32 insertions(+), 43 deletions(-)

I don't understand, what are people supposed to do with this?

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Paolo Bonzini Dec. 16, 2013, 7:39 p.m. UTC | #2 
Il 16/12/2013 16:24, Greg KH ha scritto:
> On Mon, Dec 16, 2013 at 12:38:17PM +0100, Paolo Bonzini wrote:
>> The KVM patch "fix vapic memory corruption" applies to most kernels that
>> have KVM, but the fix does not apply on many older branches.  The APIs it
>> uses are available in 3.1, but until 3.9 kvm_gfn_to_hva_cache_init had
>> one fewer parameter.
>>
>> The comments in this patch should help fixing kvm_lapic_set_vapic_addr
>> in older kernels.  I will review the backports as they are posted to
>> LKML (if I am CCed...).
>>
>> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
>> ---
>>  arch/x86/kvm/lapic.c | 38 +++++++++++++++++++++++++++++---------
>>  arch/x86/kvm/lapic.h |  4 ++--
>>  arch/x86/kvm/x86.c   | 33 +--------------------------------
>>  3 files changed, 32 insertions(+), 43 deletions(-)
> 
> I don't understand, what are people supposed to do with this?

If people want to backport the patch to their stable tree, and the
upstream patch doesn't apply, they can use this as a skeleton.  I was
asked by Francis how to backport the patch, and I think code is a better
tool than text.

It's not a particularly important bug.  It's okay if you don't apply it
anywhere where it doesn't apply cleanly (i.e. 3.10 and newer only), but
the Debian and RT folks may disagree.  If they want to do the backport,
I hope this can help.

Paolo
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Greg Kroah-Hartman Dec. 18, 2013, 5:53 p.m. UTC | #3 
On Mon, Dec 16, 2013 at 12:38:17PM +0100, Paolo Bonzini wrote:
> The KVM patch "fix vapic memory corruption" applies to most kernels that
> have KVM, but the fix does not apply on many older branches.  The APIs it
> uses are available in 3.1, but until 3.9 kvm_gfn_to_hva_cache_init had
> one fewer parameter.
> 
> The comments in this patch should help fixing kvm_lapic_set_vapic_addr
> in older kernels.  I will review the backports as they are posted to
> LKML (if I am CCed...).

This fails to build on 3.4-stable, could you provide a working backport
for that kernel?

thanks,

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
index 858432287ab6..42c75a0fc467 100644
--- a/arch/x86/kvm/lapic.c
+++ b/arch/x86/kvm/lapic.c
@@ -1278,14 +1278,12 @@  void __kvm_migrate_apic_timer(struct kvm_vcpu *vcpu)
 void kvm_lapic_sync_from_vapic(struct kvm_vcpu *vcpu)
 {
 	u32 data;
-	void *vapic;
 
 	if (!irqchip_in_kernel(vcpu->kvm) || !vcpu->arch.apic->vapic_addr)
 		return;
 
-	vapic = kmap_atomic(vcpu->arch.apic->vapic_page);
-	data = *(u32 *)(vapic + offset_in_page(vcpu->arch.apic->vapic_addr));
-	kunmap_atomic(vapic);
+	kvm_read_guest_cached(vcpu->kvm, &vcpu->arch.apic->vapic_cache, &data,
+			      sizeof(u32));
 
 	apic_set_tpr(vcpu->arch.apic, data & 0xff);
 }
@@ -1295,7 +1293,6 @@  void kvm_lapic_sync_to_vapic(struct kvm_vcpu *vcpu)
 	u32 data, tpr;
 	int max_irr, max_isr;
 	struct kvm_lapic *apic;
-	void *vapic;
 
 	if (!irqchip_in_kernel(vcpu->kvm) || !vcpu->arch.apic->vapic_addr)
 		return;
@@ -1310,17 +1307,40 @@  void kvm_lapic_sync_to_vapic(struct kvm_vcpu *vcpu)
 		max_isr = 0;
 	data = (tpr & 0xff) | ((max_isr & 0xf0) << 8) | (max_irr << 24);
 
-	vapic = kmap_atomic(vcpu->arch.apic->vapic_page);
-	*(u32 *)(vapic + offset_in_page(vcpu->arch.apic->vapic_addr)) = data;
-	kunmap_atomic(vapic);
+	kvm_write_guest_cached(vcpu->kvm, &vcpu->arch.apic->vapic_cache, &data,
+			       sizeof(u32));
 }
 
-void kvm_lapic_set_vapic_addr(struct kvm_vcpu *vcpu, gpa_t vapic_addr)
+int kvm_lapic_set_vapic_addr(struct kvm_vcpu *vcpu, gpa_t vapic_addr)
 {
 	if (!irqchip_in_kernel(vcpu->kvm))
-		return;
+		return -EINVAL;
+
+	if (vapic_addr) {
+		/* For kernels with 3-argument kvm_gfn_to_hva_cache_init.  For
+		 * newer kernels, the original patch sent to stable applies.
+		 */
+		if (offset_in_page(vapic_addr) > PAGE_SIZE - 4)
+			return -EINVAL;
+
+		if (kvm_gfn_to_hva_cache_init(vcpu->kvm,
+					     &vcpu->arch.apic->vapic_cache,
+					     vapic_addr))
+			return -EINVAL;
+
+#ifdef KVM_APIC_CHECK_VAPIC
+		/* For kernels with KVM_APIC_CHECK_VAPIC #define.  */
+               __set_bit(KVM_APIC_CHECK_VAPIC, &vcpu->arch.apic_attention);
+#endif
+       } else {
+#ifdef KVM_APIC_CHECK_VAPIC
+		/* For kernels with KVM_APIC_CHECK_VAPIC #define.  */
+               __clear_bit(KVM_APIC_CHECK_VAPIC, &vcpu->arch.apic_attention);
+#endif
+	}
 
 	vcpu->arch.apic->vapic_addr = vapic_addr;
+	return 0;
 }
 
 int kvm_x2apic_msr_write(struct kvm_vcpu *vcpu, u32 msr, u64 data)
diff --git a/arch/x86/kvm/lapic.h b/arch/x86/kvm/lapic.h
index 6f4ce2575d09..6aec0714398e 100644
--- a/arch/x86/kvm/lapic.h
+++ b/arch/x86/kvm/lapic.h
@@ -15,7 +15,7 @@  struct kvm_lapic {
 	bool irr_pending;
 	void *regs;
 	gpa_t vapic_addr;
-	struct page *vapic_page;
+	struct gfn_to_hva_cache vapic_cache;
 };
 int kvm_create_lapic(struct kvm_vcpu *vcpu);
 void kvm_free_lapic(struct kvm_vcpu *vcpu);
@@ -46,7 +46,7 @@  int kvm_lapic_find_highest_irr(struct kvm_vcpu *vcpu);
 u64 kvm_get_lapic_tscdeadline_msr(struct kvm_vcpu *vcpu);
 void kvm_set_lapic_tscdeadline_msr(struct kvm_vcpu *vcpu, u64 data);
 
-void kvm_lapic_set_vapic_addr(struct kvm_vcpu *vcpu, gpa_t vapic_addr);
+int kvm_lapic_set_vapic_addr(struct kvm_vcpu *vcpu, gpa_t vapic_addr);
 void kvm_lapic_sync_from_vapic(struct kvm_vcpu *vcpu);
 void kvm_lapic_sync_to_vapic(struct kvm_vcpu *vcpu);
 
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 185a2b823a2d..896a02f15f38 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -2737,8 +2737,7 @@  long kvm_arch_vcpu_ioctl(struct file *filp,
 		r = -EFAULT;
 		if (copy_from_user(&va, argp, sizeof va))
 			goto out;
-		r = 0;
-		kvm_lapic_set_vapic_addr(vcpu, va.vapic_addr);
+		r = kvm_lapic_set_vapic_addr(vcpu, va.vapic_addr);
 		break;
 	}
 	case KVM_X86_SETUP_MCE: {
@@ -5084,33 +5083,6 @@  static void post_kvm_run_save(struct kvm_vcpu *vcpu)
 			!kvm_event_needs_reinjection(vcpu);
 }
 
-static void vapic_enter(struct kvm_vcpu *vcpu)
-{
-	struct kvm_lapic *apic = vcpu->arch.apic;
-	struct page *page;
-
-	if (!apic || !apic->vapic_addr)
-		return;
-
-	page = gfn_to_page(vcpu->kvm, apic->vapic_addr >> PAGE_SHIFT);
-
-	vcpu->arch.apic->vapic_page = page;
-}
-
-static void vapic_exit(struct kvm_vcpu *vcpu)
-{
-	struct kvm_lapic *apic = vcpu->arch.apic;
-	int idx;
-
-	if (!apic || !apic->vapic_addr)
-		return;
-
-	idx = srcu_read_lock(&vcpu->kvm->srcu);
-	kvm_release_page_dirty(apic->vapic_page);
-	mark_page_dirty(vcpu->kvm, apic->vapic_addr >> PAGE_SHIFT);
-	srcu_read_unlock(&vcpu->kvm->srcu, idx);
-}
-
 static void update_cr8_intercept(struct kvm_vcpu *vcpu)
 {
 	int max_irr, tpr;
@@ -5394,7 +5366,6 @@  static int __vcpu_run(struct kvm_vcpu *vcpu)
 	}
 
 	vcpu->srcu_idx = srcu_read_lock(&kvm->srcu);
-	vapic_enter(vcpu);
 
 	r = 1;
 	while (r > 0) {
@@ -5451,8 +5422,6 @@  static int __vcpu_run(struct kvm_vcpu *vcpu)
 
 	srcu_read_unlock(&kvm->srcu, vcpu->srcu_idx);
 
-	vapic_exit(vcpu);
-
 	return r;
 }