diff options
author | Andy Honig <ahonig@google.com> | 2013-02-20 17:48:10 -0500 |
---|---|---|
committer | Marcelo Tosatti <mtosatti@redhat.com> | 2013-03-19 13:17:35 -0400 |
commit | 0b79459b482e85cb7426aa7da683a9f2c97aeae1 (patch) | |
tree | 05c654b29280a76ed3ce2297825d111b956b7a64 /arch/x86/kvm/x86.c | |
parent | c300aa64ddf57d9c5d9c898a64b36877345dd4a9 (diff) |
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@google.com>
Signed-off-by: Marcelo Tosatti <mtosatti@redhat.com>
Diffstat (limited to 'arch/x86/kvm/x86.c')
-rw-r--r-- | arch/x86/kvm/x86.c | 47 |
1 files changed, 20 insertions, 27 deletions
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c index 2ade60c25402..f19ac0aca60d 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) | |||
1406 | unsigned long flags, this_tsc_khz; | 1406 | unsigned long flags, this_tsc_khz; |
1407 | struct kvm_vcpu_arch *vcpu = &v->arch; | 1407 | struct kvm_vcpu_arch *vcpu = &v->arch; |
1408 | struct kvm_arch *ka = &v->kvm->arch; | 1408 | struct kvm_arch *ka = &v->kvm->arch; |
1409 | void *shared_kaddr; | ||
1410 | s64 kernel_ns, max_kernel_ns; | 1409 | s64 kernel_ns, max_kernel_ns; |
1411 | u64 tsc_timestamp, host_tsc; | 1410 | u64 tsc_timestamp, host_tsc; |
1412 | struct pvclock_vcpu_time_info *guest_hv_clock; | 1411 | struct pvclock_vcpu_time_info guest_hv_clock; |
1413 | u8 pvclock_flags; | 1412 | u8 pvclock_flags; |
1414 | bool use_master_clock; | 1413 | bool use_master_clock; |
1415 | 1414 | ||
@@ -1463,7 +1462,7 @@ static int kvm_guest_time_update(struct kvm_vcpu *v) | |||
1463 | 1462 | ||
1464 | local_irq_restore(flags); | 1463 | local_irq_restore(flags); |
1465 | 1464 | ||
1466 | if (!vcpu->time_page) | 1465 | if (!vcpu->pv_time_enabled) |
1467 | return 0; | 1466 | return 0; |
1468 | 1467 | ||
1469 | /* | 1468 | /* |
@@ -1525,12 +1524,12 @@ static int kvm_guest_time_update(struct kvm_vcpu *v) | |||
1525 | */ | 1524 | */ |
1526 | vcpu->hv_clock.version += 2; | 1525 | vcpu->hv_clock.version += 2; |
1527 | 1526 | ||
1528 | shared_kaddr = kmap_atomic(vcpu->time_page); | 1527 | if (unlikely(kvm_read_guest_cached(v->kvm, &vcpu->pv_time, |
1529 | 1528 | &guest_hv_clock, sizeof(guest_hv_clock)))) | |
1530 | guest_hv_clock = shared_kaddr + vcpu->time_offset; | 1529 | return 0; |
1531 | 1530 | ||
1532 | /* retain PVCLOCK_GUEST_STOPPED if set in guest copy */ | 1531 | /* retain PVCLOCK_GUEST_STOPPED if set in guest copy */ |
1533 | pvclock_flags = (guest_hv_clock->flags & PVCLOCK_GUEST_STOPPED); | 1532 | pvclock_flags = (guest_hv_clock.flags & PVCLOCK_GUEST_STOPPED); |
1534 | 1533 | ||
1535 | if (vcpu->pvclock_set_guest_stopped_request) { | 1534 | if (vcpu->pvclock_set_guest_stopped_request) { |
1536 | pvclock_flags |= PVCLOCK_GUEST_STOPPED; | 1535 | pvclock_flags |= PVCLOCK_GUEST_STOPPED; |
@@ -1543,12 +1542,9 @@ static int kvm_guest_time_update(struct kvm_vcpu *v) | |||
1543 | 1542 | ||
1544 | vcpu->hv_clock.flags = pvclock_flags; | 1543 | vcpu->hv_clock.flags = pvclock_flags; |
1545 | 1544 | ||
1546 | memcpy(shared_kaddr + vcpu->time_offset, &vcpu->hv_clock, | 1545 | kvm_write_guest_cached(v->kvm, &vcpu->pv_time, |
1547 | sizeof(vcpu->hv_clock)); | 1546 | &vcpu->hv_clock, |
1548 | 1547 | sizeof(vcpu->hv_clock)); | |
1549 | kunmap_atomic(shared_kaddr); | ||
1550 | |||
1551 | mark_page_dirty(v->kvm, vcpu->time >> PAGE_SHIFT); | ||
1552 | return 0; | 1548 | return 0; |
1553 | } | 1549 | } |
1554 | 1550 | ||
@@ -1837,10 +1833,7 @@ static int kvm_pv_enable_async_pf(struct kvm_vcpu *vcpu, u64 data) | |||
1837 | 1833 | ||
1838 | static void kvmclock_reset(struct kvm_vcpu *vcpu) | 1834 | static void kvmclock_reset(struct kvm_vcpu *vcpu) |
1839 | { | 1835 | { |
1840 | if (vcpu->arch.time_page) { | 1836 | vcpu->arch.pv_time_enabled = false; |
1841 | kvm_release_page_dirty(vcpu->arch.time_page); | ||
1842 | vcpu->arch.time_page = NULL; | ||
1843 | } | ||
1844 | } | 1837 | } |
1845 | 1838 | ||
1846 | static void accumulate_steal_time(struct kvm_vcpu *vcpu) | 1839 | 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) | |||
1947 | break; | 1940 | break; |
1948 | case MSR_KVM_SYSTEM_TIME_NEW: | 1941 | case MSR_KVM_SYSTEM_TIME_NEW: |
1949 | case MSR_KVM_SYSTEM_TIME: { | 1942 | case MSR_KVM_SYSTEM_TIME: { |
1943 | u64 gpa_offset; | ||
1950 | kvmclock_reset(vcpu); | 1944 | kvmclock_reset(vcpu); |
1951 | 1945 | ||
1952 | vcpu->arch.time = data; | 1946 | vcpu->arch.time = data; |
@@ -1956,19 +1950,17 @@ int kvm_set_msr_common(struct kvm_vcpu *vcpu, struct msr_data *msr_info) | |||
1956 | if (!(data & 1)) | 1950 | if (!(data & 1)) |
1957 | break; | 1951 | break; |
1958 | 1952 | ||
1959 | /* ...but clean it before doing the actual write */ | 1953 | gpa_offset = data & ~(PAGE_MASK | 1); |
1960 | vcpu->arch.time_offset = data & ~(PAGE_MASK | 1); | ||
1961 | 1954 | ||
1962 | /* Check that the address is 32-byte aligned. */ | 1955 | /* Check that the address is 32-byte aligned. */ |
1963 | if (vcpu->arch.time_offset & | 1956 | if (gpa_offset & (sizeof(struct pvclock_vcpu_time_info) - 1)) |
1964 | (sizeof(struct pvclock_vcpu_time_info) - 1)) | ||
1965 | break; | 1957 | break; |
1966 | 1958 | ||
1967 | vcpu->arch.time_page = | 1959 | if (kvm_gfn_to_hva_cache_init(vcpu->kvm, |
1968 | gfn_to_page(vcpu->kvm, data >> PAGE_SHIFT); | 1960 | &vcpu->arch.pv_time, data & ~1ULL)) |
1969 | 1961 | vcpu->arch.pv_time_enabled = false; | |
1970 | if (is_error_page(vcpu->arch.time_page)) | 1962 | else |
1971 | vcpu->arch.time_page = NULL; | 1963 | vcpu->arch.pv_time_enabled = true; |
1972 | 1964 | ||
1973 | break; | 1965 | break; |
1974 | } | 1966 | } |
@@ -2972,7 +2964,7 @@ static int kvm_vcpu_ioctl_x86_set_xcrs(struct kvm_vcpu *vcpu, | |||
2972 | */ | 2964 | */ |
2973 | static int kvm_set_guest_paused(struct kvm_vcpu *vcpu) | 2965 | static int kvm_set_guest_paused(struct kvm_vcpu *vcpu) |
2974 | { | 2966 | { |
2975 | if (!vcpu->arch.time_page) | 2967 | if (!vcpu->arch.pv_time_enabled) |
2976 | return -EINVAL; | 2968 | return -EINVAL; |
2977 | vcpu->arch.pvclock_set_guest_stopped_request = true; | 2969 | vcpu->arch.pvclock_set_guest_stopped_request = true; |
2978 | kvm_make_request(KVM_REQ_CLOCK_UPDATE, vcpu); | 2970 | kvm_make_request(KVM_REQ_CLOCK_UPDATE, vcpu); |
@@ -6723,6 +6715,7 @@ int kvm_arch_vcpu_init(struct kvm_vcpu *vcpu) | |||
6723 | goto fail_free_wbinvd_dirty_mask; | 6715 | goto fail_free_wbinvd_dirty_mask; |
6724 | 6716 | ||
6725 | vcpu->arch.ia32_tsc_adjust_msr = 0x0; | 6717 | vcpu->arch.ia32_tsc_adjust_msr = 0x0; |
6718 | vcpu->arch.pv_time_enabled = false; | ||
6726 | kvm_async_pf_hash_reset(vcpu); | 6719 | kvm_async_pf_hash_reset(vcpu); |
6727 | kvm_pmu_init(vcpu); | 6720 | kvm_pmu_init(vcpu); |
6728 | 6721 | ||