| Message ID | alpine.DEB.2.02.1304180936160.2452@ahonig-virtual-machine (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, Apr 18, 2013 at 09:38:14AM -0700, Andrew Honig wrote: > > If userspace creates and destroys multiple VMs within the same process > we leak 20k of memory in the userspace process context per VM. This > patch frees the memory in kvm_arch_destroy_vm. If the process exits > without closing the VM file descriptor or the file descriptor has been > shared with another process then we don't free the memory. > > It's still possible for a user space process to leak memory if the last > process to close the fd for the VM is not the process that created it. > However, this is an unexpected case that's only caused by a user space > process that's misbehaving. > > Signed-off-by: Andrew Honig <ahonig@google.com> Reviewed-by: Gleb Natapov <gleb@redhat.com> > --- > arch/x86/kvm/x86.c | 17 +++++++++++++++++ > 1 file changed, 17 insertions(+) > > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c > index 8ffac42..3b389bf 100644 > --- a/arch/x86/kvm/x86.c > +++ b/arch/x86/kvm/x86.c > @@ -6856,6 +6856,23 @@ void kvm_arch_sync_events(struct kvm *kvm) > > void kvm_arch_destroy_vm(struct kvm *kvm) > { > + if (current->mm == kvm->mm) { > + /* > + * Free memory regions allocated on behalf of userspace, > + * unless the the memory map has changed due to process exit > + * or fd copying. > + */ > + struct kvm_userspace_memory_region mem; > + memset(&mem, 0, sizeof(mem)); > + mem.slot = APIC_ACCESS_PAGE_PRIVATE_MEMSLOT; > + kvm_set_memory_region(kvm, &mem); > + > + mem.slot = IDENTITY_PAGETABLE_PRIVATE_MEMSLOT; > + kvm_set_memory_region(kvm, &mem); > + > + mem.slot = TSS_PRIVATE_MEMSLOT; > + kvm_set_memory_region(kvm, &mem); > + } > kvm_iommu_unmap_guest(kvm); > kfree(kvm->arch.vpic); > kfree(kvm->arch.vioapic); > -- > 1.7.10.4 > > -- > 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 -- Gleb. -- 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
On Thu, Apr 18, 2013 at 09:38:14AM -0700, Andrew Honig wrote: > > If userspace creates and destroys multiple VMs within the same process > we leak 20k of memory in the userspace process context per VM. This > patch frees the memory in kvm_arch_destroy_vm. If the process exits > without closing the VM file descriptor or the file descriptor has been > shared with another process then we don't free the memory. > > It's still possible for a user space process to leak memory if the last > process to close the fd for the VM is not the process that created it. > However, this is an unexpected case that's only caused by a user space > process that's misbehaving. > > Signed-off-by: Andrew Honig <ahonig@google.com> Applied, thanks. > --- > arch/x86/kvm/x86.c | 17 +++++++++++++++++ > 1 file changed, 17 insertions(+) > > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c > index 8ffac42..3b389bf 100644 > --- a/arch/x86/kvm/x86.c > +++ b/arch/x86/kvm/x86.c > @@ -6856,6 +6856,23 @@ void kvm_arch_sync_events(struct kvm *kvm) > > void kvm_arch_destroy_vm(struct kvm *kvm) > { > + if (current->mm == kvm->mm) { > + /* > + * Free memory regions allocated on behalf of userspace, > + * unless the the memory map has changed due to process exit > + * or fd copying. > + */ > + struct kvm_userspace_memory_region mem; > + memset(&mem, 0, sizeof(mem)); > + mem.slot = APIC_ACCESS_PAGE_PRIVATE_MEMSLOT; > + kvm_set_memory_region(kvm, &mem); > + > + mem.slot = IDENTITY_PAGETABLE_PRIVATE_MEMSLOT; > + kvm_set_memory_region(kvm, &mem); > + > + mem.slot = TSS_PRIVATE_MEMSLOT; > + kvm_set_memory_region(kvm, &mem); > + } > kvm_iommu_unmap_guest(kvm); > kfree(kvm->arch.vpic); > kfree(kvm->arch.vioapic); > -- > 1.7.10.4 > > -- > 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 -- Gleb. -- 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 --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c index 8ffac42..3b389bf 100644 --- a/arch/x86/kvm/x86.c +++ b/arch/x86/kvm/x86.c @@ -6856,6 +6856,23 @@ void kvm_arch_sync_events(struct kvm *kvm) void kvm_arch_destroy_vm(struct kvm *kvm) { + if (current->mm == kvm->mm) { + /* + * Free memory regions allocated on behalf of userspace, + * unless the the memory map has changed due to process exit + * or fd copying. + */ + struct kvm_userspace_memory_region mem; + memset(&mem, 0, sizeof(mem)); + mem.slot = APIC_ACCESS_PAGE_PRIVATE_MEMSLOT; + kvm_set_memory_region(kvm, &mem); + + mem.slot = IDENTITY_PAGETABLE_PRIVATE_MEMSLOT; + kvm_set_memory_region(kvm, &mem); + + mem.slot = TSS_PRIVATE_MEMSLOT; + kvm_set_memory_region(kvm, &mem); + } kvm_iommu_unmap_guest(kvm); kfree(kvm->arch.vpic); kfree(kvm->arch.vioapic);
If userspace creates and destroys multiple VMs within the same process we leak 20k of memory in the userspace process context per VM. This patch frees the memory in kvm_arch_destroy_vm. If the process exits without closing the VM file descriptor or the file descriptor has been shared with another process then we don't free the memory. It's still possible for a user space process to leak memory if the last process to close the fd for the VM is not the process that created it. However, this is an unexpected case that's only caused by a user space process that's misbehaving. Signed-off-by: Andrew Honig <ahonig@google.com> --- arch/x86/kvm/x86.c | 17 +++++++++++++++++ 1 file changed, 17 insertions(+)