From 75aa2c881fa66d1ac3c799b1bd2df4ef7b4e3cd5 Mon Sep 17 00:00:00 2001
From: Izik Eidus <ieidus@redhat.com>
Date: Sun, 7 Jun 2009 19:45:41 +0300
Subject: [PATCH] fix bug between mm_to_kvm() and kvm_get_kvm() (RHEL-5 backport)

Gleb Natapov wrote:
> On Sun, Jun 07, 2009 at 01:00:46PM +0300, Izik Eidus wrote:
>
>> Signed-off-by: Izik Eidus <ieidus@redhat.com>
>> ---
>>  virt/kvm/kvm_main.c |    4 ++--
>>  1 files changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
>> index 68e5b2d..afb3220 100644
>> --- a/virt/kvm/kvm_main.c
>> +++ b/virt/kvm/kvm_main.c
>> @@ -903,9 +903,7 @@ static void kvm_destroy_vm(struct kvm *kvm)
>>  	struct mm_struct *mm = kvm->mm;
>>
>>  	kvm_arch_sync_events(kvm);
>> -	spin_lock(&kvm_lock);
>>  	list_del(&kvm->vm_list);
>> -	spin_unlock(&kvm_lock);
>>  	kvm_io_bus_destroy(&kvm->pio_bus);
>>  	kvm_io_bus_destroy(&kvm->mmio_bus);
>>  #ifdef KVM_COALESCED_MMIO_PAGE_OFFSET
>> @@ -927,8 +925,10 @@ EXPORT_SYMBOL_GPL(kvm_get_kvm);
>>
>>  void kvm_put_kvm(struct kvm *kvm)
>>  {
>> +        spin_lock(&kvm_lock);
>>  	if (atomic_dec_and_test(&kvm->users_count))
>>  		kvm_destroy_vm(kvm);
>> +        spin_unlock(&kvm_lock);
>>  }
>>  EXPORT_SYMBOL_GPL(kvm_put_kvm);
>>
>>
> Now kvm_destroy_vm() runs in atomic context. This is not trivial amount
> of code to audit. (the functions that can run: kvm_free_all_assigned_devices(),
> kvm_iommu_unmap_guest(), many others).
>
> --
> 			Gleb.
>
>

Use this patch.

From b95ba91a10f58c513d4a1fd3e21231e5ff7ac994 Mon Sep 17 00:00:00 2001
From: Izik Eidus <ieidus@redhat.com>
Date: Sun, 7 Jun 2009 11:40:55 -0400
Subject: [PATCH] kvm: fix refcount race between vm destruction and users that fetch kvm_get_kvm

what that can happen is this:
kvm_put_kvm() will run:
atomic_dec_and_test(user_count) that will get the users_count to be 0
and therefore kvm_destroy_vm() will be called.

in the same time, beacuse the kvm_lock was still not taken, and the kvm strcture
of the vm was not delete from the kvm_list, another user will fetch kvm and
will run: kvm_get_kvm() that will increase users_count to be 1
then this user will run kvm_put_kvm() and we will have situation that we will
run kvm_destroy_vm() twice.

(it appear bot to be a problem in the mainline, as it doesnt have mm_to_kvm)

Thanks.

[ehabkost: backported to RHEL-5 branch]

Signed-off-by: Izik Eidus <ieidus@redhat.com>
Obsoletes: <4A2BEEB5.7070303@redhat.com>
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
Bugzilla: 504237
RH-Upstream-status: not-applicable
Obsoletes: <4A2B8FCE.80506@redhat.com>
Obsoletes: <4A2C7D6B.80908@redhat.com>
Message-ID: <20090608145226.GD18045@blackpad>
Acked-by: Rik van Riel <riel@redhat.com>
Acked-by: Izik Eidus <ieidus@redhat.com>
Acked-by: Andrea Arcangeli <aarcange@redhat.com>
Acked-by: Gleb Natapov <gleb@redhat.com>
---
 virt/kvm/kvm_main.c |   11 +++++++----
 1 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index 10966bf..21473e3 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -1023,9 +1023,6 @@ static void kvm_destroy_vm(struct kvm *kvm)
 	struct mm_struct *mm = kvm->mm;
 
 	kvm_arch_sync_events(kvm);
-	spin_lock(&kvm_lock);
-	list_del(&kvm->vm_list);
-	spin_unlock(&kvm_lock);
 	kvm_free_irq_routing(kvm);
 	kvm_io_bus_destroy(&kvm->pio_bus);
 	kvm_io_bus_destroy(&kvm->mmio_bus);
@@ -1048,8 +1045,14 @@ EXPORT_SYMBOL_GPL(kvm_get_kvm);
 
 void kvm_put_kvm(struct kvm *kvm)
 {
-	if (atomic_dec_and_test(&kvm->users_count))
+	spin_lock(&kvm_lock);
+	if (atomic_dec_and_test(&kvm->users_count)) {
+		list_del(&kvm->vm_list);
+		spin_unlock(&kvm_lock);
 		kvm_destroy_vm(kvm);
+	} else {
+       		spin_unlock(&kvm_lock);
+	}
 }
 EXPORT_SYMBOL_GPL(kvm_put_kvm);
 
-- 
1.6.3.rc4.29.g8146