Re: kvm/arm64: use-after-free in kvm_unmap_hva_handler/unmap_stage2_pmds

[Suzuki K Poulose]

On Thu, Apr 13, 2017 at 04:50:46PM +0100, Suzuki K. Poulose wrote:
> On Thu, Apr 13, 2017 at 10:17:54AM +0100, Suzuki K Poulose wrote:
> > On 12/04/17 19:43, Marc Zyngier wrote:
> > > On 12/04/17 17:19, Andrey Konovalov wrote:
> > >
> > > Hi Andrey,
> > >
> > > > Apparently this wasn't fixed, I've got this report again on
> > > > linux-next-c4e7b35a3 (Apr 11), which includes 8b3405e34 "kvm:
> > > > arm/arm64: Fix locking for kvm_free_stage2_pgd".
> > >
> > > This looks like a different bug.
> > >
> > > >
> > > > I now have a way to reproduce it, so I can test proposed patches. I
> > > > don't have a simple C reproducer though.
> > > >
> > > > The bug happens when the following syzkaller program is executed:
> > > >
> > > > mmap(&(0x7f00/0xc000)=nil, (0xc000), 0x3, 0x32, 
> > > > 0x, 0x0)
> > > > unshare(0x400)
> > > > perf_event_open(&(0x7f02f000-0x78)={0x1, 0x78, 0x0, 0x0, 0x0, 0x0,
> > > > 0x0, 0x6, 0x0, 0x0, 0xd34, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0,
> > > > 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0}, 0x0, 0x,
> > > > 0x, 0x0)
> > > > r0 = openat$kvm(0xff9c,
> > > > &(0x7f00c000-0x9)="2f6465762f6b766d00", 0x0, 0x0)
> > > > ioctl$TIOCSBRK(0x, 0x5427)
> > > > r1 = ioctl$KVM_CREATE_VM(r0, 0xae01, 0x0)
> > > > syz_kvm_setup_cpu$arm64(r1, 0x,
> > > > &(0x7fdc6000/0x18000)=nil, &(0x7f00c000)=[{0x0,
> > > > &(0x7f00c000)="5ba3c16f533efbed09f8221253c73763327fadce2371813b45dd7f7982f84a873e4ae89a6c2bd1af83a6024c36a1ff518318",
> > > > 0x32}], 0x1, 0x0, &(0x7f00d000-0x10)=[@featur2={0x1, 0x3}], 0x1)
> > >
> > > Is that the only thing the program does? Or is there anything running in
> > > parallel?
> > >
> > > > ==
> > > > BUG: KASAN: use-after-free in arch_spin_is_locked
> > > > include/linux/compiler.h:254 [inline]
> > > > BUG: KASAN: use-after-free in unmap_stage2_range+0x990/0x9a8
> > > > arch/arm64/kvm/../../../arch/arm/kvm/mmu.c:295
> > > > Read of size 8 at addr 84476730 by task syz-executor/13106
> > > >
> > > > CPU: 1 PID: 13106 Comm: syz-executor Not tainted
> > > > 4.11.0-rc6-next-20170411-xc2-11025-gc4e7b35a33d4-dirty #5
> > > > Hardware name: Hardkernel ODROID-C2 (DT)
> > > > Call trace:
> > > > [] dump_backtrace+0x0/0x440 
> > > > arch/arm64/kernel/traps.c:505
> > > > [] show_stack+0x20/0x30 arch/arm64/kernel/traps.c:228
> > > > [] __dump_stack lib/dump_stack.c:16 [inline]
> > > > [] dump_stack+0x110/0x168 lib/dump_stack.c:52
> > > > [] print_address_description+0x60/0x248 
> > > > mm/kasan/report.c:252
> > > > [] kasan_report_error mm/kasan/report.c:351 [inline]
> > > > [] kasan_report+0x218/0x300 mm/kasan/report.c:408
> > > > [] __asan_report_load8_noabort+0x18/0x20 
> > > > mm/kasan/report.c:429
> > > > [] arch_spin_is_locked include/linux/compiler.h:254 
> > > > [inline]
> > >
> > > This is the assert on the spinlock, and the memory is gone.
> > >
> > > > [] unmap_stage2_range+0x990/0x9a8
> > > > arch/arm64/kvm/../../../arch/arm/kvm/mmu.c:295
> > > > [] kvm_free_stage2_pgd.part.16+0x30/0x98
> > > > arch/arm64/kvm/../../../arch/arm/kvm/mmu.c:842
> > > > [] kvm_free_stage2_pgd
> > > > arch/arm64/kvm/../../../arch/arm/kvm/mmu.c:838 [inline]
> > >
> > > But we've taken than lock here. There's only a handful of instructions
> > > in between, and the memory can only go away if there is something
> > > messing with us in parallel.
> > >
> > > > [] kvm_arch_flush_shadow_all+0x40/0x58
> > > > arch/arm64/kvm/../../../arch/arm/kvm/mmu.c:1895
> > > > [] kvm_mmu_notifier_release+0x154/0x1d0
> > > > arch/arm64/kvm/../../../virt/kvm/kvm_main.c:472
> > > > [] __mmu_notifier_release+0x1c0/0x3e0 
> > > > mm/mmu_notifier.c:75
> > > > [] mmu_notifier_release
> > > > include/linux/mmu_notifier.h:235 [inline]
> > > > [] exit_mmap+0x21c/0x288 mm/mmap.c:2941
> > > > [] __mmput kernel/fork.c:888 [inline]
> > > > [] mmput+0xdc/0x2e0 kernel/fork.c:910
> > > > [] exit_mm kernel/exit.c:557 [inline]
> > > > [] do_exit+0x648/0x2020 kernel/exit.c:865
> > > > [] do_group_exit+0xdc/0x260 kernel/exit.c:982
> > > > [] get_signal+0x358/0xf58 kernel/signal.c:2318
> > > > [] do_signal+0x170/0xc10 
> > > > arch/arm64/kernel/signal.c:370
> > > > [] do_notify_resume+0xe4/0x120 
> > > > arch/arm64/kernel/signal.c:421
> > > > [] work_pending+0x8/0x14
> > >
> > > So we're being serviced with a signal. Do you know if this signal is
> > > generated by your syzkaller program? We could be racing between do_exit
> > > triggered by a fatal signal (this trace) and the closing of the two file
> > > descriptors (vcpu and vm).
> > >
> > > Paolo: does this look possible to you? I can't see what locking we have
> > > that could prevent this race.
> >
> > On a quick look, I see two issues:
> >
> > 1) It looks like the mmu_notifier->ops.release could be called twice for a 
> > notifier,
> > from mmu_notifier_unregister() and exit_mmap()->mmu_notifier_release(), 
> > 

Re: kvm/arm64: use-after-free in kvm_unmap_hva_handler/unmap_stage2_pmds

[Suzuki K Poulose]

On 18/04/17 10:08, Mark Rutland wrote:

On Tue, Apr 18, 2017 at 09:32:31AM +0100, Mark Rutland wrote:

Hi Suzuki,

On Thu, Apr 13, 2017 at 04:50:46PM +0100, Suzuki K. Poulose wrote:

kvm: Hold reference to the user address space

The core KVM code, uses mmgrab/mmdrop to pin the mm struct of the user
application. mmgrab only guarantees that the mm struct is available,
while the "real address space" (see Documentation/vm/active_mm.txt) may
be destroyed. Since the KVM depends on the user space page tables for
the Guest pages, we should instead do an mmget/mmput. Even though
mmget/mmput is not encouraged for uses with unbounded time, the KVM
is fine to do so, as we are doing it from the context of the same process.

This also prevents the race condition where mmu_notifier_release() could
be called in parallel and one instance could end up using a free'd kvm
instance.

Cc: Mark Rutland 
Cc: Paolo Bonzin 
Cc: Radim Krčmář 
Cc: Marc Zyngier 
Cc: Christoffer Dall 
Cc: andreyk...@google.com
Signed-off-by: Suzuki K Poulose 
---
 virt/kvm/kvm_main.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index 88257b3..555712e 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -613,7 +613,7 @@ static struct kvm *kvm_create_vm(unsigned long type)
return ERR_PTR(-ENOMEM);

spin_lock_init(>mmu_lock);
-   mmgrab(current->mm);
+   mmget(current->mm);
kvm->mm = current->mm;
kvm_eventfd_init(kvm);
mutex_init(>lock);
@@ -685,7 +685,7 @@ static struct kvm *kvm_create_vm(unsigned long type)
for (i = 0; i < KVM_ADDRESS_SPACE_NUM; i++)
kvm_free_memslots(kvm, kvm->memslots[i]);
kvm_arch_free_vm(kvm);
-   mmdrop(current->mm);
+   mmput(current->mm);
return ERR_PTR(r);
 }

@@ -747,7 +747,7 @@ static void kvm_destroy_vm(struct kvm *kvm)
kvm_arch_free_vm(kvm);
preempt_notifier_dec();
hardware_disable_all();
-   mmdrop(mm);
+   mmput(mm);
 }



As a heads-up, I'm seeing what looks to be a KVM memory leak with this
patch applied atop of next-20170411.

I don't yet know if this is a problem with next-20170411 or this patch
in particular -- I will try to track that down. In the mean time, info
dump below.


This is indeed a side effect of the new patch. The VCPU doesn't get released
completely, due to an mmap count held on the VCPU fd, even when we close the
VCPU fd. This keeps the refcount on the KVM instance which in turn holds the
mmap count (with the new patch). So the mmap count on VCPU will never get
released due to the circular dependency here. :-(

Suzuki
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Re: kvm/arm64: use-after-free in kvm_unmap_hva_handler/unmap_stage2_pmds

[Mark Rutland]

On Tue, Apr 18, 2017 at 09:32:31AM +0100, Mark Rutland wrote:
> Hi Suzuki,
> 
> On Thu, Apr 13, 2017 at 04:50:46PM +0100, Suzuki K. Poulose wrote:
> > kvm: Hold reference to the user address space
> > 
> > The core KVM code, uses mmgrab/mmdrop to pin the mm struct of the user
> > application. mmgrab only guarantees that the mm struct is available,
> > while the "real address space" (see Documentation/vm/active_mm.txt) may
> > be destroyed. Since the KVM depends on the user space page tables for
> > the Guest pages, we should instead do an mmget/mmput. Even though
> > mmget/mmput is not encouraged for uses with unbounded time, the KVM
> > is fine to do so, as we are doing it from the context of the same process.
> > 
> > This also prevents the race condition where mmu_notifier_release() could
> > be called in parallel and one instance could end up using a free'd kvm
> > instance.
> > 
> > Cc: Mark Rutland 
> > Cc: Paolo Bonzin 
> > Cc: Radim Krčmář 
> > Cc: Marc Zyngier 
> > Cc: Christoffer Dall 
> > Cc: andreyk...@google.com
> > Signed-off-by: Suzuki K Poulose 
> > ---
> >  virt/kvm/kvm_main.c | 6 +++---
> >  1 file changed, 3 insertions(+), 3 deletions(-)
> > 
> > diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
> > index 88257b3..555712e 100644
> > --- a/virt/kvm/kvm_main.c
> > +++ b/virt/kvm/kvm_main.c
> > @@ -613,7 +613,7 @@ static struct kvm *kvm_create_vm(unsigned long type)
> > return ERR_PTR(-ENOMEM);
> >  
> > spin_lock_init(>mmu_lock);
> > -   mmgrab(current->mm);
> > +   mmget(current->mm);
> > kvm->mm = current->mm;
> > kvm_eventfd_init(kvm);
> > mutex_init(>lock);
> > @@ -685,7 +685,7 @@ static struct kvm *kvm_create_vm(unsigned long type)
> > for (i = 0; i < KVM_ADDRESS_SPACE_NUM; i++)
> > kvm_free_memslots(kvm, kvm->memslots[i]);
> > kvm_arch_free_vm(kvm);
> > -   mmdrop(current->mm);
> > +   mmput(current->mm);
> > return ERR_PTR(r);
> >  }
> >  
> > @@ -747,7 +747,7 @@ static void kvm_destroy_vm(struct kvm *kvm)
> > kvm_arch_free_vm(kvm);
> > preempt_notifier_dec();
> > hardware_disable_all();
> > -   mmdrop(mm);
> > +   mmput(mm);
> >  }
> 
> 
> As a heads-up, I'm seeing what looks to be a KVM memory leak with this
> patch applied atop of next-20170411.
> 
> I don't yet know if this is a problem with next-20170411 or this patch
> in particular -- I will try to track that down. In the mean time, info
> dump below.
> 
> I left syzkaller running over the weekend using this kernel on the host,
> and OOM kicked in after it had been running for a short while. Almost
> all of my memory is in use, but judging by top, almost none of this is
> associated with processes.

FWIW, it seems easy enough to trigger this leak with kvmtool. Start a
guest that'll allocate a tonne of memory:

$ lkvm run --console virtio -m 4096 --kernel Image \
  -p "memtest=1 console=hvc0,38400 earlycon=uart,mmio,0x3f8"

... then kill this with a SIGKILL from your favourite process management
tool.

Also, if the guest exits normally, everything appears to be cleaned up
correctly. So it looks like this is in some way related to our fatal
signal handling.

Thanks,
Mark.
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Re: kvm/arm64: use-after-free in kvm_unmap_hva_handler/unmap_stage2_pmds

[Mark Rutland]

Hi Suzuki,

On Thu, Apr 13, 2017 at 04:50:46PM +0100, Suzuki K. Poulose wrote:
> kvm: Hold reference to the user address space
> 
> The core KVM code, uses mmgrab/mmdrop to pin the mm struct of the user
> application. mmgrab only guarantees that the mm struct is available,
> while the "real address space" (see Documentation/vm/active_mm.txt) may
> be destroyed. Since the KVM depends on the user space page tables for
> the Guest pages, we should instead do an mmget/mmput. Even though
> mmget/mmput is not encouraged for uses with unbounded time, the KVM
> is fine to do so, as we are doing it from the context of the same process.
> 
> This also prevents the race condition where mmu_notifier_release() could
> be called in parallel and one instance could end up using a free'd kvm
> instance.
> 
> Cc: Mark Rutland 
> Cc: Paolo Bonzin 
> Cc: Radim Krčmář 
> Cc: Marc Zyngier 
> Cc: Christoffer Dall 
> Cc: andreyk...@google.com
> Signed-off-by: Suzuki K Poulose 
> ---
>  virt/kvm/kvm_main.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
> index 88257b3..555712e 100644
> --- a/virt/kvm/kvm_main.c
> +++ b/virt/kvm/kvm_main.c
> @@ -613,7 +613,7 @@ static struct kvm *kvm_create_vm(unsigned long type)
>   return ERR_PTR(-ENOMEM);
>  
>   spin_lock_init(>mmu_lock);
> - mmgrab(current->mm);
> + mmget(current->mm);
>   kvm->mm = current->mm;
>   kvm_eventfd_init(kvm);
>   mutex_init(>lock);
> @@ -685,7 +685,7 @@ static struct kvm *kvm_create_vm(unsigned long type)
>   for (i = 0; i < KVM_ADDRESS_SPACE_NUM; i++)
>   kvm_free_memslots(kvm, kvm->memslots[i]);
>   kvm_arch_free_vm(kvm);
> - mmdrop(current->mm);
> + mmput(current->mm);
>   return ERR_PTR(r);
>  }
>  
> @@ -747,7 +747,7 @@ static void kvm_destroy_vm(struct kvm *kvm)
>   kvm_arch_free_vm(kvm);
>   preempt_notifier_dec();
>   hardware_disable_all();
> - mmdrop(mm);
> + mmput(mm);
>  }


As a heads-up, I'm seeing what looks to be a KVM memory leak with this
patch applied atop of next-20170411.

I don't yet know if this is a problem with next-20170411 or this patch
in particular -- I will try to track that down. In the mean time, info
dump below.

I left syzkaller running over the weekend using this kernel on the host,
and OOM kicked in after it had been running for a short while. Almost
all of my memory is in use, but judging by top, almost none of this is
associated with processes.

It looks like this is almost all AnonPages allocations:

nanook@medister:~$ cat /proc/meminfo 
MemTotal:   14258176 kB
MemFree:  106192 kB
MemAvailable:  38196 kB
Buffers:   27160 kB
Cached:42508 kB
SwapCached:0 kB
Active: 13442912 kB
Inactive:   7388 kB
Active(anon):   13380876 kB
Inactive(anon):  400 kB
Active(file):  62036 kB
Inactive(file): 6988 kB
Unevictable:   0 kB
Mlocked:   0 kB
SwapTotal: 0 kB
SwapFree:  0 kB
Dirty: 0 kB
Writeback: 0 kB
AnonPages:  13380688 kB
Mapped: 7352 kB
Shmem:   620 kB
Slab: 568196 kB
SReclaimable:  21756 kB
SUnreclaim:   546440 kB
KernelStack:2832 kB
PageTables:49168 kB
NFS_Unstable:  0 kB
Bounce:0 kB
WritebackTmp:  0 kB
CommitLimit: 7129088 kB
Committed_AS:   41554652 kB
VmallocTotal:   100930551744 kB
VmallocUsed:   0 kB
VmallocChunk:  0 kB
AnonHugePages:  12728320 kB
ShmemHugePages:0 kB
ShmemPmdMapped:0 kB
CmaTotal:  16384 kB
CmaFree:   0 kB
HugePages_Total:   0
HugePages_Free:0
HugePages_Rsvd:0
HugePages_Surp:0
Hugepagesize:   2048 kB

Looking at slabtop, there are large number of vm_area_structs around:

 Active / Total Objects (% used): 531511 / 587214 (90.5%)
 Active / Total Slabs (% used)  : 29443 / 29443 (100.0%)
 Active / Total Caches (% used) : 108 / 156 (69.2%)
 Active / Total Size (% used)   : 514052.23K / 536839.57K (95.8%)
 Minimum / Average / Maximum Object : 0.03K / 0.91K / 8.28K

  OBJS ACTIVE  USE OBJ SIZE  SLABS OBJ/SLAB CACHE SIZE NAME   
 94924  89757  94%0.24K   2877   33 23016K vm_area_struct
 72400  60687  83%0.31K   2896   25 23168K filp
 70553  70484  99%4.25K  100797322528K names_cache
 70112  64605  92%0.25K   2191   32 17528K kmalloc-128
 52458  50837  96%0.09K   1249   42  4996K anon_vma_chain
 23492  22949  97%4.25K   33567107392K kmalloc-4096
 20631  20631 100%0.10K529   39  2116K anon_vma

... so it looks like we could be leaking the mm and associated mappings.

Full OOM 

Re: kvm/arm64: use-after-free in kvm_unmap_hva_handler/unmap_stage2_pmds

[Andrey Konovalov]

On Thu, Apr 13, 2017 at 5:50 PM, Suzuki K. Poulose
 wrote:
> On Thu, Apr 13, 2017 at 10:17:54AM +0100, Suzuki K Poulose wrote:
>> On 12/04/17 19:43, Marc Zyngier wrote:
>> > On 12/04/17 17:19, Andrey Konovalov wrote:
>> >
>> > Hi Andrey,
>> >
>> > > Apparently this wasn't fixed, I've got this report again on
>> > > linux-next-c4e7b35a3 (Apr 11), which includes 8b3405e34 "kvm:
>> > > arm/arm64: Fix locking for kvm_free_stage2_pgd".
>> >
>> > This looks like a different bug.
>> >
>> > >
>> > > I now have a way to reproduce it, so I can test proposed patches. I
>> > > don't have a simple C reproducer though.
>> > >
>> > > The bug happens when the following syzkaller program is executed:
>> > >
>> > > mmap(&(0x7f00/0xc000)=nil, (0xc000), 0x3, 0x32, 
>> > > 0x, 0x0)
>> > > unshare(0x400)
>> > > perf_event_open(&(0x7f02f000-0x78)={0x1, 0x78, 0x0, 0x0, 0x0, 0x0,
>> > > 0x0, 0x6, 0x0, 0x0, 0xd34, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0,
>> > > 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0}, 0x0, 0x,
>> > > 0x, 0x0)
>> > > r0 = openat$kvm(0xff9c,
>> > > &(0x7f00c000-0x9)="2f6465762f6b766d00", 0x0, 0x0)
>> > > ioctl$TIOCSBRK(0x, 0x5427)
>> > > r1 = ioctl$KVM_CREATE_VM(r0, 0xae01, 0x0)
>> > > syz_kvm_setup_cpu$arm64(r1, 0x,
>> > > &(0x7fdc6000/0x18000)=nil, &(0x7f00c000)=[{0x0,
>> > > &(0x7f00c000)="5ba3c16f533efbed09f8221253c73763327fadce2371813b45dd7f7982f84a873e4ae89a6c2bd1af83a6024c36a1ff518318",
>> > > 0x32}], 0x1, 0x0, &(0x7f00d000-0x10)=[@featur2={0x1, 0x3}], 0x1)
>> >
>> > Is that the only thing the program does? Or is there anything running in
>> > parallel?
>> >
>> > > ==
>> > > BUG: KASAN: use-after-free in arch_spin_is_locked
>> > > include/linux/compiler.h:254 [inline]
>> > > BUG: KASAN: use-after-free in unmap_stage2_range+0x990/0x9a8
>> > > arch/arm64/kvm/../../../arch/arm/kvm/mmu.c:295
>> > > Read of size 8 at addr 84476730 by task syz-executor/13106
>> > >
>> > > CPU: 1 PID: 13106 Comm: syz-executor Not tainted
>> > > 4.11.0-rc6-next-20170411-xc2-11025-gc4e7b35a33d4-dirty #5
>> > > Hardware name: Hardkernel ODROID-C2 (DT)
>> > > Call trace:
>> > > [] dump_backtrace+0x0/0x440 
>> > > arch/arm64/kernel/traps.c:505
>> > > [] show_stack+0x20/0x30 arch/arm64/kernel/traps.c:228
>> > > [] __dump_stack lib/dump_stack.c:16 [inline]
>> > > [] dump_stack+0x110/0x168 lib/dump_stack.c:52
>> > > [] print_address_description+0x60/0x248 
>> > > mm/kasan/report.c:252
>> > > [] kasan_report_error mm/kasan/report.c:351 [inline]
>> > > [] kasan_report+0x218/0x300 mm/kasan/report.c:408
>> > > [] __asan_report_load8_noabort+0x18/0x20 
>> > > mm/kasan/report.c:429
>> > > [] arch_spin_is_locked include/linux/compiler.h:254 
>> > > [inline]
>> >
>> > This is the assert on the spinlock, and the memory is gone.
>> >
>> > > [] unmap_stage2_range+0x990/0x9a8
>> > > arch/arm64/kvm/../../../arch/arm/kvm/mmu.c:295
>> > > [] kvm_free_stage2_pgd.part.16+0x30/0x98
>> > > arch/arm64/kvm/../../../arch/arm/kvm/mmu.c:842
>> > > [] kvm_free_stage2_pgd
>> > > arch/arm64/kvm/../../../arch/arm/kvm/mmu.c:838 [inline]
>> >
>> > But we've taken than lock here. There's only a handful of instructions
>> > in between, and the memory can only go away if there is something
>> > messing with us in parallel.
>> >
>> > > [] kvm_arch_flush_shadow_all+0x40/0x58
>> > > arch/arm64/kvm/../../../arch/arm/kvm/mmu.c:1895
>> > > [] kvm_mmu_notifier_release+0x154/0x1d0
>> > > arch/arm64/kvm/../../../virt/kvm/kvm_main.c:472
>> > > [] __mmu_notifier_release+0x1c0/0x3e0 
>> > > mm/mmu_notifier.c:75
>> > > [] mmu_notifier_release
>> > > include/linux/mmu_notifier.h:235 [inline]
>> > > [] exit_mmap+0x21c/0x288 mm/mmap.c:2941
>> > > [] __mmput kernel/fork.c:888 [inline]
>> > > [] mmput+0xdc/0x2e0 kernel/fork.c:910
>> > > [] exit_mm kernel/exit.c:557 [inline]
>> > > [] do_exit+0x648/0x2020 kernel/exit.c:865
>> > > [] do_group_exit+0xdc/0x260 kernel/exit.c:982
>> > > [] get_signal+0x358/0xf58 kernel/signal.c:2318
>> > > [] do_signal+0x170/0xc10 arch/arm64/kernel/signal.c:370
>> > > [] do_notify_resume+0xe4/0x120 
>> > > arch/arm64/kernel/signal.c:421
>> > > [] work_pending+0x8/0x14
>> >
>> > So we're being serviced with a signal. Do you know if this signal is
>> > generated by your syzkaller program? We could be racing between do_exit
>> > triggered by a fatal signal (this trace) and the closing of the two file
>> > descriptors (vcpu and vm).
>> >
>> > Paolo: does this look possible to you? I can't see what locking we have
>> > that could prevent this race.
>>
>> On a quick look, I see two issues:
>>
>> 1) It looks like the mmu_notifier->ops.release could be called twice for a 
>> notifier,
>> from mmu_notifier_unregister() and exit_mmap()->mmu_notifier_release(), 
>> which is
>> causing the problem as above.
>>
>> This could possibly be avoided by swapping the order of 

Re: kvm/arm64: use-after-free in kvm_unmap_hva_handler/unmap_stage2_pmds

[Suzuki K Poulose]

On 13/04/17 16:50, Suzuki K. Poulose wrote:

On Thu, Apr 13, 2017 at 10:17:54AM +0100, Suzuki K Poulose wrote:

On 12/04/17 19:43, Marc Zyngier wrote:

On 12/04/17 17:19, Andrey Konovalov wrote:


Please ignore the footer below, that was mistake from my side.


--
2.7.4
IMPORTANT NOTICE: The contents of this email and any attachments are 
confidential and may also be privileged. If you are not the intended recipient, 
please notify the sender immediately and do not disclose the contents to any 
other person, use it for any purpose, or store or copy the information in any 
medium. Thank you.



___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Re: kvm/arm64: use-after-free in kvm_unmap_hva_handler/unmap_stage2_pmds

[Suzuki K. Poulose]

On Thu, Apr 13, 2017 at 10:17:54AM +0100, Suzuki K Poulose wrote:
> On 12/04/17 19:43, Marc Zyngier wrote:
> > On 12/04/17 17:19, Andrey Konovalov wrote:
> >
> > Hi Andrey,
> >
> > > Apparently this wasn't fixed, I've got this report again on
> > > linux-next-c4e7b35a3 (Apr 11), which includes 8b3405e34 "kvm:
> > > arm/arm64: Fix locking for kvm_free_stage2_pgd".
> >
> > This looks like a different bug.
> >
> > >
> > > I now have a way to reproduce it, so I can test proposed patches. I
> > > don't have a simple C reproducer though.
> > >
> > > The bug happens when the following syzkaller program is executed:
> > >
> > > mmap(&(0x7f00/0xc000)=nil, (0xc000), 0x3, 0x32, 
> > > 0x, 0x0)
> > > unshare(0x400)
> > > perf_event_open(&(0x7f02f000-0x78)={0x1, 0x78, 0x0, 0x0, 0x0, 0x0,
> > > 0x0, 0x6, 0x0, 0x0, 0xd34, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0,
> > > 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0}, 0x0, 0x,
> > > 0x, 0x0)
> > > r0 = openat$kvm(0xff9c,
> > > &(0x7f00c000-0x9)="2f6465762f6b766d00", 0x0, 0x0)
> > > ioctl$TIOCSBRK(0x, 0x5427)
> > > r1 = ioctl$KVM_CREATE_VM(r0, 0xae01, 0x0)
> > > syz_kvm_setup_cpu$arm64(r1, 0x,
> > > &(0x7fdc6000/0x18000)=nil, &(0x7f00c000)=[{0x0,
> > > &(0x7f00c000)="5ba3c16f533efbed09f8221253c73763327fadce2371813b45dd7f7982f84a873e4ae89a6c2bd1af83a6024c36a1ff518318",
> > > 0x32}], 0x1, 0x0, &(0x7f00d000-0x10)=[@featur2={0x1, 0x3}], 0x1)
> >
> > Is that the only thing the program does? Or is there anything running in
> > parallel?
> >
> > > ==
> > > BUG: KASAN: use-after-free in arch_spin_is_locked
> > > include/linux/compiler.h:254 [inline]
> > > BUG: KASAN: use-after-free in unmap_stage2_range+0x990/0x9a8
> > > arch/arm64/kvm/../../../arch/arm/kvm/mmu.c:295
> > > Read of size 8 at addr 84476730 by task syz-executor/13106
> > >
> > > CPU: 1 PID: 13106 Comm: syz-executor Not tainted
> > > 4.11.0-rc6-next-20170411-xc2-11025-gc4e7b35a33d4-dirty #5
> > > Hardware name: Hardkernel ODROID-C2 (DT)
> > > Call trace:
> > > [] dump_backtrace+0x0/0x440 
> > > arch/arm64/kernel/traps.c:505
> > > [] show_stack+0x20/0x30 arch/arm64/kernel/traps.c:228
> > > [] __dump_stack lib/dump_stack.c:16 [inline]
> > > [] dump_stack+0x110/0x168 lib/dump_stack.c:52
> > > [] print_address_description+0x60/0x248 
> > > mm/kasan/report.c:252
> > > [] kasan_report_error mm/kasan/report.c:351 [inline]
> > > [] kasan_report+0x218/0x300 mm/kasan/report.c:408
> > > [] __asan_report_load8_noabort+0x18/0x20 
> > > mm/kasan/report.c:429
> > > [] arch_spin_is_locked include/linux/compiler.h:254 
> > > [inline]
> >
> > This is the assert on the spinlock, and the memory is gone.
> >
> > > [] unmap_stage2_range+0x990/0x9a8
> > > arch/arm64/kvm/../../../arch/arm/kvm/mmu.c:295
> > > [] kvm_free_stage2_pgd.part.16+0x30/0x98
> > > arch/arm64/kvm/../../../arch/arm/kvm/mmu.c:842
> > > [] kvm_free_stage2_pgd
> > > arch/arm64/kvm/../../../arch/arm/kvm/mmu.c:838 [inline]
> >
> > But we've taken than lock here. There's only a handful of instructions
> > in between, and the memory can only go away if there is something
> > messing with us in parallel.
> >
> > > [] kvm_arch_flush_shadow_all+0x40/0x58
> > > arch/arm64/kvm/../../../arch/arm/kvm/mmu.c:1895
> > > [] kvm_mmu_notifier_release+0x154/0x1d0
> > > arch/arm64/kvm/../../../virt/kvm/kvm_main.c:472
> > > [] __mmu_notifier_release+0x1c0/0x3e0 
> > > mm/mmu_notifier.c:75
> > > [] mmu_notifier_release
> > > include/linux/mmu_notifier.h:235 [inline]
> > > [] exit_mmap+0x21c/0x288 mm/mmap.c:2941
> > > [] __mmput kernel/fork.c:888 [inline]
> > > [] mmput+0xdc/0x2e0 kernel/fork.c:910
> > > [] exit_mm kernel/exit.c:557 [inline]
> > > [] do_exit+0x648/0x2020 kernel/exit.c:865
> > > [] do_group_exit+0xdc/0x260 kernel/exit.c:982
> > > [] get_signal+0x358/0xf58 kernel/signal.c:2318
> > > [] do_signal+0x170/0xc10 arch/arm64/kernel/signal.c:370
> > > [] do_notify_resume+0xe4/0x120 
> > > arch/arm64/kernel/signal.c:421
> > > [] work_pending+0x8/0x14
> >
> > So we're being serviced with a signal. Do you know if this signal is
> > generated by your syzkaller program? We could be racing between do_exit
> > triggered by a fatal signal (this trace) and the closing of the two file
> > descriptors (vcpu and vm).
> >
> > Paolo: does this look possible to you? I can't see what locking we have
> > that could prevent this race.
>
> On a quick look, I see two issues:
>
> 1) It looks like the mmu_notifier->ops.release could be called twice for a 
> notifier,
> from mmu_notifier_unregister() and exit_mmap()->mmu_notifier_release(), which 
> is
> causing the problem as above.
>
> This could possibly be avoided by swapping the order of the following 
> operations
> in themmu_notifier_unregister():
>
>  a) Invoke ops->release under src_read_lock()
>  b) Delete the notifier from the list.
>
> which can prevent 

Re: kvm/arm64: use-after-free in kvm_unmap_hva_handler/unmap_stage2_pmds

[Andrey Konovalov]

On Thu, Apr 13, 2017 at 11:34 AM, Mark Rutland  wrote:
> On Wed, Apr 12, 2017 at 08:51:31PM +0200, Andrey Konovalov wrote:
>> On Wed, Apr 12, 2017 at 8:43 PM, Marc Zyngier  wrote:
>> > On 12/04/17 17:19, Andrey Konovalov wrote:
>
>> >> I now have a way to reproduce it, so I can test proposed patches. I
>> >> don't have a simple C reproducer though.
>> >>
>> >> The bug happens when the following syzkaller program is executed:
>> >>
>> >> mmap(&(0x7f00/0xc000)=nil, (0xc000), 0x3, 0x32, 
>> >> 0x, 0x0)
>> >> unshare(0x400)
>> >> perf_event_open(&(0x7f02f000-0x78)={0x1, 0x78, 0x0, 0x0, 0x0, 0x0,
>> >> 0x0, 0x6, 0x0, 0x0, 0xd34, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0,
>> >> 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0}, 0x0, 0x,
>> >> 0x, 0x0)
>> >> r0 = openat$kvm(0xff9c,
>> >> &(0x7f00c000-0x9)="2f6465762f6b766d00", 0x0, 0x0)
>> >> ioctl$TIOCSBRK(0x, 0x5427)
>> >> r1 = ioctl$KVM_CREATE_VM(r0, 0xae01, 0x0)
>> >> syz_kvm_setup_cpu$arm64(r1, 0x,
>> >> &(0x7fdc6000/0x18000)=nil, &(0x7f00c000)=[{0x0,
>> >> &(0x7f00c000)="5ba3c16f533efbed09f8221253c73763327fadce2371813b45dd7f7982f84a873e4ae89a6c2bd1af83a6024c36a1ff518318",
>> >> 0x32}], 0x1, 0x0, &(0x7f00d000-0x10)=[@featur2={0x1, 0x3}], 0x1)
>> >
>> > Is that the only thing the program does? Or is there anything running in
>> > parallel?
>>
>> These calls are executed repeatedly and in random order. That's all.
>>
>> Except that I'm running the reproducer on a real arm board, so there's
>> probably a bunch of stuff going on besides these calls.
>
> I had a go at reproducing this on an arm64 board following [1], but so
> far I've had no luck. I've dumped the above into syz-kvm-bug, and I'm
> trying to reproduce the issue with:
>
> PATH=$PATH:$(pwd)/bin syz-execprog \
> -executor $(pwd)/bin/syz-executor \
> -cover=0 -repeat=0 -procs 16 \
> syz-kvm-bug
>
> Just to check, is that the correct way to reproduce the problem with the
> above log?

Hi Mark,

You assume that you have KASAN enabled.

Since a few unintended line breaks were added in the email, here's the
program in plaintext:
https://gist.githubusercontent.com/xairy/69864355b5a64f74e7cb445b7325a7df/raw/bdbbbf177dbea13eac0a5ddfa9c3e6c32695b13b/kvm-arm-uaf-log

I run it as:
# ./syz-execprog -repeat=0 -collide=false -sandbox=namespace ./kvm-arm-uaf-log

And it takes less than a second to trigger the bug.

Thanks!

>
> How quickly does that reproduce the problem for you?
>
> Thanks,
> Mark.
>
> [1] https://github.com/google/syzkaller/wiki/How-to-execute-syzkaller-programs
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Re: kvm/arm64: use-after-free in kvm_unmap_hva_handler/unmap_stage2_pmds

[Mark Rutland]

On Wed, Apr 12, 2017 at 08:51:31PM +0200, Andrey Konovalov wrote:
> On Wed, Apr 12, 2017 at 8:43 PM, Marc Zyngier  wrote:
> > On 12/04/17 17:19, Andrey Konovalov wrote:

> >> I now have a way to reproduce it, so I can test proposed patches. I
> >> don't have a simple C reproducer though.
> >>
> >> The bug happens when the following syzkaller program is executed:
> >>
> >> mmap(&(0x7f00/0xc000)=nil, (0xc000), 0x3, 0x32, 
> >> 0x, 0x0)
> >> unshare(0x400)
> >> perf_event_open(&(0x7f02f000-0x78)={0x1, 0x78, 0x0, 0x0, 0x0, 0x0,
> >> 0x0, 0x6, 0x0, 0x0, 0xd34, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0,
> >> 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0}, 0x0, 0x,
> >> 0x, 0x0)
> >> r0 = openat$kvm(0xff9c,
> >> &(0x7f00c000-0x9)="2f6465762f6b766d00", 0x0, 0x0)
> >> ioctl$TIOCSBRK(0x, 0x5427)
> >> r1 = ioctl$KVM_CREATE_VM(r0, 0xae01, 0x0)
> >> syz_kvm_setup_cpu$arm64(r1, 0x,
> >> &(0x7fdc6000/0x18000)=nil, &(0x7f00c000)=[{0x0,
> >> &(0x7f00c000)="5ba3c16f533efbed09f8221253c73763327fadce2371813b45dd7f7982f84a873e4ae89a6c2bd1af83a6024c36a1ff518318",
> >> 0x32}], 0x1, 0x0, &(0x7f00d000-0x10)=[@featur2={0x1, 0x3}], 0x1)
> >
> > Is that the only thing the program does? Or is there anything running in
> > parallel?
> 
> These calls are executed repeatedly and in random order. That's all.
> 
> Except that I'm running the reproducer on a real arm board, so there's
> probably a bunch of stuff going on besides these calls.

I had a go at reproducing this on an arm64 board following [1], but so
far I've had no luck. I've dumped the above into syz-kvm-bug, and I'm
trying to reproduce the issue with:

PATH=$PATH:$(pwd)/bin syz-execprog \
-executor $(pwd)/bin/syz-executor \
-cover=0 -repeat=0 -procs 16 \
syz-kvm-bug

Just to check, is that the correct way to reproduce the problem with the
above log?

How quickly does that reproduce the problem for you?

Thanks,
Mark.

[1] https://github.com/google/syzkaller/wiki/How-to-execute-syzkaller-programs
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Re: kvm/arm64: use-after-free in kvm_unmap_hva_handler/unmap_stage2_pmds

[Andrey Konovalov]

On Wed, Apr 12, 2017 at 8:43 PM, Marc Zyngier  wrote:
> On 12/04/17 17:19, Andrey Konovalov wrote:
>
> Hi Andrey,
>
>> Apparently this wasn't fixed, I've got this report again on
>> linux-next-c4e7b35a3 (Apr 11), which includes 8b3405e34 "kvm:
>> arm/arm64: Fix locking for kvm_free_stage2_pgd".
>
> This looks like a different bug.

Oh, OK.

>
>>
>> I now have a way to reproduce it, so I can test proposed patches. I
>> don't have a simple C reproducer though.
>>
>> The bug happens when the following syzkaller program is executed:
>>
>> mmap(&(0x7f00/0xc000)=nil, (0xc000), 0x3, 0x32, 0x, 
>> 0x0)
>> unshare(0x400)
>> perf_event_open(&(0x7f02f000-0x78)={0x1, 0x78, 0x0, 0x0, 0x0, 0x0,
>> 0x0, 0x6, 0x0, 0x0, 0xd34, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0,
>> 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0}, 0x0, 0x,
>> 0x, 0x0)
>> r0 = openat$kvm(0xff9c,
>> &(0x7f00c000-0x9)="2f6465762f6b766d00", 0x0, 0x0)
>> ioctl$TIOCSBRK(0x, 0x5427)
>> r1 = ioctl$KVM_CREATE_VM(r0, 0xae01, 0x0)
>> syz_kvm_setup_cpu$arm64(r1, 0x,
>> &(0x7fdc6000/0x18000)=nil, &(0x7f00c000)=[{0x0,
>> &(0x7f00c000)="5ba3c16f533efbed09f8221253c73763327fadce2371813b45dd7f7982f84a873e4ae89a6c2bd1af83a6024c36a1ff518318",
>> 0x32}], 0x1, 0x0, &(0x7f00d000-0x10)=[@featur2={0x1, 0x3}], 0x1)
>
> Is that the only thing the program does? Or is there anything running in
> parallel?

These calls are executed repeatedly and in random order. That's all.

Except that I'm running the reproducer on a real arm board, so there's
probably a bunch of stuff going on besides these calls.

>
>> ==
>> BUG: KASAN: use-after-free in arch_spin_is_locked
>> include/linux/compiler.h:254 [inline]
>> BUG: KASAN: use-after-free in unmap_stage2_range+0x990/0x9a8
>> arch/arm64/kvm/../../../arch/arm/kvm/mmu.c:295
>> Read of size 8 at addr 84476730 by task syz-executor/13106
>>
>> CPU: 1 PID: 13106 Comm: syz-executor Not tainted
>> 4.11.0-rc6-next-20170411-xc2-11025-gc4e7b35a33d4-dirty #5
>> Hardware name: Hardkernel ODROID-C2 (DT)
>> Call trace:
>> [] dump_backtrace+0x0/0x440 arch/arm64/kernel/traps.c:505
>> [] show_stack+0x20/0x30 arch/arm64/kernel/traps.c:228
>> [] __dump_stack lib/dump_stack.c:16 [inline]
>> [] dump_stack+0x110/0x168 lib/dump_stack.c:52
>> [] print_address_description+0x60/0x248 
>> mm/kasan/report.c:252
>> [] kasan_report_error mm/kasan/report.c:351 [inline]
>> [] kasan_report+0x218/0x300 mm/kasan/report.c:408
>> [] __asan_report_load8_noabort+0x18/0x20 
>> mm/kasan/report.c:429
>> [] arch_spin_is_locked include/linux/compiler.h:254 
>> [inline]
>
> This is the assert on the spinlock, and the memory is gone.
>
>> [] unmap_stage2_range+0x990/0x9a8
>> arch/arm64/kvm/../../../arch/arm/kvm/mmu.c:295
>> [] kvm_free_stage2_pgd.part.16+0x30/0x98
>> arch/arm64/kvm/../../../arch/arm/kvm/mmu.c:842
>> [] kvm_free_stage2_pgd
>> arch/arm64/kvm/../../../arch/arm/kvm/mmu.c:838 [inline]
>
> But we've taken than lock here. There's only a handful of instructions
> in between, and the memory can only go away if there is something
> messing with us in parallel.
>
>> [] kvm_arch_flush_shadow_all+0x40/0x58
>> arch/arm64/kvm/../../../arch/arm/kvm/mmu.c:1895
>> [] kvm_mmu_notifier_release+0x154/0x1d0
>> arch/arm64/kvm/../../../virt/kvm/kvm_main.c:472
>> [] __mmu_notifier_release+0x1c0/0x3e0 mm/mmu_notifier.c:75
>> [] mmu_notifier_release
>> include/linux/mmu_notifier.h:235 [inline]
>> [] exit_mmap+0x21c/0x288 mm/mmap.c:2941
>> [] __mmput kernel/fork.c:888 [inline]
>> [] mmput+0xdc/0x2e0 kernel/fork.c:910
>> [] exit_mm kernel/exit.c:557 [inline]
>> [] do_exit+0x648/0x2020 kernel/exit.c:865
>> [] do_group_exit+0xdc/0x260 kernel/exit.c:982
>> [] get_signal+0x358/0xf58 kernel/signal.c:2318
>> [] do_signal+0x170/0xc10 arch/arm64/kernel/signal.c:370
>> [] do_notify_resume+0xe4/0x120 
>> arch/arm64/kernel/signal.c:421
>> [] work_pending+0x8/0x14
>
> So we're being serviced with a signal. Do you know if this signal is
> generated by your syzkaller program? We could be racing between do_exit
> triggered by a fatal signal (this trace) and the closing of the two file
> descriptors (vcpu and vm).

I'm not sure.

>
> Paolo: does this look possible to you? I can't see what locking we have
> that could prevent this race.
>
> Thanks,
>
> M.
> --
> Jazz is not dead. It just smells funny...
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Re: kvm/arm64: use-after-free in kvm_unmap_hva_handler/unmap_stage2_pmds

[Marc Zyngier]

On 12/04/17 17:19, Andrey Konovalov wrote:

Hi Andrey,

> Apparently this wasn't fixed, I've got this report again on
> linux-next-c4e7b35a3 (Apr 11), which includes 8b3405e34 "kvm:
> arm/arm64: Fix locking for kvm_free_stage2_pgd".

This looks like a different bug.

> 
> I now have a way to reproduce it, so I can test proposed patches. I
> don't have a simple C reproducer though.
> 
> The bug happens when the following syzkaller program is executed:
> 
> mmap(&(0x7f00/0xc000)=nil, (0xc000), 0x3, 0x32, 0x, 
> 0x0)
> unshare(0x400)
> perf_event_open(&(0x7f02f000-0x78)={0x1, 0x78, 0x0, 0x0, 0x0, 0x0,
> 0x0, 0x6, 0x0, 0x0, 0xd34, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0,
> 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0}, 0x0, 0x,
> 0x, 0x0)
> r0 = openat$kvm(0xff9c,
> &(0x7f00c000-0x9)="2f6465762f6b766d00", 0x0, 0x0)
> ioctl$TIOCSBRK(0x, 0x5427)
> r1 = ioctl$KVM_CREATE_VM(r0, 0xae01, 0x0)
> syz_kvm_setup_cpu$arm64(r1, 0x,
> &(0x7fdc6000/0x18000)=nil, &(0x7f00c000)=[{0x0,
> &(0x7f00c000)="5ba3c16f533efbed09f8221253c73763327fadce2371813b45dd7f7982f84a873e4ae89a6c2bd1af83a6024c36a1ff518318",
> 0x32}], 0x1, 0x0, &(0x7f00d000-0x10)=[@featur2={0x1, 0x3}], 0x1)

Is that the only thing the program does? Or is there anything running in
parallel?

> ==
> BUG: KASAN: use-after-free in arch_spin_is_locked
> include/linux/compiler.h:254 [inline]
> BUG: KASAN: use-after-free in unmap_stage2_range+0x990/0x9a8
> arch/arm64/kvm/../../../arch/arm/kvm/mmu.c:295
> Read of size 8 at addr 84476730 by task syz-executor/13106
> 
> CPU: 1 PID: 13106 Comm: syz-executor Not tainted
> 4.11.0-rc6-next-20170411-xc2-11025-gc4e7b35a33d4-dirty #5
> Hardware name: Hardkernel ODROID-C2 (DT)
> Call trace:
> [] dump_backtrace+0x0/0x440 arch/arm64/kernel/traps.c:505
> [] show_stack+0x20/0x30 arch/arm64/kernel/traps.c:228
> [] __dump_stack lib/dump_stack.c:16 [inline]
> [] dump_stack+0x110/0x168 lib/dump_stack.c:52
> [] print_address_description+0x60/0x248 
> mm/kasan/report.c:252
> [] kasan_report_error mm/kasan/report.c:351 [inline]
> [] kasan_report+0x218/0x300 mm/kasan/report.c:408
> [] __asan_report_load8_noabort+0x18/0x20 
> mm/kasan/report.c:429
> [] arch_spin_is_locked include/linux/compiler.h:254 [inline]

This is the assert on the spinlock, and the memory is gone.

> [] unmap_stage2_range+0x990/0x9a8
> arch/arm64/kvm/../../../arch/arm/kvm/mmu.c:295
> [] kvm_free_stage2_pgd.part.16+0x30/0x98
> arch/arm64/kvm/../../../arch/arm/kvm/mmu.c:842
> [] kvm_free_stage2_pgd
> arch/arm64/kvm/../../../arch/arm/kvm/mmu.c:838 [inline]

But we've taken than lock here. There's only a handful of instructions
in between, and the memory can only go away if there is something
messing with us in parallel.

> [] kvm_arch_flush_shadow_all+0x40/0x58
> arch/arm64/kvm/../../../arch/arm/kvm/mmu.c:1895
> [] kvm_mmu_notifier_release+0x154/0x1d0
> arch/arm64/kvm/../../../virt/kvm/kvm_main.c:472
> [] __mmu_notifier_release+0x1c0/0x3e0 mm/mmu_notifier.c:75
> [] mmu_notifier_release
> include/linux/mmu_notifier.h:235 [inline]
> [] exit_mmap+0x21c/0x288 mm/mmap.c:2941
> [] __mmput kernel/fork.c:888 [inline]
> [] mmput+0xdc/0x2e0 kernel/fork.c:910
> [] exit_mm kernel/exit.c:557 [inline]
> [] do_exit+0x648/0x2020 kernel/exit.c:865
> [] do_group_exit+0xdc/0x260 kernel/exit.c:982
> [] get_signal+0x358/0xf58 kernel/signal.c:2318
> [] do_signal+0x170/0xc10 arch/arm64/kernel/signal.c:370
> [] do_notify_resume+0xe4/0x120 
> arch/arm64/kernel/signal.c:421
> [] work_pending+0x8/0x14

So we're being serviced with a signal. Do you know if this signal is
generated by your syzkaller program? We could be racing between do_exit
triggered by a fatal signal (this trace) and the closing of the two file
descriptors (vcpu and vm).

Paolo: does this look possible to you? I can't see what locking we have
that could prevent this race.

Thanks,

M.
-- 
Jazz is not dead. It just smells funny...
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Re: kvm/arm64: use-after-free in kvm_unmap_hva_handler/unmap_stage2_pmds

[Andrey Konovalov]

On Tue, Mar 14, 2017 at 5:57 PM, Paolo Bonzini  wrote:
>
>
> On 14/03/2017 12:07, Suzuki K Poulose wrote:
>> On 10/03/17 13:34, Andrey Konovalov wrote:
>>> Hi,
>>>
>>> I've got the following error report while fuzzing the kernel with
>>> syzkaller.
>>>
>>> On linux-next commit 56b8bad5e066c23e8fa273ef5fba50bd3da2ace8 (Mar 8).
>>>
>>> Unfortunately I can't reproduce it.
>>>
>>> ==
>>> BUG: KASAN: use-after-free in put_page include/linux/compiler.h:243
>>> [inline]
>>> BUG: KASAN: use-after-free in unmap_stage2_pmds
>>> arch/arm64/kvm/../../../arch/arm/kvm/mmu.c:240 [inline]
>>> BUG: KASAN: use-after-free in unmap_stage2_puds
>>> arch/arm64/kvm/../../../arch/arm/kvm/mmu.c:269 [inline]
>>> BUG: KASAN: use-after-free in unmap_stage2_range+0x884/0x938
>>> arch/arm64/kvm/../../../arch/arm/kvm/mmu.c:299
>>> Read of size 8 at addr 80004585c000 by task syz-executor/5176
>>
>>
>>> [] put_page include/linux/compiler.h:243 [inline]
>>> [] unmap_stage2_pmds
>>> arch/arm64/kvm/../../../arch/arm/kvm/mmu.c:240 [inline]
>>> [] unmap_stage2_puds
>>> arch/arm64/kvm/../../../arch/arm/kvm/mmu.c:269 [inline]
>>> [] unmap_stage2_range+0x884/0x938
>>> arch/arm64/kvm/../../../arch/arm/kvm/mmu.c:299
>>> [] kvm_unmap_hva_handler+0x28/0x38
>>> arch/arm64/kvm/../../../arch/arm/kvm/mmu.c:1556
>>> [] handle_hva_to_gpa+0x140/0x250
>>> arch/arm64/kvm/../../../arch/arm/kvm/mmu.c:1547
>>> [] kvm_unmap_hva_range+0x60/0x80
>>> arch/arm64/kvm/../../../arch/arm/kvm/mmu.c:1579
>>> []
>>> kvm_mmu_notifier_invalidate_range_start+0x194/0x278
>>> arch/arm64/kvm/../../../virt/kvm/kvm_main.c:357
>>> [] __mmu_notifier_invalidate_range_start+0x1d0/0x2a0
>>> mm/mmu_notifier.c:199
>>> [] mmu_notifier_invalidate_range_start
>>> include/linux/mmu_notifier.h:282 [inline]
>>> [] unmap_vmas+0x12c/0x198 mm/memory.c:1372
>>> [] unmap_region+0x128/0x230 mm/mmap.c:2460
>>> [] update_hiwater_vm include/linux/mm.h:1483 [inline]
>>> [] remove_vma_list mm/mmap.c:2432 [inline]
>>> [] do_munmap+0x598/0x9b0 mm/mmap.c:2662
>>> [] find_vma_links mm/mmap.c:495 [inline]
>>> [] mmap_region+0x138/0xc78 mm/mmap.c:1610
>>> [] is_file_hugepages include/linux/hugetlb.h:269
>>> [inline]
>>> [] do_mmap+0x3cc/0x848 mm/mmap.c:1446
>>> [] do_mmap_pgoff include/linux/mm.h:2039 [inline]
>>> [] vm_mmap_pgoff+0xec/0x120 mm/util.c:305
>>> [] SYSC_mmap_pgoff mm/mmap.c:1475 [inline]
>>> [] SyS_mmap_pgoff+0x220/0x420 mm/mmap.c:1458
>>> [] sys_mmap+0x58/0x80 arch/arm64/kernel/sys.c:37
>>> [] el0_svc_naked+0x24/0x28
>>>
>>
>> We hold kvm->mmu_lock, while manipulating the stage2 ranges. However, I
>> find that
>> we don't take the lock, when we do it f rom kvm_free_stage2_pgd(), which
>> could
>> potentially have caused a problem with an munmap of a memslot ?
>>
>> Lightly tested...
>>
>>
>> commit fa75684dbf0fe845cf8403517d6e0c2c3344a544
>> Author: Suzuki K Poulose 
>> Date:   Tue Mar 14 10:26:54 2017 +
>>
>> kvm: arm: Fix locking for kvm_free_stage2_pgd
>> In kvm_free_stage2_pgd() we don't hold the kvm->mmu_lock while
>> calling
>> unmap_stage2_range() on the entire memory range for the guest. This
>> could
>> cause problems with other callers (e.g, munmap on a memslot) trying to
>> unmap a range.
>> Cc: Marc Zyngier 
>> Cc: Christoffer Dall 
>> Signed-off-by: Suzuki K Poulose 
>>
>> diff --git a/arch/arm/kvm/mmu.c b/arch/arm/kvm/mmu.c
>> index a07ce3e..7f97063 100644
>> --- a/arch/arm/kvm/mmu.c
>> +++ b/arch/arm/kvm/mmu.c
>> @@ -831,7 +831,10 @@ void kvm_free_stage2_pgd(struct kvm *kvm)
>> if (kvm->arch.pgd == NULL)
>> return;
>>
>> +   spin_lock(>mmu_lock);
>> unmap_stage2_range(kvm, 0, KVM_PHYS_SIZE);
>> +   spin_unlock(>mmu_lock);
>> +
>> /* Free the HW pgd, one page at a time */
>> free_pages_exact(kvm->arch.pgd, S2_PGD_SIZE);
>> kvm->arch.pgd = NULL;
>
> Reviewed-by: Paolo Bonzini 
>>
>>
>>
>>> The buggy address belongs to the page:
>>> page:7e0001161700 count:0 mapcount:0 mapping:  (null)
>>> index:0x0
>>> flags: 0xfffc000()
>>> raw: 0fffc000   
>>> raw: 7e00018c9120 7eea8b60  
>>> page dumped because: kasan: bad access detected
>>>
>>> Memory state around the buggy address:
>>>  80004585bf00: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
>>>  80004585bf80: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
 80004585c000: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
>>>^
>>>  80004585c080: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
>>>  80004585c100: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
>>> ==
>>>
>>

Hi,

Apparently this wasn't 

Re: kvm/arm64: use-after-free in kvm_unmap_hva_handler/unmap_stage2_pmds

[Paolo Bonzini]

On 14/03/2017 12:07, Suzuki K Poulose wrote:
> On 10/03/17 13:34, Andrey Konovalov wrote:
>> Hi,
>>
>> I've got the following error report while fuzzing the kernel with
>> syzkaller.
>>
>> On linux-next commit 56b8bad5e066c23e8fa273ef5fba50bd3da2ace8 (Mar 8).
>>
>> Unfortunately I can't reproduce it.
>>
>> ==
>> BUG: KASAN: use-after-free in put_page include/linux/compiler.h:243
>> [inline]
>> BUG: KASAN: use-after-free in unmap_stage2_pmds
>> arch/arm64/kvm/../../../arch/arm/kvm/mmu.c:240 [inline]
>> BUG: KASAN: use-after-free in unmap_stage2_puds
>> arch/arm64/kvm/../../../arch/arm/kvm/mmu.c:269 [inline]
>> BUG: KASAN: use-after-free in unmap_stage2_range+0x884/0x938
>> arch/arm64/kvm/../../../arch/arm/kvm/mmu.c:299
>> Read of size 8 at addr 80004585c000 by task syz-executor/5176
> 
> 
>> [] put_page include/linux/compiler.h:243 [inline]
>> [] unmap_stage2_pmds
>> arch/arm64/kvm/../../../arch/arm/kvm/mmu.c:240 [inline]
>> [] unmap_stage2_puds
>> arch/arm64/kvm/../../../arch/arm/kvm/mmu.c:269 [inline]
>> [] unmap_stage2_range+0x884/0x938
>> arch/arm64/kvm/../../../arch/arm/kvm/mmu.c:299
>> [] kvm_unmap_hva_handler+0x28/0x38
>> arch/arm64/kvm/../../../arch/arm/kvm/mmu.c:1556
>> [] handle_hva_to_gpa+0x140/0x250
>> arch/arm64/kvm/../../../arch/arm/kvm/mmu.c:1547
>> [] kvm_unmap_hva_range+0x60/0x80
>> arch/arm64/kvm/../../../arch/arm/kvm/mmu.c:1579
>> []
>> kvm_mmu_notifier_invalidate_range_start+0x194/0x278
>> arch/arm64/kvm/../../../virt/kvm/kvm_main.c:357
>> [] __mmu_notifier_invalidate_range_start+0x1d0/0x2a0
>> mm/mmu_notifier.c:199
>> [] mmu_notifier_invalidate_range_start
>> include/linux/mmu_notifier.h:282 [inline]
>> [] unmap_vmas+0x12c/0x198 mm/memory.c:1372
>> [] unmap_region+0x128/0x230 mm/mmap.c:2460
>> [] update_hiwater_vm include/linux/mm.h:1483 [inline]
>> [] remove_vma_list mm/mmap.c:2432 [inline]
>> [] do_munmap+0x598/0x9b0 mm/mmap.c:2662
>> [] find_vma_links mm/mmap.c:495 [inline]
>> [] mmap_region+0x138/0xc78 mm/mmap.c:1610
>> [] is_file_hugepages include/linux/hugetlb.h:269
>> [inline]
>> [] do_mmap+0x3cc/0x848 mm/mmap.c:1446
>> [] do_mmap_pgoff include/linux/mm.h:2039 [inline]
>> [] vm_mmap_pgoff+0xec/0x120 mm/util.c:305
>> [] SYSC_mmap_pgoff mm/mmap.c:1475 [inline]
>> [] SyS_mmap_pgoff+0x220/0x420 mm/mmap.c:1458
>> [] sys_mmap+0x58/0x80 arch/arm64/kernel/sys.c:37
>> [] el0_svc_naked+0x24/0x28
>>
> 
> We hold kvm->mmu_lock, while manipulating the stage2 ranges. However, I
> find that
> we don't take the lock, when we do it f rom kvm_free_stage2_pgd(), which
> could
> potentially have caused a problem with an munmap of a memslot ?
> 
> Lightly tested...
> 
> 
> commit fa75684dbf0fe845cf8403517d6e0c2c3344a544
> Author: Suzuki K Poulose 
> Date:   Tue Mar 14 10:26:54 2017 +
> 
> kvm: arm: Fix locking for kvm_free_stage2_pgd
> In kvm_free_stage2_pgd() we don't hold the kvm->mmu_lock while
> calling
> unmap_stage2_range() on the entire memory range for the guest. This
> could
> cause problems with other callers (e.g, munmap on a memslot) trying to
> unmap a range.
> Cc: Marc Zyngier 
> Cc: Christoffer Dall 
> Signed-off-by: Suzuki K Poulose 
> 
> diff --git a/arch/arm/kvm/mmu.c b/arch/arm/kvm/mmu.c
> index a07ce3e..7f97063 100644
> --- a/arch/arm/kvm/mmu.c
> +++ b/arch/arm/kvm/mmu.c
> @@ -831,7 +831,10 @@ void kvm_free_stage2_pgd(struct kvm *kvm)
> if (kvm->arch.pgd == NULL)
> return;
>  
> +   spin_lock(>mmu_lock);
> unmap_stage2_range(kvm, 0, KVM_PHYS_SIZE);
> +   spin_unlock(>mmu_lock);
> +
> /* Free the HW pgd, one page at a time */
> free_pages_exact(kvm->arch.pgd, S2_PGD_SIZE);
> kvm->arch.pgd = NULL;

Reviewed-by: Paolo Bonzini 
> 
> 
> 
>> The buggy address belongs to the page:
>> page:7e0001161700 count:0 mapcount:0 mapping:  (null)
>> index:0x0
>> flags: 0xfffc000()
>> raw: 0fffc000   
>> raw: 7e00018c9120 7eea8b60  
>> page dumped because: kasan: bad access detected
>>
>> Memory state around the buggy address:
>>  80004585bf00: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
>>  80004585bf80: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
>>> 80004585c000: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
>>^
>>  80004585c080: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
>>  80004585c100: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
>> ==
>>
> 
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Re: kvm/arm64: use-after-free in kvm_unmap_hva_handler/unmap_stage2_pmds

[Suzuki K Poulose]

On 10/03/17 13:34, Andrey Konovalov wrote:

Hi,

I've got the following error report while fuzzing the kernel with syzkaller.

On linux-next commit 56b8bad5e066c23e8fa273ef5fba50bd3da2ace8 (Mar 8).

Unfortunately I can't reproduce it.

==
BUG: KASAN: use-after-free in put_page include/linux/compiler.h:243 [inline]
BUG: KASAN: use-after-free in unmap_stage2_pmds
arch/arm64/kvm/../../../arch/arm/kvm/mmu.c:240 [inline]
BUG: KASAN: use-after-free in unmap_stage2_puds
arch/arm64/kvm/../../../arch/arm/kvm/mmu.c:269 [inline]
BUG: KASAN: use-after-free in unmap_stage2_range+0x884/0x938
arch/arm64/kvm/../../../arch/arm/kvm/mmu.c:299
Read of size 8 at addr 80004585c000 by task syz-executor/5176




[] put_page include/linux/compiler.h:243 [inline]
[] unmap_stage2_pmds
arch/arm64/kvm/../../../arch/arm/kvm/mmu.c:240 [inline]
[] unmap_stage2_puds
arch/arm64/kvm/../../../arch/arm/kvm/mmu.c:269 [inline]
[] unmap_stage2_range+0x884/0x938
arch/arm64/kvm/../../../arch/arm/kvm/mmu.c:299
[] kvm_unmap_hva_handler+0x28/0x38
arch/arm64/kvm/../../../arch/arm/kvm/mmu.c:1556
[] handle_hva_to_gpa+0x140/0x250
arch/arm64/kvm/../../../arch/arm/kvm/mmu.c:1547
[] kvm_unmap_hva_range+0x60/0x80
arch/arm64/kvm/../../../arch/arm/kvm/mmu.c:1579
[]
kvm_mmu_notifier_invalidate_range_start+0x194/0x278
arch/arm64/kvm/../../../virt/kvm/kvm_main.c:357
[] __mmu_notifier_invalidate_range_start+0x1d0/0x2a0
mm/mmu_notifier.c:199
[] mmu_notifier_invalidate_range_start
include/linux/mmu_notifier.h:282 [inline]
[] unmap_vmas+0x12c/0x198 mm/memory.c:1372
[] unmap_region+0x128/0x230 mm/mmap.c:2460
[] update_hiwater_vm include/linux/mm.h:1483 [inline]
[] remove_vma_list mm/mmap.c:2432 [inline]
[] do_munmap+0x598/0x9b0 mm/mmap.c:2662
[] find_vma_links mm/mmap.c:495 [inline]
[] mmap_region+0x138/0xc78 mm/mmap.c:1610
[] is_file_hugepages include/linux/hugetlb.h:269 [inline]
[] do_mmap+0x3cc/0x848 mm/mmap.c:1446
[] do_mmap_pgoff include/linux/mm.h:2039 [inline]
[] vm_mmap_pgoff+0xec/0x120 mm/util.c:305
[] SYSC_mmap_pgoff mm/mmap.c:1475 [inline]
[] SyS_mmap_pgoff+0x220/0x420 mm/mmap.c:1458
[] sys_mmap+0x58/0x80 arch/arm64/kernel/sys.c:37
[] el0_svc_naked+0x24/0x28



We hold kvm->mmu_lock, while manipulating the stage2 ranges. However, I find 
that
we don't take the lock, when we do it f rom kvm_free_stage2_pgd(), which could
potentially have caused a problem with an munmap of a memslot ?

Lightly tested...


commit fa75684dbf0fe845cf8403517d6e0c2c3344a544
Author: Suzuki K Poulose 
Date:   Tue Mar 14 10:26:54 2017 +

kvm: arm: Fix locking for kvm_free_stage2_pgd

In kvm_free_stage2_pgd() we don't hold the kvm->mmu_lock while calling

unmap_stage2_range() on the entire memory range for the guest. This could
cause problems with other callers (e.g, munmap on a memslot) trying to
unmap a range.

Cc: Marc Zyngier 

Cc: Christoffer Dall 
Signed-off-by: Suzuki K Poulose 

diff --git a/arch/arm/kvm/mmu.c b/arch/arm/kvm/mmu.c
index a07ce3e..7f97063 100644
--- a/arch/arm/kvm/mmu.c
+++ b/arch/arm/kvm/mmu.c
@@ -831,7 +831,10 @@ void kvm_free_stage2_pgd(struct kvm *kvm)
if (kvm->arch.pgd == NULL)
return;
 
+   spin_lock(>mmu_lock);

unmap_stage2_range(kvm, 0, KVM_PHYS_SIZE);
+   spin_unlock(>mmu_lock);
+
/* Free the HW pgd, one page at a time */
free_pages_exact(kvm->arch.pgd, S2_PGD_SIZE);
kvm->arch.pgd = NULL;





The buggy address belongs to the page:
page:7e0001161700 count:0 mapcount:0 mapping:  (null) index:0x0
flags: 0xfffc000()
raw: 0fffc000   
raw: 7e00018c9120 7eea8b60  
page dumped because: kasan: bad access detected

Memory state around the buggy address:
 80004585bf00: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
 80004585bf80: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff

80004585c000: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff

   ^
 80004585c080: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
 80004585c100: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
==



___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm