[kernel/f18] CVE-2013-1796 kvm: buffer overflow in handling of MSR_KVM_SYSTEM_TIME
Josh Boyer
jwboyer at fedoraproject.org
Wed Mar 20 20:13:50 UTC 2013
commit dcfb7ac98d593cbdc549163efc37e44bb80189f3
Author: Josh Boyer <jwboyer at redhat.com>
Date: Wed Mar 20 16:12:06 2013 -0400
CVE-2013-1796 kvm: buffer overflow in handling of MSR_KVM_SYSTEM_TIME
(rhbz 917012 923966)
- CVE-2013-1797 kvm: after free issue with the handling of MSR_KVM_SYSTEM_TIME
(rhbz 917013 923967)
- CVE-2013-1798 kvm: out-of-bounds access in ioapic indirect register reads
(rhbz 917017 923968)
...nds-checking-in-ioapic-indirect-register-.patch | 44 ++++++
...-for-buffer-overflow-in-handling-of-MSR_K.patch | 41 +++++
...vert-MSR_KVM_SYSTEM_TIME-to-use-gfn_to_hv.patch | 163 ++++++++++++++++++++
kernel.spec | 28 ++++-
4 files changed, 275 insertions(+), 1 deletions(-)
---
diff --git a/0001-KVM-Fix-bounds-checking-in-ioapic-indirect-register-.patch b/0001-KVM-Fix-bounds-checking-in-ioapic-indirect-register-.patch
new file mode 100644
index 0000000..7583f74
--- /dev/null
+++ b/0001-KVM-Fix-bounds-checking-in-ioapic-indirect-register-.patch
@@ -0,0 +1,44 @@
+From a2c118bfab8bc6b8bb213abfc35201e441693d55 Mon Sep 17 00:00:00 2001
+From: Andy Honig <ahonig at google.com>
+Date: Wed, 20 Feb 2013 14:49:16 -0800
+Subject: [PATCH] KVM: Fix bounds checking in ioapic indirect register reads
+ (CVE-2013-1798)
+
+If the guest specifies a IOAPIC_REG_SELECT with an invalid value and follows
+that with a read of the IOAPIC_REG_WINDOW KVM does not properly validate
+that request. ioapic_read_indirect contains an
+ASSERT(redir_index < IOAPIC_NUM_PINS), but the ASSERT has no effect in
+non-debug builds. In recent kernels this allows a guest to cause a kernel
+oops by reading invalid memory. In older kernels (pre-3.3) this allows a
+guest to read from large ranges of host memory.
+
+Tested: tested against apic unit tests.
+
+Signed-off-by: Andrew Honig <ahonig at google.com>
+Signed-off-by: Marcelo Tosatti <mtosatti at redhat.com>
+---
+ virt/kvm/ioapic.c | 7 +++++--
+ 1 file changed, 5 insertions(+), 2 deletions(-)
+
+diff --git a/virt/kvm/ioapic.c b/virt/kvm/ioapic.c
+index ce82b94..5ba005c 100644
+--- a/virt/kvm/ioapic.c
++++ b/virt/kvm/ioapic.c
+@@ -74,9 +74,12 @@ static unsigned long ioapic_read_indirect(struct kvm_ioapic *ioapic,
+ u32 redir_index = (ioapic->ioregsel - 0x10) >> 1;
+ u64 redir_content;
+
+- ASSERT(redir_index < IOAPIC_NUM_PINS);
++ if (redir_index < IOAPIC_NUM_PINS)
++ redir_content =
++ ioapic->redirtbl[redir_index].bits;
++ else
++ redir_content = ~0ULL;
+
+- redir_content = ioapic->redirtbl[redir_index].bits;
+ result = (ioapic->ioregsel & 0x1) ?
+ (redir_content >> 32) & 0xffffffff :
+ redir_content & 0xffffffff;
+--
+1.8.1.4
+
diff --git a/0002-KVM-x86-fix-for-buffer-overflow-in-handling-of-MSR_K.patch b/0002-KVM-x86-fix-for-buffer-overflow-in-handling-of-MSR_K.patch
new file mode 100644
index 0000000..a4516e4
--- /dev/null
+++ b/0002-KVM-x86-fix-for-buffer-overflow-in-handling-of-MSR_K.patch
@@ -0,0 +1,41 @@
+From c300aa64ddf57d9c5d9c898a64b36877345dd4a9 Mon Sep 17 00:00:00 2001
+From: Andy Honig <ahonig at google.com>
+Date: Mon, 11 Mar 2013 09:34:52 -0700
+Subject: [PATCH 2/3] KVM: x86: fix for buffer overflow in handling of
+ MSR_KVM_SYSTEM_TIME (CVE-2013-1796)
+
+If the guest sets the GPA of the time_page so that the request to update the
+time straddles a page then KVM will write onto an incorrect page. The
+write is done byusing kmap atomic to get a pointer to the page for the time
+structure and then performing a memcpy to that page starting at an offset
+that the guest controls. Well behaved guests always provide a 32-byte aligned
+address, however a malicious guest could use this to corrupt host kernel
+memory.
+
+Tested: Tested against kvmclock unit test.
+
+Signed-off-by: Andrew Honig <ahonig at google.com>
+Signed-off-by: Marcelo Tosatti <mtosatti at redhat.com>
+---
+ arch/x86/kvm/x86.c | 5 +++++
+ 1 file changed, 5 insertions(+)
+
+diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
+index f7c850b..2ade60c 100644
+--- a/arch/x86/kvm/x86.c
++++ b/arch/x86/kvm/x86.c
+@@ -1959,6 +1959,11 @@ int kvm_set_msr_common(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
+ /* ...but clean it before doing the actual write */
+ vcpu->arch.time_offset = data & ~(PAGE_MASK | 1);
+
++ /* Check that the address is 32-byte aligned. */
++ if (vcpu->arch.time_offset &
++ (sizeof(struct pvclock_vcpu_time_info) - 1))
++ break;
++
+ vcpu->arch.time_page =
+ gfn_to_page(vcpu->kvm, data >> PAGE_SHIFT);
+
+--
+1.8.1.4
+
diff --git a/0003-KVM-x86-Convert-MSR_KVM_SYSTEM_TIME-to-use-gfn_to_hv.patch b/0003-KVM-x86-Convert-MSR_KVM_SYSTEM_TIME-to-use-gfn_to_hv.patch
new file mode 100644
index 0000000..7a2fe65
--- /dev/null
+++ b/0003-KVM-x86-Convert-MSR_KVM_SYSTEM_TIME-to-use-gfn_to_hv.patch
@@ -0,0 +1,163 @@
+From 0b79459b482e85cb7426aa7da683a9f2c97aeae1 Mon Sep 17 00:00:00 2001
+From: Andy Honig <ahonig at google.com>
+Date: Wed, 20 Feb 2013 14:48:10 -0800
+Subject: [PATCH 3/3] KVM: x86: Convert MSR_KVM_SYSTEM_TIME to use
+ gfn_to_hva_cache functions (CVE-2013-1797)
+
+There is a potential use after free issue with the handling of
+MSR_KVM_SYSTEM_TIME. If the guest specifies a GPA in a movable or removable
+memory such as frame buffers then KVM might continue to write to that
+address even after it's removed via KVM_SET_USER_MEMORY_REGION. KVM pins
+the page in memory so it's unlikely to cause an issue, but if the user
+space component re-purposes the memory previously used for the guest, then
+the guest will be able to corrupt that memory.
+
+Tested: Tested against kvmclock unit test
+
+Signed-off-by: Andrew Honig <ahonig at google.com>
+Signed-off-by: Marcelo Tosatti <mtosatti at redhat.com>
+---
+ arch/x86/include/asm/kvm_host.h | 4 ++--
+ arch/x86/kvm/x86.c | 47 ++++++++++++++++++-----------------------
+ 2 files changed, 22 insertions(+), 29 deletions(-)
+
+diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
+index 635a74d..4979778 100644
+--- a/arch/x86/include/asm/kvm_host.h
++++ b/arch/x86/include/asm/kvm_host.h
+@@ -414,8 +414,8 @@ struct kvm_vcpu_arch {
+ gpa_t time;
+ struct pvclock_vcpu_time_info hv_clock;
+ unsigned int hw_tsc_khz;
+- unsigned int time_offset;
+- struct page *time_page;
++ struct gfn_to_hva_cache pv_time;
++ bool pv_time_enabled;
+ /* set guest stopped flag in pvclock flags field */
+ bool pvclock_set_guest_stopped_request;
+
+diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
+index 2ade60c..f19ac0a 100644
+--- a/arch/x86/kvm/x86.c
++++ b/arch/x86/kvm/x86.c
+@@ -1406,10 +1406,9 @@ static int kvm_guest_time_update(struct kvm_vcpu *v)
+ unsigned long flags, this_tsc_khz;
+ struct kvm_vcpu_arch *vcpu = &v->arch;
+ struct kvm_arch *ka = &v->kvm->arch;
+- void *shared_kaddr;
+ s64 kernel_ns, max_kernel_ns;
+ u64 tsc_timestamp, host_tsc;
+- struct pvclock_vcpu_time_info *guest_hv_clock;
++ struct pvclock_vcpu_time_info guest_hv_clock;
+ u8 pvclock_flags;
+ bool use_master_clock;
+
+@@ -1463,7 +1462,7 @@ static int kvm_guest_time_update(struct kvm_vcpu *v)
+
+ local_irq_restore(flags);
+
+- if (!vcpu->time_page)
++ if (!vcpu->pv_time_enabled)
+ return 0;
+
+ /*
+@@ -1525,12 +1524,12 @@ static int kvm_guest_time_update(struct kvm_vcpu *v)
+ */
+ vcpu->hv_clock.version += 2;
+
+- shared_kaddr = kmap_atomic(vcpu->time_page);
+-
+- guest_hv_clock = shared_kaddr + vcpu->time_offset;
++ if (unlikely(kvm_read_guest_cached(v->kvm, &vcpu->pv_time,
++ &guest_hv_clock, sizeof(guest_hv_clock))))
++ return 0;
+
+ /* retain PVCLOCK_GUEST_STOPPED if set in guest copy */
+- pvclock_flags = (guest_hv_clock->flags & PVCLOCK_GUEST_STOPPED);
++ pvclock_flags = (guest_hv_clock.flags & PVCLOCK_GUEST_STOPPED);
+
+ if (vcpu->pvclock_set_guest_stopped_request) {
+ pvclock_flags |= PVCLOCK_GUEST_STOPPED;
+@@ -1543,12 +1542,9 @@ static int kvm_guest_time_update(struct kvm_vcpu *v)
+
+ vcpu->hv_clock.flags = pvclock_flags;
+
+- memcpy(shared_kaddr + vcpu->time_offset, &vcpu->hv_clock,
+- sizeof(vcpu->hv_clock));
+-
+- kunmap_atomic(shared_kaddr);
+-
+- mark_page_dirty(v->kvm, vcpu->time >> PAGE_SHIFT);
++ kvm_write_guest_cached(v->kvm, &vcpu->pv_time,
++ &vcpu->hv_clock,
++ sizeof(vcpu->hv_clock));
+ return 0;
+ }
+
+@@ -1837,10 +1833,7 @@ static int kvm_pv_enable_async_pf(struct kvm_vcpu *vcpu, u64 data)
+
+ static void kvmclock_reset(struct kvm_vcpu *vcpu)
+ {
+- if (vcpu->arch.time_page) {
+- kvm_release_page_dirty(vcpu->arch.time_page);
+- vcpu->arch.time_page = NULL;
+- }
++ vcpu->arch.pv_time_enabled = false;
+ }
+
+ static void accumulate_steal_time(struct kvm_vcpu *vcpu)
+@@ -1947,6 +1940,7 @@ int kvm_set_msr_common(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
+ break;
+ case MSR_KVM_SYSTEM_TIME_NEW:
+ case MSR_KVM_SYSTEM_TIME: {
++ u64 gpa_offset;
+ kvmclock_reset(vcpu);
+
+ vcpu->arch.time = data;
+@@ -1956,19 +1950,17 @@ int kvm_set_msr_common(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
+ if (!(data & 1))
+ break;
+
+- /* ...but clean it before doing the actual write */
+- vcpu->arch.time_offset = data & ~(PAGE_MASK | 1);
++ gpa_offset = data & ~(PAGE_MASK | 1);
+
+ /* Check that the address is 32-byte aligned. */
+- if (vcpu->arch.time_offset &
+- (sizeof(struct pvclock_vcpu_time_info) - 1))
++ if (gpa_offset & (sizeof(struct pvclock_vcpu_time_info) - 1))
+ break;
+
+- vcpu->arch.time_page =
+- gfn_to_page(vcpu->kvm, data >> PAGE_SHIFT);
+-
+- if (is_error_page(vcpu->arch.time_page))
+- vcpu->arch.time_page = NULL;
++ if (kvm_gfn_to_hva_cache_init(vcpu->kvm,
++ &vcpu->arch.pv_time, data & ~1ULL))
++ vcpu->arch.pv_time_enabled = false;
++ else
++ vcpu->arch.pv_time_enabled = true;
+
+ break;
+ }
+@@ -2972,7 +2964,7 @@ static int kvm_vcpu_ioctl_x86_set_xcrs(struct kvm_vcpu *vcpu,
+ */
+ static int kvm_set_guest_paused(struct kvm_vcpu *vcpu)
+ {
+- if (!vcpu->arch.time_page)
++ if (!vcpu->arch.pv_time_enabled)
+ return -EINVAL;
+ vcpu->arch.pvclock_set_guest_stopped_request = true;
+ kvm_make_request(KVM_REQ_CLOCK_UPDATE, vcpu);
+@@ -6723,6 +6715,7 @@ int kvm_arch_vcpu_init(struct kvm_vcpu *vcpu)
+ goto fail_free_wbinvd_dirty_mask;
+
+ vcpu->arch.ia32_tsc_adjust_msr = 0x0;
++ vcpu->arch.pv_time_enabled = false;
+ kvm_async_pf_hash_reset(vcpu);
+ kvm_pmu_init(vcpu);
+
+--
+1.8.1.4
+
diff --git a/kernel.spec b/kernel.spec
index 25519b8..46563ed 100644
--- a/kernel.spec
+++ b/kernel.spec
@@ -62,7 +62,7 @@ Summary: The Linux kernel
# For non-released -rc kernels, this will be appended after the rcX and
# gitX tags, so a 3 here would become part of release "0.rcX.gitX.3"
#
-%global baserelease 203
+%global baserelease 204
%global fedora_build %{baserelease}
# base_sublevel is the kernel version we're starting with and patching
@@ -805,6 +805,15 @@ Patch25001: i7300_edac_single_mode_fixup.patch
#rhbz 922304
Patch25002: drm-ilk-rc6-reverts.patch
+#CVE-2013-1798 rhbz 917017 923968
+Patch25003: 0001-KVM-Fix-bounds-checking-in-ioapic-indirect-register-.patch
+
+#CVE-2013-1796 rhbz 917012 923966
+Patch25004: 0002-KVM-x86-fix-for-buffer-overflow-in-handling-of-MSR_K.patch
+
+#CVE-2013-1797 rhbz 917013 923967
+Patch25005: 0003-KVM-x86-Convert-MSR_KVM_SYSTEM_TIME-to-use-gfn_to_hv.patch
+
# END OF PATCH DEFINITIONS
%endif
@@ -1560,6 +1569,15 @@ ApplyPatch USB-cdc-wdm-fix-buffer-overflow.patch
#rhbz 922304
ApplyPatch drm-ilk-rc6-reverts.patch -R
+#CVE-2013-1798 rhbz 917017 923968
+ApplyPatch 0001-KVM-Fix-bounds-checking-in-ioapic-indirect-register-.patch
+
+#CVE-2013-1796 rhbz 917012 923966
+ApplyPatch 0002-KVM-x86-fix-for-buffer-overflow-in-handling-of-MSR_K.patch
+
+#CVE-2013-1797 rhbz 917013 923967
+ApplyPatch 0003-KVM-x86-Convert-MSR_KVM_SYSTEM_TIME-to-use-gfn_to_hv.patch
+
# END OF PATCH APPLICATIONS
%endif
@@ -2417,6 +2435,14 @@ fi
# ||----w |
# || ||
%changelog
+* Wed Mar 20 2013 Josh Boyer <jwboyer at redhat.com>
+- CVE-2013-1796 kvm: buffer overflow in handling of MSR_KVM_SYSTEM_TIME
+ (rhbz 917012 923966)
+- CVE-2013-1797 kvm: after free issue with the handling of MSR_KVM_SYSTEM_TIME
+ (rhbz 917013 923967)
+- CVE-2013-1798 kvm: out-of-bounds access in ioapic indirect register reads
+ (rhbz 917017 923968)
+
* Mon Mar 18 2013 Justin M. Forbes
- Revert rc6 ilk changes from 3.8.3 stable (rhbz 922304)
More information about the scm-commits
mailing list