Memory leak in vmx.c

Memory leak in vmx.c

[Andrew Honig]

I've noticed a memory leak that occurs in vmx.c.

In alloc_apic_access_page, it calls __kvm_set_memory_region(kvm,
&kvm_userspace_mem, 0).  __kvm_set_memory_region calls
kvm_arch_prepare_memory_region, which because the user_alloc parameter
is 0 will allocate memory for the page with vm_mmap.

This memory never gets freed.   In kvm_arch_destroy_vm it calls
put_page(kvm->arch.apic_access_page), but that only unpins the memory
(necessary due to an earlier call to gfn_to_page), it never actually
frees the memory.  The memory is allocated in the current process
context so it's cleaned up when the process exits, but if a process
creates and destroys multiple VMs then this leak starts to become a
problem.

Similar leaks occur in alloc_identity_pagetable and vmx_set_tss_addr
for a total of 5 pages of memory leak for a VM.  The vmx_set_tss_addr
actually leaks each time vmx_set_tss_addr is called so this could also
become a problem if a program had occasion to set the tss addr several
times.

thanks,
Andy

Re: Memory leak in vmx.c

[Jan Kiszka]

[-- Attachment #1: Type: text/plain, Size: 1250 bytes --]

On 2012-12-07 20:51, Andrew Honig wrote:
> I've noticed a memory leak that occurs in vmx.c.
> 
> In alloc_apic_access_page, it calls __kvm_set_memory_region(kvm,
> &kvm_userspace_mem, 0).  __kvm_set_memory_region calls
> kvm_arch_prepare_memory_region, which because the user_alloc parameter
> is 0 will allocate memory for the page with vm_mmap.
> 
> This memory never gets freed.   In kvm_arch_destroy_vm it calls
> put_page(kvm->arch.apic_access_page), but that only unpins the memory
> (necessary due to an earlier call to gfn_to_page), it never actually
> frees the memory.  The memory is allocated in the current process
> context so it's cleaned up when the process exits, but if a process
> creates and destroys multiple VMs then this leak starts to become a
> problem.
> 
> Similar leaks occur in alloc_identity_pagetable and vmx_set_tss_addr
> for a total of 5 pages of memory leak for a VM.  The vmx_set_tss_addr
> actually leaks each time vmx_set_tss_addr is called so this could also
> become a problem if a program had occasion to set the tss addr several
> times.

Both pages are per-vm. Therefore they are freed in kvm_arch_destroy_vm.
But I have to admit that I dug a while as well to find this out.

Jan


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 261 bytes --]

Re: Memory leak in vmx.c

[Andrew Honig]

Jan,

Thanks for taking a look at that for me.  It still looks like it leaks
to me.  Could you point me to the actual free?  In
kvm_arch_destroy_vm, it calls put_page on apic_access_page and
ept_identity_pagetable, but that doesn't actually free the memory.  It
unpins it from the kernel, but doesn't free them.  There's also the
tss_addr pages which are not referenced at all in kvm_arch_destroy_vm.

I also wrote a simple program to demonstrate the leak.  This program
creates and destroys VMs with a single CPU and a TSS addr.  On Intel
platforms this program consumes memory without bound (albeit slowly).
 Would you mind taking a look?

thanks,
Andy

#include <asm/kvm.h>
#include <fcntl.h>
#include <linux/kvm.h>
#include <sys/ioctl.h>

int main() {

  int kvm_fd = open("/dev/kvm", O_RDWR);

  if (kvm_fd < 0) printf("Error opening /dev/kvm\n");

  int count = 0;

  while (1) {
    int vm_fd = ioctl(kvm_fd, KVM_CREATE_VM, 0);
    if (vm_fd < 0) printf("Error creating vm\n");
    int vcpu_id = ioctl(vm_fd, KVM_CREATE_VCPU, 1);
    if (vcpu_id < 0) printf("Error creating vcpu\n");
    ioctl(vm_fd, KVM_SET_TSS_ADDR, 0xffc00000);
    close(vcpu_id);
    close(vm_fd);
    printf("Iteration %d\n", ++count);
  }
}




On Sat, Dec 8, 2012 at 12:31 AM, Jan Kiszka <jan.kiszka@web.de> wrote:
> On 2012-12-07 20:51, Andrew Honig wrote:
>> I've noticed a memory leak that occurs in vmx.c.
>>
>> In alloc_apic_access_page, it calls __kvm_set_memory_region(kvm,
>> &kvm_userspace_mem, 0).  __kvm_set_memory_region calls
>> kvm_arch_prepare_memory_region, which because the user_alloc parameter
>> is 0 will allocate memory for the page with vm_mmap.
>>
>> This memory never gets freed.   In kvm_arch_destroy_vm it calls
>> put_page(kvm->arch.apic_access_page), but that only unpins the memory
>> (necessary due to an earlier call to gfn_to_page), it never actually
>> frees the memory.  The memory is allocated in the current process
>> context so it's cleaned up when the process exits, but if a process
>> creates and destroys multiple VMs then this leak starts to become a
>> problem.
>>
>> Similar leaks occur in alloc_identity_pagetable and vmx_set_tss_addr
>> for a total of 5 pages of memory leak for a VM.  The vmx_set_tss_addr
>> actually leaks each time vmx_set_tss_addr is called so this could also
>> become a problem if a program had occasion to set the tss addr several
>> times.
>
> Both pages are per-vm. Therefore they are freed in kvm_arch_destroy_vm.
> But I have to admit that I dug a while as well to find this out.
>
> Jan
>

Re: Memory leak in vmx.c

[Jan Kiszka]

[-- Attachment #1: Type: text/plain, Size: 4366 bytes --]

On 2012-12-10 19:36, Andrew Honig wrote:
> Jan,
> 
> Thanks for taking a look at that for me.  It still looks like it leaks
> to me.  Could you point me to the actual free?  In
> kvm_arch_destroy_vm, it calls put_page on apic_access_page and
> ept_identity_pagetable, but that doesn't actually free the memory.  It
> unpins it from the kernel, but doesn't free them.  There's also the
> tss_addr pages which are not referenced at all in kvm_arch_destroy_vm.
> 
> I also wrote a simple program to demonstrate the leak.  This program
> creates and destroys VMs with a single CPU and a TSS addr.  On Intel
> platforms this program consumes memory without bound (albeit slowly).
>  Would you mind taking a look?

I can confirm that the memory usage increases as long as the program
runs. However, once I terminate it, all is fine again as the last user
of the mapped pages then disappears and the resources are freed.

The reason for the continuous growth of mapped memory is the lack of
releasing the TSS mapping on vm destruction. I'm not seeing an easy way
yet how to resolve this as - apparently - we cannot vm_munmap the pages
on fd release (kvm destruction) when triggered on process exit.

> 
> thanks,
> Andy
> 
> #include <asm/kvm.h>
> #include <fcntl.h>
> #include <linux/kvm.h>
> #include <sys/ioctl.h>
> 
> int main() {
> 
>   int kvm_fd = open("/dev/kvm", O_RDWR);
> 
>   if (kvm_fd < 0) printf("Error opening /dev/kvm\n");
> 
>   int count = 0;
> 
>   while (1) {
>     int vm_fd = ioctl(kvm_fd, KVM_CREATE_VM, 0);
>     if (vm_fd < 0) printf("Error creating vm\n");
>     int vcpu_id = ioctl(vm_fd, KVM_CREATE_VCPU, 1);
>     if (vcpu_id < 0) printf("Error creating vcpu\n");
>     ioctl(vm_fd, KVM_SET_TSS_ADDR, 0xffc00000);
>     close(vcpu_id);
>     close(vm_fd);
>     printf("Iteration %d\n", ++count);
>   }
> }
>

But you test case also triggers a warning of lockdep after a few
thousand cycles. Not sure yet what it means:

[  227.061010] 
[  227.061016] =====================================
[  227.061020] [ BUG: bad unlock balance detected! ]
[  227.061025] 3.7.0-dbg+ #25 Not tainted
[  227.061028] -------------------------------------
[  227.061031] kvm-leak/4624 is trying to release lock (&mapping->i_mmap_mutex) at:
[  227.061043] [<ffffffff8144cd6f>] mutex_unlock+0xe/0x10
[  227.061046] but there are no more locks to release!
[  227.061050] 
[  227.061050] other info that might help us debug this:
[  227.061054] 3 locks held by kvm-leak/4624:
[  227.061057]  #0:  (&mm->mmap_sem){++++++}, at: [<ffffffff8111af29>] do_mmu_notifier_register+0x6d/0x12b
[  227.061070]  #1:  (mm_all_locks_mutex){+.+...}, at: [<ffffffff8110f188>] mm_take_all_locks+0x40/0x168
[  227.061080]  #2:  (&anon_vma->mutex){+.+...}, at: [<ffffffff8110f258>] mm_take_all_locks+0x110/0x168
[  227.061090] 
[  227.061090] stack backtrace:
[  227.061094] Pid: 4624, comm: kvm-leak Not tainted 3.7.0-dbg+ #25
[  227.061098] Call Trace:
[  227.061103]  [<ffffffff8144cd6f>] ? mutex_unlock+0xe/0x10
[  227.061109]  [<ffffffff8107aaf8>] print_unlock_inbalance_bug+0xe9/0xf4
[  227.061115]  [<ffffffff8107f148>] lock_release_non_nested+0xc4/0x293
[  227.061121]  [<ffffffff8110f258>] ? mm_take_all_locks+0x110/0x168
[  227.061126]  [<ffffffff8144c92f>] ? _mutex_lock_nest_lock+0x2c0/0x2e2
[  227.061131]  [<ffffffff8144cd6f>] ? mutex_unlock+0xe/0x10
[  227.061161]  [<ffffffff8144cd6f>] ? mutex_unlock+0xe/0x10
[  227.061167]  [<ffffffff8107f4be>] lock_release+0x1a7/0x1f9
[  227.061172]  [<ffffffff8144ccfd>] __mutex_unlock_slowpath+0xcb/0x12f
[  227.061179]  [<ffffffff8144cd6f>] mutex_unlock+0xe/0x10
[  227.061184]  [<ffffffff8110f114>] mm_drop_all_locks+0xc7/0xfb
[  227.061193]  [<ffffffff8111afb6>] do_mmu_notifier_register+0xfa/0x12b
[  227.061198]  [<ffffffff8111b00c>] mmu_notifier_register+0x13/0x15
[  227.061219]  [<ffffffffa009f6b0>] kvm_dev_ioctl+0x298/0x3ef [kvm]
[  227.061225]  [<ffffffff81134603>] do_vfs_ioctl+0x484/0x4c3
[  227.061232]  [<ffffffff8113d785>] ? fget_light+0x120/0x383
[  227.061238]  [<ffffffff81457085>] ? sysret_check+0x22/0x5d
[  227.061243]  [<ffffffff811346a0>] sys_ioctl+0x5e/0x82
[  227.061249]  [<ffffffff8127708e>] ? trace_hardirqs_on_thunk+0x3a/0x3f
[  227.061255]  [<ffffffff81457059>] system_call_fastpath+0x16/0x1b

Jan



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 261 bytes --]