redonkable/remarkable-linux
redonkable/remarkable-linux
1
0
Fork 0
Code Releases Activity

KVM: set_memory_region: Disallow changing read-only attribute later

Browse source 
As Xiao pointed out, there are a few problems with it:
 - kvm_arch_commit_memory_region() write protects the memory slot only
   for GET_DIRTY_LOG when modifying the flags.
 - FNAME(sync_page) uses the old spte value to set a new one without
   checking KVM_MEM_READONLY flag.

Since we flush all shadow pages when creating a new slot, the simplest
fix is to disallow such problematic flag changes: this is safe because
no one is doing such things.

Reviewed-by: Gleb Natapov <gleb@redhat.com>
Signed-off-by: Takuya Yoshikawa <yoshikawa_takuya_b1@lab.ntt.co.jp>
Cc: Xiao Guangrong <xiaoguangrong@linux.vnet.ibm.com>
Cc: Alex Williamson <alex.williamson@redhat.com>
Signed-off-by: Marcelo Tosatti <mtosatti@redhat.com>
This commit is contained in:
Takuya Yoshikawa 2013-01-30 19:40:41 +09:00 committed by Marcelo Tosatti
parent f64c039893
commit 75d61fbcf5
2 changed files with 18 additions and 29 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

12
Documentation/virtual/kvm/api.txt
View file

@ -874,12 +874,12 @@ It is recommended that the lower 21 bits of guest_phys_addr and userspace_addr
be identical. This allows large pages in the guest to be backed by large be identical. This allows large pages in the guest to be backed by large
pages in the host. pages in the host.
The flags field supports two flag, KVM_MEM_LOG_DIRTY_PAGES, which instructs The flags field supports two flags: KVM_MEM_LOG_DIRTY_PAGES and
kvm to keep track of writes to memory within the slot. See KVM_GET_DIRTY_LOG KVM_MEM_READONLY. The former can be set to instruct KVM to keep track of
ioctl. The KVM_CAP_READONLY_MEM capability indicates the availability of the writes to memory within the slot. See KVM_GET_DIRTY_LOG ioctl to know how to
KVM_MEM_READONLY flag. When this flag is set for a memory region, KVM only use it. The latter can be set, if KVM_CAP_READONLY_MEM capability allows it,
allows read accesses. Writes will be posted to userspace as KVM_EXIT_MMIO to make a new slot read-only. In this case, writes to this memory will be
exits. posted to userspace as KVM_EXIT_MMIO exits.
When the KVM_CAP_SYNC_MMU capability is available, changes in the backing of When the KVM_CAP_SYNC_MMU capability is available, changes in the backing of
the memory region are automatically reflected into the guest. For example, an the memory region are automatically reflected into the guest. For example, an

29
virt/kvm/kvm_main.c
View file

@ -754,7 +754,6 @@ int __kvm_set_memory_region(struct kvm *kvm,
struct kvm_memory_slot *slot; struct kvm_memory_slot *slot;
struct kvm_memory_slot old, new; struct kvm_memory_slot old, new;
struct kvm_memslots *slots = NULL, *old_memslots; struct kvm_memslots *slots = NULL, *old_memslots;
bool old_iommu_mapped;
enum kvm_mr_change change; enum kvm_mr_change change;
r = check_memory_region_flags(mem); r = check_memory_region_flags(mem);
@ -797,15 +796,14 @@ int __kvm_set_memory_region(struct kvm *kvm,
new.npages = npages; new.npages = npages;
new.flags = mem->flags; new.flags = mem->flags;
old_iommu_mapped = old.npages;
r = -EINVAL; r = -EINVAL;
if (npages) { if (npages) {
if (!old.npages) if (!old.npages)
change = KVM_MR_CREATE; change = KVM_MR_CREATE;
else { /* Modify an existing slot. */ else { /* Modify an existing slot. */
if ((mem->userspace_addr != old.userspace_addr) || if ((mem->userspace_addr != old.userspace_addr) ||
(npages != old.npages)) (npages != old.npages) ||
((new.flags ^ old.flags) & KVM_MEM_READONLY))
goto out; goto out;
if (base_gfn != old.base_gfn) if (base_gfn != old.base_gfn)
@ -867,7 +865,6 @@ int __kvm_set_memory_region(struct kvm *kvm,
/* slot was deleted or moved, clear iommu mapping */ /* slot was deleted or moved, clear iommu mapping */
kvm_iommu_unmap_pages(kvm, &old); kvm_iommu_unmap_pages(kvm, &old);
old_iommu_mapped = false;
/* From this point no new shadow pages pointing to a deleted, /* From this point no new shadow pages pointing to a deleted,
* or moved, memslot will be created. * or moved, memslot will be created.
* *
@ -898,26 +895,18 @@ int __kvm_set_memory_region(struct kvm *kvm,
/* /*
* IOMMU mapping: New slots need to be mapped. Old slots need to be * IOMMU mapping: New slots need to be mapped. Old slots need to be
* un-mapped and re-mapped if their base changes or if flags that the * un-mapped and re-mapped if their base changes. Since base change
* iommu cares about change (read-only). Base change unmapping is * unmapping is handled above with slot deletion, mapping alone is
* handled above with slot deletion, so we only unmap incompatible * needed here. Anything else the iommu might care about for existing
* flags here. Anything else the iommu might care about for existing * slots (size changes, userspace addr changes and read-only flag
* slots (size changes, userspace addr changes) is disallowed above, * changes) is disallowed above, so any other attribute changes getting
* so any other attribute changes getting here can be skipped. * here can be skipped.
*/ */
if (change != KVM_MR_DELETE) { if ((change == KVM_MR_CREATE) || (change == KVM_MR_MOVE)) {
if (old_iommu_mapped &&
((new.flags ^ old.flags) & KVM_MEM_READONLY)) {
kvm_iommu_unmap_pages(kvm, &old);
old_iommu_mapped = false;
}
if (!old_iommu_mapped) {
r = kvm_iommu_map_pages(kvm, &new); r = kvm_iommu_map_pages(kvm, &new);
if (r) if (r)
goto out_slots; goto out_slots;
} }
}
/* actual memory is freed via old in kvm_free_physmem_slot below */ /* actual memory is freed via old in kvm_free_physmem_slot below */
if (change == KVM_MR_DELETE) { if (change == KVM_MR_DELETE) {