Re: KASAN: use-after-free Read in kvm_put_kvm

[Paolo Bonzini]

On 20/10/2018 18:57, syzbot wrote:
> Hello,
> 
> syzbot found the following crash on:
> 
> HEAD commit:ÂÂÂ 8c60c36d0b8c Add linux-next specific files for 20181019
> git tree:ÂÂÂÂÂÂ linux-next
> console output: https://syzkaller.appspot.com/x/log.txt?x=12d808b5400000
> kernel config:Â https://syzkaller.appspot.com/x/.config?x=8b6d7c4c81535e89
> dashboard link:
> https://syzkaller.appspot.com/bug?extid=f87f60bb6f13f39b54e3
> compiler:ÂÂÂÂÂÂ gcc (GCC) 8.0.1 20180413 (experimental)
> syz repro:ÂÂÂÂÂ https://syzkaller.appspot.com/x/repro.syz?x=17cf2f5e400000
> C reproducer:ÂÂ https://syzkaller.appspot.com/x/repro.c?x=1036dcce400000
> 
> IMPORTANT: if you fix the bug, please add the following tag to the commit:
> Reported-by: syzbot+f87f60bb6f13f39b54e3@xxxxxxxxxxxxxxxxxxxxxxxxx

Tentative (untested) patch:

diff --git a/virt/kvm/coalesced_mmio.c b/virt/kvm/coalesced_mmio.c
index 3710342cf6ad..dc76cc8d24fd 100644
--- a/virt/kvm/coalesced_mmio.c
+++ b/virt/kvm/coalesced_mmio.c
@@ -90,6 +90,7 @@ static int coalesced_mmio_write(struct kvm_vcpu *vcpu,
 	return 0;
 }

+/* called with kvm->slots_lock held */
 static void coalesced_mmio_destructor(struct kvm_io_device *this)
 {
 	struct kvm_coalesced_mmio_dev *dev = to_mmio(this);
diff --git a/virt/kvm/eventfd.c b/virt/kvm/eventfd.c
index b20b751286fc..001e1ef18c8c 100644
--- a/virt/kvm/eventfd.c
+++ b/virt/kvm/eventfd.c
@@ -741,8 +741,8 @@ ioeventfd_write(struct kvm_vcpu *vcpu, struct
kvm_io_device *this, gpa_t addr,
 }

 /*
- * This function is called as KVM is completely shutting down.  We do not
- * need to worry about locking just nuke anything we have as quickly as
possible
+ * This function is called as KVM is completely shutting down, so there
+ * are no RCU readers anymore. Called with kvm->slots_lock held.
  */
 static void
 ioeventfd_destructor(struct kvm_io_device *this)
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index aff1242b7af7..e6f2ae6fedcd 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -783,6 +783,7 @@ static void kvm_destroy_vm(struct kvm *kvm)
 	list_del(&kvm->vm_list);
 	spin_unlock(&kvm_lock);
 	kvm_free_irq_routing(kvm);
+	mutex_lock(&kvm->slots_lock);
 	for (i = 0; i < KVM_NR_BUSES; i++) {
 		struct kvm_io_bus *bus = kvm_get_bus(kvm, i);

@@ -790,6 +791,7 @@ static void kvm_destroy_vm(struct kvm *kvm)
 			kvm_io_bus_destroy(bus);
 		kvm->buses[i] = NULL;
 	}
+	mutex_unlock(&kvm->slots_lock);
 	kvm_coalesced_mmio_free(kvm);
 #ifdef KVM_DIRTY_LOG_PAGE_OFFSET
 	if (kvm->dirty_ring_size)