Re: [PATCHv2] KVM: x86: Fix memory leak in vmx.c

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, Apr 17, 2013 at 10:54 AM, Andrew Honig <ahonig@xxxxxxxxxx> 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 need to free the memory.

Technically, I think there's still a (temporary) leak in the case
where the last close happened from the wrong process: f_op->release()
gets called from a context where it won't whack the kvm memory
regions.  However, that's a perverse case not expected in practice --
it will get cleaned up when the original process exits and has it's mm
cleaned up.  Since the one affected (the original open()ing process of
/dev/kvm) is the one the one affected and also the one who misbehaved
(shared its file descriptor), I don't know that it's worth trying to
nail that case down as long as the host kernel isn't compromised (it
won't be).  Perhaps comment it though, at least in the changelog
entry?

>
> Signed-off-by: Andrew Honig <ahonig@xxxxxxxxxx>
> ---
>  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 e172132..e93e16b 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -6811,6 +6811,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, 0);
> +
> +               mem.slot = IDENTITY_PAGETABLE_PRIVATE_MEMSLOT;
> +               kvm_set_memory_region(kvm, &mem, 0);
> +
> +               mem.slot = TSS_PRIVATE_MEMSLOT;
> +               kvm_set_memory_region(kvm, &mem, 0);
> +       }
>         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@xxxxxxxxxxxxxxx
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux